Details
-
Type:
Defect
-
Status:
Open
-
Priority:
Major
-
Resolution: Unresolved
-
Affects Version/s: None
-
Fix Version/s: Release 1.6
-
Component/s: None
-
Labels:None
-
Patch:Code and Test
-
Approval:Screened
Description
Summary
The compiler tries to emit jvm native instanceof expressions for direct clojure.core/instance? calls.
For that, it tries to resolve its first argument as a class name. However, it disregards lexical bindings when doing that.
This is incongruent to the default implementation in core.clj
Patches
[Stu] All three patches should be applied IMO.
- 0002 makes instance? respect lexical bindings
- 0003 makes instance?'s compiled form check arity, consistent with higher-order behavior
- 0001 has a minimal test for both 0002 and 0003.
Data
Test case
user=> (let [Long String] (instance? Long "abc")) false ;; expected true as in user=> (let [Long String] (apply instance? [Long "abc"])) true
Culprit method
List Discussion
https://groups.google.com/d/topic/clojure/mf25OlFRpa8/discussion
Tangent
This was discovered because the same compiler macro also omits the arity check implicit in the default definition. This could also conveniently be fixed when touching that method:
user=> (instance? String) false ;; expected user=> (apply instance? [String]) ArityException Wrong number of args (1) passed to: core$instance-QMARK- clojure.lang.AFn.throwArity (AFn.java:437)
EDIT elaborated on ticket title and description; added tangent
Attachments
Activity
Summarizing the decisions in these patches:
- instance? and apply instance? should be consistent
- they should check arity (matching apply instance? existing behavior)
- they should allow lexical shadowing of the class argument (matching apply instance? existing behavior)
It is possible (although unlikely) that existing code relies on the current eccentric behavior of instance?. I think it would be fair to categorize programs relying on this behavior as buggy, but that is easy for me to say. 
- instance? and apply instance? should be consistent
- they should check arity (matching apply instance? existing behavior)
- they should allow lexical shadowing of the class argument (matching apply instance? existing behavior)
Attached patches test and fix issue + tangent