Comments (20)
@kolharsam Is this something you'd be interested in helping to fix?
from ordered.
In an experiment with Clojure 1.10.1 and this ordered library, I made a local change to the deftype OrderedMap
so that its type hints have fully qualified Java interface/class names, like so:
(deftype OrderedMap [^clojure.lang.IPersistentMap backing-map
^clojure.lang.IPersistentVector order]
With only those changes, the REPL sequence in the original issue gives no exceptions or messages of any kind.
I do not know all the rules here, but I will point out that this Clojure source code written by Rich Hickey uses fully qualified interface/class names in similar situations: https://github.com/clojure/clojure/blob/master/src/clj/clojure/gvec.clj
from ordered.
I would guess there should be no harm in replacing all occurrences of such Java class/interface names with their fully qualified counterparts, throughout this library, other than a few extra characters in the source code.
from ordered.
@kolharsam The lines of code you reference that are in the current latest version of this library:
(deftype OrderedMap [^IPersistentMap backing-map
^IPersistentVector order]
lead to the exception being thrown in the REPL when attempting to evaluate the sequence of expressions in the first comment of this issue.
My experiment was to change those lines to something similar, but not the same as they currently are. Look carefully for the changes. I guarantee you my suggested replacement lines are different than the current ones:
(deftype OrderedMap [^clojure.lang.IPersistentMap backing-map
^clojure.lang.IPersistentVector order]
from ordered.
@kolharsam asked: "I'm interested in how/when did you realize that eval was causing the problem?"
Two clues:
(1) The first is the stack trace in my previous comment, that came as output from evaluating (pst)
after the exception occurred. It has several lines with clojure.core/eval in them.
(2) The call to read-string
was not causing an exception, but entering the expression #ordered ...
in the REPL was. REPL is Read, Eval, Print, Loop. Read was not the cause, so perhaps Eval was.
from ordered.
I think I see now. The second commit you point at is a reformatting one only, after another separate commit just before that which added those tests. Those tests do not exist in the latest master of the original repository, so that is why the latest master is not failing those tests.
You should also add similar tests for ordered sets as the one you added for ordered maps.
from ordered.
@borkdude affirmative!
The tests are failing. I got the same CompilerException java.lang.IllegalArgumentException: Unable to resolve classname: IPersistentMap
error on those tests
from ordered.
@kolharsam I think so, yes.
from ordered.
I do not know why this happens, but off the top of my head as a guess, perhaps adding an import or three into the code, and/or replacing some occurrence of IPersistentVector with clojure.lang.IPersistentVector in the code of the ordered library, might avoid this behavior?
from ordered.
@borkdude I'll try and get back to you soon. Upon investigating it further. If you've got any ideas, please share :)
from ordered.
@jafingerhut I found one occurrence here:
ordered/src/flatland/ordered/map.clj
Line 29 in 714f32a
I guess the solution that you suggested is already present. Would it be wrong to think that something else might be the cause of the error?
from ordered.
@jafingerhut What you suggested seems to be working for the sequence above. And also my apologies for not looking at it carefully initially. Should I write some test(s) for this?
Also, might there be other classes where this problem might occur? You did already suggest replacing all of the classnames with their fully qualified names. Should I be doing that as well? Or would that better in a different fix/issue altogether?
from ordered.
It might be best to only change the occurrences that cause the symptoms of this problem to go away, which seem to be the ones in my previous message for the ordered maps.
For ordered sets, I bet there is a similar issue, and a similar fix.
It would be good if there were a way to write a test that fails for the current code, but succeeds with the fix. I do not understand why this test is passing, for example, given the behavior at a REPL given in the original issue: https://github.com/clj-commons/ordered/blob/master/test/flatland/ordered/map_test.clj#L184-L187
from ordered.
Given this interaction below, I do not know how to add a failing test for the current code, since read-string
works without error, without any changes to the latest source code. I do not know why the difference between read-string
versus entering the expression at a REPL, except that it might have something to do with Leiningen and/or nrepl.
$ lein version
Leiningen 2.9.3 on Java 1.8.0_192 Java HotSpot(TM) 64-Bit Server VM
$ lein repl
nREPL server started on port 63296 on host 127.0.0.1 - nrepl://127.0.0.1:63296
REPL-y 0.4.4, nREPL 0.6.0
Clojure 1.9.0
Java HotSpot(TM) 64-Bit Server VM 1.8.0_192-b12
Docs: (doc function-name-here)
(find-doc "part-of-name-here")
Source: (source function-name-here)
Javadoc: (javadoc java-object-or-class-here)
Exit: Control+D or (exit) or (quit)
Results: Stored in vars *1, *2, *3, an exception in *e
user=> *clojure-version*
{:major 1, :minor 9, :incremental 0, :qualifier nil}
user=> (require '[flatland.ordered.map :as m])
nil
user=> (read-string "#ordered/map[[:a 1] [:b 2]]")
#ordered/map ([:a 1] [:b 2])
user=> #ordered/map[[:a 1] [:b 2]]
CompilerException java.lang.IllegalArgumentException: Unable to resolve classname: IPersistentMap, compiling:(/private/var/folders/rv/2vbzx7zj64x4hss7rdl1xywc0000gn/T/form-init2106877030880329831.clj:1:1479)
user=> (pst)
CompilerException java.lang.IllegalArgumentException: Unable to resolve classname: IPersistentMap, compiling:(/private/var/folders/rv/2vbzx7zj64x4hss7rdl1xywc0000gn/T/form-init2106877030880329831.clj:1:1479)
clojure.lang.Compiler.analyzeSeq (Compiler.java:7010)
clojure.lang.Compiler.analyze (Compiler.java:6773)
clojure.lang.Compiler.eval (Compiler.java:7059)
clojure.lang.Compiler.eval (Compiler.java:7025)
clojure.core/eval (core.clj:3206)
clojure.core/eval (core.clj:3202)
clojure.main/repl/read-eval-print--8572/fn--8575 (main.clj:243)
clojure.main/repl/read-eval-print--8572 (main.clj:243)
clojure.main/repl/fn--8581 (main.clj:261)
clojure.main/repl (main.clj:261)
clojure.main/repl (main.clj:177)
nrepl.middleware.interruptible-eval/evaluate (interruptible_eval.clj:79)
Caused by:
IllegalArgumentException Unable to resolve classname: IPersistentMap
clojure.lang.Compiler$HostExpr.tagToClass (Compiler.java:1129)
clojure.lang.Compiler.tagClass (Compiler.java:8567)
clojure.lang.Compiler$ObjExpr.emitValue (Compiler.java:4799)
clojure.lang.Compiler$ObjExpr.emitConstants (Compiler.java:4927)
clojure.lang.Compiler$ObjExpr.compile (Compiler.java:4605)
clojure.lang.Compiler$FnExpr.parse (Compiler.java:4099)
clojure.lang.Compiler.analyzeSeq (Compiler.java:7001)
clojure.lang.Compiler.analyze (Compiler.java:6773)
nil
user=>
from ordered.
It is not due to Leiningen or nrepl, as I guessed in my previous comment, because I get the same behavior with the Clojure CLI clj
command, too.
It is due to attempting to eval
the read result, as one can do in an automated test by trying to evaluate this expression: (eval (read-string "#ordered/map[[:a 1] [:b 2]]"))
from ordered.
@jafingerhut I have added these 2 commits, would it be sufficient to resolve this issue? (to submit a PR)
1.kolharsam@8c6c8cc
2.kolharsam@fa854a0
Also, I'm interested in how/when did you realize that eval
was causing the problem? Once I read your last comment it was a real aha moment for me.
from ordered.
Why the formatting change in kolharsam@fa854a0? And how do you test if the issue was resolved?
from ordered.
Also, as borkdude mentioned, it would be good to add a test that fails without the fix, and passes with the fix.
The tests that you reformat in one of your commits are interesting. They include (eval (read-string "#ordered ..."))
, but they do not throw an exception already, without the fix? I wonder why they do not throw an exception now?
from ordered.
Ah right. Just to confirm, did the tests you added fail without the fix @kolharsam?
from ordered.
@jafingerhut upon adding similar tests for Ordered Sets
, this should be good for a PR?
from ordered.
Related Issues (20)
- read from file into ordered map HOT 1
- What can it be used at? HOT 4
- [1.5.6] Must hint overloaded method: toArray HOT 5
- Interested in transitioning amalloy/ordered to clj-commons? HOT 1
- Can we get this to work on cljs? HOT 7
- ordered/set fails on Java 11 HOT 3
- Ordered pulls in useful
- Is it possible to get the min/max element? HOT 4
- ordered-1.5.8 gives reflection warnings
- Strange rendering at the repl HOT 4
- No changelog updates
- Reader literals assume imports HOT 5
- Make `ordered` auto-deployable
- Remove support for Clojure versions prior to 1.8
- Make `ordered-set` available for cljs
- Associative methods don't work on transient ordered maps HOT 1
- Add ordered set operations?
- Runtime resolve bloats native image size
- CI seems to suffer from bitrot HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from ordered.