<< Back to previous view

[CLJ-1171] Compiler macro for clojure.core/instance? disregards lexical shadows on class names Created: 27/Feb/13  Updated: 14/Aug/13  Resolved: 14/Aug/13

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

Type: Defect Priority: Major
Reporter: Herwig Hochleitner Assignee: Stuart Halloway
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 0001-CLJ-1171-Tests-for-clojure.core-instance-compiler-ma.patch     Text File 0002-CLJ-1171-Obey-lexical-scope-for-class-argument-in-in.patch     Text File 0003-CLJ-1171-Check-arity-in-instance-compiler-macro.patch    
Patch: Code and Test
Approval: Ok



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


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


Test case

user=> (let [Long String] (instance? Long "abc"))
;; expected true as in
user=> (let [Long String] (apply instance? [Long "abc"]))

Culprit method


List Discussion



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)
;; 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

Comment by Herwig Hochleitner [ 27/Feb/13 8:11 PM ]

Attached patches test and fix issue + tangent

Comment by Herwig Hochleitner [ 04/Mar/13 3:51 PM ]

Note: Patch 0003 just adds the arity check, hence is optional, but if it's omitted from the patchset, the corresponding test from patch 0001 will fail.

Comment by Stuart Halloway [ 29/Mar/13 7:31 AM ]

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.

Generated at Sun Jan 21 13:13:21 CST 2018 using JIRA 4.4#649-r158309.