Clojure

Compiler macro for clojure.core/instance? disregards lexical shadows on class names

Details

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

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

https://github.com/clojure/clojure/blob/4ccb10edbe66eae81336a4c0972050d9759b0bf7/src/jvm/clojure/lang/Compiler.java#L3578

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

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
Andy Fingerhut made changes -
Patch Code and Test [ 10002 ]
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 ]
Stuart Halloway made changes -
Assignee Stuart Halloway [ stu ]
Stuart Halloway made changes -
Approval Vetted [ 10003 ] Screened [ 10004 ]
Description 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}
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

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

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}
Rich Hickey made changes -
Approval Screened [ 10004 ] Ok [ 10007 ]
Stuart Halloway made changes -
Resolution Completed [ 1 ]
Status Open [ 1 ] Closed [ 6 ]

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: