Clojure

NPE when AOTing overrided clojure.core functions

Details

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

Description

When performing AOT compilation on a namespace that overrides a clojure.core function without excluding the original clojure.core function from the ns, you get a NullPointerException.

To reproduce aot compile a namespace like "(ns x) (defn get [])"

For example:

$ lein new aot-get
$ cd aot-get
$ sed -i s/foo/get/
$ lein compile :all
WARNING: get already refers to: #'clojure.core/get in namespace: aot-get.core, being replaced by: #'aot-get.core/get
Exception in thread "main" java.lang.NullPointerException
	at clojure.lang.Compiler$ObjExpr.emitVar(Compiler.java:4858)
	at clojure.lang.Compiler$DefExpr.emit(Compiler.java:428)
	at clojure.lang.Compiler.compile1(Compiler.java:7152)
	at clojure.lang.Compiler.compile(Compiler.java:7219)
	at clojure.lang.RT.compile(RT.java:398)
	at clojure.lang.RT.load(RT.java:438)
	at clojure.lang.RT.load(RT.java:411)

Cause: DefExpr.parse does not call registerVar for vars overridding clojure.core ones, thus when AOT compiling the var is not registered in the constant table.

Proposed: The attached patch makes DefExpr.parse call registerVar for vars overridding clojure.core ones.

Patch: 0001-fix-CLJ-1241.patch

Screened by: Alex Miller

Activity

Phil Hagelberg made changes -
Field Original Value New Value
Description When performing AOT compilation on a namespace that defines a `get` function without `(:refer-clojure :exclude [get])`, you get a null-pointer exception. To reproduce:

```
$ lein new aot-get
$ cd aot-get
$ sed -i s/foo/get/
$ lein compile :all
```
When performing AOT compilation on a namespace that defines a `get` function without `(:refer-clojure :exclude [get])`, you get a null-pointer exception. To reproduce:

$ lein new aot-get
$ cd aot-get
$ sed -i s/foo/get/
$ lein compile :all
WARNING: get already refers to: #'clojure.core/get in namespace: aot-get.core, being replaced by: #'aot-get.core/get
Exception in thread "main" java.lang.NullPointerException
at clojure.lang.Compiler$ObjExpr.emitVar(Compiler.java:4858)
at clojure.lang.Compiler$DefExpr.emit(Compiler.java:428)
at clojure.lang.Compiler.compile1(Compiler.java:7152)
at clojure.lang.Compiler.compile(Compiler.java:7219)
at clojure.lang.RT.compile(RT.java:398)
at clojure.lang.RT.load(RT.java:438)
at clojure.lang.RT.load(RT.java:411)
[...]
Hide
Nicola Mometto added a comment -

DefExpr.parse was not calling registerVar for vars overridding clojure.core ones.

Show
Nicola Mometto added a comment - DefExpr.parse was not calling registerVar for vars overridding clojure.core ones.
Nicola Mometto made changes -
Attachment 0001-fix-CLJ-1241.patch [ 12082 ]
Nicola Mometto made changes -
Patch Code [ 10001 ]
Alex Miller made changes -
Approval Triaged [ 10120 ]
Description When performing AOT compilation on a namespace that defines a `get` function without `(:refer-clojure :exclude [get])`, you get a null-pointer exception. To reproduce:

$ lein new aot-get
$ cd aot-get
$ sed -i s/foo/get/
$ lein compile :all
WARNING: get already refers to: #'clojure.core/get in namespace: aot-get.core, being replaced by: #'aot-get.core/get
Exception in thread "main" java.lang.NullPointerException
at clojure.lang.Compiler$ObjExpr.emitVar(Compiler.java:4858)
at clojure.lang.Compiler$DefExpr.emit(Compiler.java:428)
at clojure.lang.Compiler.compile1(Compiler.java:7152)
at clojure.lang.Compiler.compile(Compiler.java:7219)
at clojure.lang.RT.compile(RT.java:398)
at clojure.lang.RT.load(RT.java:438)
at clojure.lang.RT.load(RT.java:411)
[...]
When performing AOT compilation on a namespace that defines a `get` function without `(:refer-clojure :exclude [get])`, you get a NullPointerException. To reproduce:

$ lein new aot-get
$ cd aot-get
$ sed -i s/foo/get/
$ lein compile :all
WARNING: get already refers to: #'clojure.core/get in namespace: aot-get.core, being replaced by: #'aot-get.core/get
Exception in thread "main" java.lang.NullPointerException
at clojure.lang.Compiler$ObjExpr.emitVar(Compiler.java:4858)
at clojure.lang.Compiler$DefExpr.emit(Compiler.java:428)
at clojure.lang.Compiler.compile1(Compiler.java:7152)
at clojure.lang.Compiler.compile(Compiler.java:7219)
at clojure.lang.RT.compile(RT.java:398)
at clojure.lang.RT.load(RT.java:438)
at clojure.lang.RT.load(RT.java:411)
[...]
Priority Trivial [ 5 ] Minor [ 4 ]
Hide
Alex Miller added a comment -

Verified on Clojure 1.5.1.

Show
Alex Miller added a comment - Verified on Clojure 1.5.1.
Alex Miller made changes -
Affects Version/s Release 1.5 [ 10150 ]
Hide
Javier Neira Sanchez added a comment -

Reproduced with `key` function without `(:refer-clojure :exclude [key])`

Show
Javier Neira Sanchez added a comment - Reproduced with `key` function without `(:refer-clojure :exclude [key])`
Alex Miller made changes -
Labels aot
Hide
Rich Hickey added a comment -

This doesn't meet triage guidelines - i.e. there is this problem, therefore we will fix it by _____ so it then does _____

Show
Rich Hickey added a comment - This doesn't meet triage guidelines - i.e. there is this problem, therefore we will fix it by _____ so it then does _____
Rich Hickey made changes -
Approval Triaged [ 10120 ]
Nicola Mometto made changes -
Labels aot aot compiler
Nicola Mometto made changes -
Description When performing AOT compilation on a namespace that defines a `get` function without `(:refer-clojure :exclude [get])`, you get a NullPointerException. To reproduce:

$ lein new aot-get
$ cd aot-get
$ sed -i s/foo/get/
$ lein compile :all
WARNING: get already refers to: #'clojure.core/get in namespace: aot-get.core, being replaced by: #'aot-get.core/get
Exception in thread "main" java.lang.NullPointerException
at clojure.lang.Compiler$ObjExpr.emitVar(Compiler.java:4858)
at clojure.lang.Compiler$DefExpr.emit(Compiler.java:428)
at clojure.lang.Compiler.compile1(Compiler.java:7152)
at clojure.lang.Compiler.compile(Compiler.java:7219)
at clojure.lang.RT.compile(RT.java:398)
at clojure.lang.RT.load(RT.java:438)
at clojure.lang.RT.load(RT.java:411)
[...]
When performing AOT compilation on a namespace that overrides a clojure.core function without excluding the original clojure.core function from the ns, you get a NullPointerException. To reproduce:

$ lein new aot-get
$ cd aot-get
$ sed -i s/foo/get/
$ lein compile :all
WARNING: get already refers to: #'clojure.core/get in namespace: aot-get.core, being replaced by: #'aot-get.core/get
Exception in thread "main" java.lang.NullPointerException
at clojure.lang.Compiler$ObjExpr.emitVar(Compiler.java:4858)
at clojure.lang.Compiler$DefExpr.emit(Compiler.java:428)
at clojure.lang.Compiler.compile1(Compiler.java:7152)
at clojure.lang.Compiler.compile(Compiler.java:7219)
at clojure.lang.RT.compile(RT.java:398)
at clojure.lang.RT.load(RT.java:438)
at clojure.lang.RT.load(RT.java:411)
[...]

This is caused because DefExpr.parse does not call registerVar for vars overridding clojure.core ones, thus when AOT compiling the var is not registered in the constant table.

The attached patch makes DefExpr.parse call registerVar for vars overridding clojure.core ones.
Summary NPE when AOTing overrided clojure.core/get NPE when AOTing overrided clojure.core functions
Aaron Cohen made changes -
Affects Version/s Release 1.6 [ 10157 ]
Hide
Aaron Cohen added a comment -

This is still present in the 1.6 release. I think it's mis-classified as low priority.

Show
Aaron Cohen added a comment - This is still present in the 1.6 release. I think it's mis-classified as low priority.
Hide
Aaron Cohen added a comment -

See for instance the cascalog mailing list: https://groups.google.com/forum/#!topic/cascalog-user/Pe5QIpmU0vA

Show
Aaron Cohen added a comment - See for instance the cascalog mailing list: https://groups.google.com/forum/#!topic/cascalog-user/Pe5QIpmU0vA
Hide
Andy Fingerhut added a comment - - edited

It may help if someone could clarify Rich's comment.

Does it mean that the ticket should include a plan of the form "therefore we will fix it by _____ so it then does _____", but this ticket doesn't have that?

Or perhaps it means that the ticket should not include a plan of that form, but this ticket does? If so, I don't see it, except perhaps the very last sentence of the description. If that is a problem for vetting a ticket, perhaps we could just delete that sentence and proceed from there?

