Clojure

IllegalArgumentException thrown when defining a var whose value is calculated with a primitive fn.

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Critical Critical
  • Resolution: Completed
  • Affects Version/s: Release 1.3
  • Fix Version/s: Release 1.4
  • Component/s: None
  • Labels:
    None
  • Patch:
    Code and Test
  • Approval:
    Ok

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

Attachments

  1. Text File
    CLJ-852-fix.patch
    13/Nov/11 9:18 AM
    5 kB
    Ben Smith-Mannschott
  2. Text File
    clj-852-patch2.txt
    22/Feb/12 9:02 PM
    4 kB
    Andy Fingerhut
  3. Text File
    CLJ-852-rfc-fix.patch
    15/Oct/11 2:59 PM
    4 kB
    Ben Smith-Mannschott
  4. Text File
    CLJ-852-test.patch
    15/Oct/11 10:41 AM
    2 kB
    Ben Smith-Mannschott

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Ben Smith-Mannschott added a comment -

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

Show
Ben Smith-Mannschott added a comment - CLJ-852-test.patch reproduces this issue on master
Hide
Permalink
Ben Smith-Mannschott added a comment -

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

Show
Ben Smith-Mannschott added a comment - CLJ-852-test.patch initially failed to use (is ...) leading to ugly output on failure.
Hide
Permalink
Ben Smith-Mannschott added a comment -

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.

Show
Ben Smith-Mannschott added a comment - 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.
Hide
Permalink
Stuart Halloway added a comment -

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

Show
Stuart Halloway added a comment - This looks reasonable to me, but given the number of places tagToClass gets called I would definitely want Rich to take a look.
Hide
Permalink
Ben Smith-Mannschott added a comment -

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.

Show
Ben Smith-Mannschott added a comment - 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.
Hide
Permalink
Andy Fingerhut added a comment -

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".

Show
Andy Fingerhut added a comment - 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".
Hide
Permalink
Stuart Sierra added a comment -

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

Show
Stuart Sierra added a comment - Screened. The addition seems harmless, merely recognizing more tags in tagToClass.

People

Vote (4)
Watch (5)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4#649-r158309) | Report a problem
Powered by a free Atlassian JIRA open source license for Clojure. Try JIRA - bug tracking software for your team.