<< Back to previous view

[CLJ-852] IllegalArgumentException thrown when defining a var whose value is calculated with a primitive fn. Created: 10/Oct/11  Updated: 23/Mar/12  Resolved: 23/Mar/12

Status: Closed
Project: Clojure
Component/s: None
Affects Version/s: Release 1.3
Fix Version/s: Release 1.4

Type: Defect Priority: Critical
Reporter: Alexander Taggart Assignee: Ben Smith-Mannschott
Resolution: Completed Votes: 4
Labels: None

Attachments: Text File CLJ-852-fix.patch     Text File clj-852-patch2.txt     Text File CLJ-852-rfc-fix.patch     Text File CLJ-852-test.patch    
Patch: Code and Test
Approval: Ok
Waiting On: Rich Hickey

 Description   

Note the following stacktrace line numbers are from the latest 1.4-snapshot, though this happens on 1.3 as well.

Example:

user=> (defn foo ^long [^long x] x)
#'user/foo
user=> (def x (inc (foo 10)))
CompilerException java.lang.IllegalArgumentException: Unable to resolve classname: long, compiling:(NO_SOURCE_PATH:2) 
user=> (pst)
CompilerException java.lang.IllegalArgumentException: Unable to resolve classname: long, compiling:(NO_SOURCE_PATH:2)
	clojure.lang.Compiler.analyzeSeq (Compiler.java:6444)
	clojure.lang.Compiler.analyze (Compiler.java:6244)
	clojure.lang.Compiler.analyze (Compiler.java:6205)
	clojure.lang.Compiler.analyzeSeq (Compiler.java:6432)
	clojure.lang.Compiler.analyze (Compiler.java:6244)
	clojure.lang.Compiler.access$100 (Compiler.java:37)
	clojure.lang.Compiler$DefExpr$Parser.parse (Compiler.java:492)
	clojure.lang.Compiler.analyzeSeq (Compiler.java:6437)
	clojure.lang.Compiler.analyze (Compiler.java:6244)
	clojure.lang.Compiler.analyze (Compiler.java:6205)
	clojure.lang.Compiler.eval (Compiler.java:6497)
	clojure.lang.Compiler.eval (Compiler.java:6459)
Caused by:
IllegalArgumentException Unable to resolve classname: long
	clojure.lang.Compiler$HostExpr.tagToClass (Compiler.java:1003)
	clojure.lang.Compiler$InvokeExpr.getJavaClass (Compiler.java:3474)
	clojure.lang.Compiler.getMatchingParams (Compiler.java:2304)
	clojure.lang.Compiler$StaticMethodExpr.<init> (Compiler.java:1510)
	clojure.lang.Compiler$HostExpr$Parser.parse (Compiler.java:910)
	clojure.lang.Compiler.analyzeSeq (Compiler.java:6437)
	clojure.lang.Compiler.analyze (Compiler.java:6244)

Note though that the following works fine:

user=> (def x (foo (inc 10)))
#'user/x


 Comments   
Comment by Ben Smith-Mannschott [ 15/Oct/11 10:13 AM ]

CLJ-852-test.patch
reproduces this issue on master

Comment by Ben Smith-Mannschott [ 15/Oct/11 10:41 AM ]

CLJ-852-test.patch initially failed to use (is ...) leading to ugly output on failure.

Comment by Ben Smith-Mannschott [ 15/Oct/11 2:59 PM ]

CLJ-852-rfc-fix.patch

add special cases for "int"…"boolean" to tagToClass()

This causes the test to pass, however, there are still some open questions:

Why does tagToClass first call maybeClass and then possibly overwrite the result it gets back without even looking at it? This strikes me as surprising. I would have expected a check for null, and then only running the if/else chain of special cases if maybeClass failed to return a non-null result.

The suppressed exception in maybeClass commented with "//aargh" makes me wonder if maybeClass and tagToClass are factored correctly.

Comment by Stuart Halloway [ 25/Oct/11 6:00 PM ]

This looks reasonable to me, but given the number of places tagToClass gets called I would definitely want Rich to take a look.

Comment by Ben Smith-Mannschott [ 02/Nov/11 6:12 AM ]

See also
http://groups.google.com/group/clojure/browse_thread/thread/f1672a7e09c34721

Comment by Ben Smith-Mannschott [ 13/Nov/11 9:18 AM ]

CLJ-852-fix.patch removes the RFC comments

This fixes the issue. It appears to me like the cases "int" .. "boolean" (i.e. the non-array primitive types) were simply forgotten in tagToClass().

It's not impossible that tagToClass is being misused somehow ant that's the cause of the problem, but I see no evidence for that. (I'd need a better understanding of the code base to make that determination – mind reading skills would help too.)

It's also conceivable that maybeClass() should be doing the resolution from primitive type to Class object, but that doesn't seem likely to me.

Comment by Andy Fingerhut [ 22/Feb/12 9:02 PM ]

clj-852-patch2.txt is exactly the same as Ben's CLJ-852-fix.patch (including keeping his name and dates on the commits), except:

(1) I moved his new unit tests to an already-existing file metadata.clj, which seemed to have related tests with def and metadata, whereas Ben's patch adds a brand new test file. Not sure whether keeping the number of files down is important or not.

(2) I removed some non-ASCII characters from his commit notes.

Both his patch and this newer one apply cleanly to latest master as of Feb 22, 2012. Neither adds any new errors or warnings to compilation or tests (unless you apply the new test part of the patch without his proposed compiler fix, where the new test does fail as expected). No docstrings need changing that I can think of. Ben and I have both signed CAs. Patch status is "Code and Test".

Comment by Stuart Sierra [ 24/Feb/12 1:32 PM ]

Screened. The addition seems harmless, merely recognizing more tags in tagToClass.

Generated at Thu Jul 24 18:09:32 CDT 2014 using JIRA 4.4#649-r158309.