Something else?

Show
Andy Fingerhut added a comment - - edited It may help if someone could clarify Rich's comment. Does it mean that the ticket should include a plan of the form "therefore we will fix it by _____ so it then does _____", but this ticket doesn't have that? Or perhaps it means that the ticket should not include a plan of that form, but this ticket does? If so, I don't see it, except perhaps the very last sentence of the description. If that is a problem for vetting a ticket, perhaps we could just delete that sentence and proceed from there? Something else?
Hide
Nicola Mometto added a comment -

Andy, I added the two last lines in the description after reading Rich's comment to explain why this bug happens and how the patch I attached works around this.

I don't know if this is what he was asking for though.

Show
Nicola Mometto added a comment - Andy, I added the two last lines in the description after reading Rich's comment to explain why this bug happens and how the patch I attached works around this. I don't know if this is what he was asking for though.
Hide
Alex Miller added a comment -

I think Rich meant that a ticket should have a plan of that form but does not. My own take on "triaged" is that it should state actual and expected results demonstrating a problem - I don't think it needs to actually describe the solution (as that can happen later in development). It is entirely possible that Rich and I differ in our interpretation of that. I will see if I can rework the description a bit to match what I've been doing elsewhere.

Show
Alex Miller added a comment - I think Rich meant that a ticket should have a plan of that form but does not. My own take on "triaged" is that it should state actual and expected results demonstrating a problem - I don't think it needs to actually describe the solution (as that can happen later in development). It is entirely possible that Rich and I differ in our interpretation of that. I will see if I can rework the description a bit to match what I've been doing elsewhere.
Alex Miller made changes -
Approval Triaged [ 10120 ]
Description When performing AOT compilation on a namespace that overrides a clojure.core function without excluding the original clojure.core function from the ns, you get a NullPointerException. To reproduce:

$ lein new aot-get
$ cd aot-get
$ sed -i s/foo/get/
$ lein compile :all
WARNING: get already refers to: #'clojure.core/get in namespace: aot-get.core, being replaced by: #'aot-get.core/get
Exception in thread "main" java.lang.NullPointerException
at clojure.lang.Compiler$ObjExpr.emitVar(Compiler.java:4858)
at clojure.lang.Compiler$DefExpr.emit(Compiler.java:428)
at clojure.lang.Compiler.compile1(Compiler.java:7152)
at clojure.lang.Compiler.compile(Compiler.java:7219)
at clojure.lang.RT.compile(RT.java:398)
at clojure.lang.RT.load(RT.java:438)
at clojure.lang.RT.load(RT.java:411)
[...]

This is caused because DefExpr.parse does not call registerVar for vars overridding clojure.core ones, thus when AOT compiling the var is not registered in the constant table.

The attached patch makes DefExpr.parse call registerVar for vars overridding clojure.core ones.
When performing AOT compilation on a namespace that overrides a clojure.core function without excluding the original clojure.core function from the ns, you get a NullPointerException.

To reproduce:

{code}
$ lein new aot-get
$ cd aot-get
$ sed -i s/foo/get/
$ lein compile :all
WARNING: get already refers to: #'clojure.core/get in namespace: aot-get.core, being replaced by: #'aot-get.core/get
Exception in thread "main" java.lang.NullPointerException
at clojure.lang.Compiler$ObjExpr.emitVar(Compiler.java:4858)
at clojure.lang.Compiler$DefExpr.emit(Compiler.java:428)
at clojure.lang.Compiler.compile1(Compiler.java:7152)
at clojure.lang.Compiler.compile(Compiler.java:7219)
at clojure.lang.RT.compile(RT.java:398)
at clojure.lang.RT.load(RT.java:438)
at clojure.lang.RT.load(RT.java:411)
{code}

*Cause:* DefExpr.parse does not call registerVar for vars overridding clojure.core ones, thus when AOT compiling the var is not registered in the constant table.

*Proposed:* The attached patch makes DefExpr.parse call registerVar for vars overridding clojure.core ones.

*Patch:* 0001-fix-CLJ-1241.patch
Hide
Andy Fingerhut added a comment - - edited

Alex, I have looked through the existing wiki pages on the ticket tracking process, and do not recall seeing anything about this desired aspect of a triaged ticket. Is it already documented somewhere and I missed it? Not that it has to be documented, necessarily, but Rich saying "triage guidelines" makes it sound like a filter he applies that ticket creators and screeners maybe should know about.

Show
Andy Fingerhut added a comment - - edited Alex, I have looked through the existing wiki pages on the ticket tracking process, and do not recall seeing anything about this desired aspect of a triaged ticket. Is it already documented somewhere and I missed it? Not that it has to be documented, necessarily, but Rich saying "triage guidelines" makes it sound like a filter he applies that ticket creators and screeners maybe should know about.
Hide
Alex Miller added a comment -

