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:Vetted
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
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
Herwig Hochleitner
made changes -
| Field | Original Value | New Value |
|---|---|---|
| Description |
The compiler tries to emit instanceof expressions for instance? calls.
For that, it tries to resolve its first argument as a class name. However, it disregards lexical bindings when doing that. Test case: {code} user=> (let [Long String] (instance? Long "abc")) false {code} Culprit: https://github.com/clojure/clojure/blob/4ccb10edbe66eae81336a4c0972050d9759b0bf7/src/jvm/clojure/lang/Compiler.java#L3578 Discussion: https://groups.google.com/d/topic/clojure/mf25OlFRpa8/discussion |
h1. 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 h1. Data h2. Test case {code} user=> (let [Long String] (instance? Long "abc")) false ;; expected true as in user=> (let [Long String] (apply instance? [Long "abc"])) true {code} h2. Culprit method https://github.com/clojure/clojure/blob/4ccb10edbe66eae81336a4c0972050d9759b0bf7/src/jvm/clojure/lang/Compiler.java#L3578 h2. List Discussion https://groups.google.com/d/topic/clojure/mf25OlFRpa8/discussion h1. 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: {code} 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) {code} {quote} *EDIT* elaborated on ticket title and description; added tangent {quote} |
| Summary | Direct calls to instance? disregard shadowed class names | Compiler macro for clojure.core/instance? disregards lexical shadows on class names |
Herwig Hochleitner
made changes -
| Attachment | 0003-CLJ-1171-Check-arity-in-instance-compiler-macro.patch [ 11879 ] | |
| Attachment | 0002-CLJ-1171-Obey-lexical-scope-for-class-argument-in-in.patch [ 11878 ] | |
| Attachment | 0001-CLJ-1171-Tests-for-clojure.core-instance-compiler-ma.patch [ 11877 ] |
Andy Fingerhut
made changes -
| Patch | Code and Test [ 10002 ] |
Hide
Stuart Halloway
added a comment -
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. 
Show
Stuart Halloway
added a comment - 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)
Stuart Halloway
made changes -
| Approval | Triaged [ 10120 ] |
Stuart Halloway
made changes -
| Comment | [ Do you think you're dealing with a lot of economic related difficulties? If yes how to overcome these individuals properly is not known to your account while you possess eliminated out of fund. In such circumstance you can not head over to your current family members and enquire for cash for anyone who is working someplace. In their normal time period this will become noticeable that you should hunt for many trustworthy method to obtain dollars from and acquire cash aid devoid of proceeding long distance and regarding throughout lengthy means of acceptance. http://www.online12monthloans24h.co.uk/ Next, you shouldn't be concerned whatsoever and test immediate 12 calendar month financial products seeing that below this system you can get the quantity you'll need instantly and for you don't should anyplace just like financial institution or perhaps any office on the loan companies. It is possible to acquire the income through this system on the web and for total one year. ] |
Rich Hickey
made changes -
| Approval | Triaged [ 10120 ] | Vetted [ 10003 ] |
Rich Hickey
made changes -
| Fix Version/s | Release 1.6 [ 10157 ] |
Attached patches test and fix issue + tangent