To me, Triage (and Vetting) is all about having good problem statements. For a defect, it is most important to demonstrate the problem (what happens now) and what you expect to happen instead. I do not usually expect there to necessarily be "by ____" in the ticket - to me that is part of working through the solution (although it is typical to have this in an enhancement). This ticket, as it stands now, seems to have both a good problem statement and a good cause/solution statement so seems to exceed Triaging standards afaik.

Two places where I have tried to write about these things in the past are http://dev.clojure.org/display/community/Creating+Tickets and in the Triage process on the workflow page http://dev.clojure.org/display/community/JIRA+workflow.

Show
Alex Miller added a comment - To me, Triage (and Vetting) is all about having good problem statements. For a defect, it is most important to demonstrate the problem (what happens now) and what you expect to happen instead. I do not usually expect there to necessarily be "by ____" in the ticket - to me that is part of working through the solution (although it is typical to have this in an enhancement). This ticket, as it stands now, seems to have both a good problem statement and a good cause/solution statement so seems to exceed Triaging standards afaik. Two places where I have tried to write about these things in the past are http://dev.clojure.org/display/community/Creating+Tickets and in the Triage process on the workflow page http://dev.clojure.org/display/community/JIRA+workflow.
Rich Hickey made changes -
Priority Minor [ 4 ] Major [ 3 ]
Rich Hickey made changes -
Fix Version/s Release 1.7 [ 10250 ]
Approval Triaged [ 10120 ] Vetted [ 10003 ]
Alex Miller made changes -
Approval Vetted [ 10003 ] Screened [ 10004 ]
Description When performing AOT compilation on a namespace that overrides a clojure.core function without excluding the original clojure.core function from the ns, you get a NullPointerException.

To reproduce:

{code}
$ lein new aot-get
$ cd aot-get
$ sed -i s/foo/get/
$ lein compile :all
WARNING: get already refers to: #'clojure.core/get in namespace: aot-get.core, being replaced by: #'aot-get.core/get
Exception in thread "main" java.lang.NullPointerException
at clojure.lang.Compiler$ObjExpr.emitVar(Compiler.java:4858)
at clojure.lang.Compiler$DefExpr.emit(Compiler.java:428)
at clojure.lang.Compiler.compile1(Compiler.java:7152)
at clojure.lang.Compiler.compile(Compiler.java:7219)
at clojure.lang.RT.compile(RT.java:398)
at clojure.lang.RT.load(RT.java:438)
at clojure.lang.RT.load(RT.java:411)
{code}

*Cause:* DefExpr.parse does not call registerVar for vars overridding clojure.core ones, thus when AOT compiling the var is not registered in the constant table.

*Proposed:* The attached patch makes DefExpr.parse call registerVar for vars overridding clojure.core ones.

*Patch:* 0001-fix-CLJ-1241.patch
When performing AOT compilation on a namespace that overrides a clojure.core function without excluding the original clojure.core function from the ns, you get a NullPointerException.

To reproduce aot compile a namespace like "(ns x) (defn get [])"

For example:
{code}
$ lein new aot-get
$ cd aot-get
$ sed -i s/foo/get/
$ lein compile :all
WARNING: get already refers to: #'clojure.core/get in namespace: aot-get.core, being replaced by: #'aot-get.core/get
Exception in thread "main" java.lang.NullPointerException
at clojure.lang.Compiler$ObjExpr.emitVar(Compiler.java:4858)
at clojure.lang.Compiler$DefExpr.emit(Compiler.java:428)
at clojure.lang.Compiler.compile1(Compiler.java:7152)
at clojure.lang.Compiler.compile(Compiler.java:7219)
at clojure.lang.RT.compile(RT.java:398)
at clojure.lang.RT.load(RT.java:438)
at clojure.lang.RT.load(RT.java:411)
{code}

*Cause:* DefExpr.parse does not call registerVar for vars overridding clojure.core ones, thus when AOT compiling the var is not registered in the constant table.

*Proposed:* The attached patch makes DefExpr.parse call registerVar for vars overridding clojure.core ones.

*Patch:* 0001-fix-CLJ-1241.patch

*Screened by:* Alex Miller
Rich Hickey made changes -
Approval Screened [ 10004 ] Ok [ 10007 ]
Stuart Halloway made changes -
Resolution Completed [ 1 ]
Status Open [ 1 ] Closed [ 6 ]

People

Vote (2)
Watch (6)

Dates

  • Created:
    Updated:
    Resolved: