<< Back to previous view

[CLJ-837] java.lang.VerifyError when compiling deftype or defrecord with argument name starting with double underscore characters. Created: 11/Sep/11  Updated: 09/Dec/11  Resolved: 09/Dec/11

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

Type: Defect Priority: Minor
Reporter: Tchavdar Roussanov Assignee: Fogus
Resolution: Completed Votes: 1
Labels: None
Environment:

clojure-1.3.0-beta3 and git master, JDK 1.6.0_26, OS X 1.7.1


Attachments: Text File 0837-fixed.patch     File CLJ-837-double-underscore-type-records.diff    
Patch: Code and Test
Approval: Ok
Waiting On: Rich Hickey

 Description   

The minimal test to trigger the verification error is :

(deftype t [__])
or
(defrecord r [__])

It works fine in clojure 1.2, 1.2.1

Works fine with one underscore.

Output from REPL:

user=> (defrecord r [__])
CompilerException java.lang.VerifyError: (class: user/r, method: create signature: (Lclojure/lang/IPersistentMap;)Luser/r Expecting to find object/array on stack, compiling:(NO_SOURCE_PATH:2)

Stack trace from slime session:

(class: clojure/data/priority_map/r, method: create signature: (Lclojure/lang/IPersistentMap;)Lclojure/data/priority_map/r Expecting to find object/array on stack
[Thrown class java.lang.VerifyError]

Restarts:
0: [QUIT] Quit to the SLIME top level

Backtrace:
0: java.lang.Class.forName0(Native Method)
1: java.lang.Class.forName(Class.java:247)
2: clojure.lang.RT.classForName(RT.java:2013)
3: clojure.lang.Compiler$HostExpr.maybeClass(Compiler.java:929)
4: clojure.lang.Compiler$HostExpr.access$400(Compiler.java:710)
5: clojure.lang.Compiler$NewExpr$Parser.parse(Compiler.java:2391)
6: clojure.lang.Compiler.analyzeSeq(Compiler.java:6368)
7: clojure.lang.Compiler.analyze(Compiler.java:6175)
8: clojure.lang.Compiler.analyze(Compiler.java:6136)
9: clojure.lang.Compiler$BodyExpr$Parser.parse(Compiler.java:5531)
10: clojure.lang.Compiler$FnMethod.parse(Compiler.java:4967)
[No Locals]
11: clojure.lang.Compiler$FnExpr.parse(Compiler.java:3588)
12: clojure.lang.Compiler.analyzeSeq(Compiler.java:6366)
13: clojure.lang.Compiler.analyze(Compiler.java:6175)
14: clojure.lang.Compiler.analyzeSeq(Compiler.java:6356)
15: clojure.lang.Compiler.analyze(Compiler.java:6175)
16: clojure.lang.Compiler.access$100(Compiler.java:37)
17: clojure.lang.Compiler$DefExpr$Parser.parse(Compiler.java:492)
18: clojure.lang.Compiler.analyzeSeq(Compiler.java:6368)
19: clojure.lang.Compiler.analyze(Compiler.java:6175)
20: clojure.lang.Compiler.analyzeSeq(Compiler.java:6356)
21: clojure.lang.Compiler.analyze(Compiler.java:6175)
22: clojure.lang.Compiler.analyze(Compiler.java:6136)
23: clojure.lang.Compiler$BodyExpr$Parser.parse(Compiler.java:5531)
24: clojure.lang.Compiler$LetExpr$Parser.parse(Compiler.java:5832)
25: clojure.lang.Compiler.analyzeSeq(Compiler.java:6368)
26: clojure.lang.Compiler.analyze(Compiler.java:6175)
27: clojure.lang.Compiler.analyze(Compiler.java:6136)
28: clojure.lang.Compiler$BodyExpr$Parser.parse(Compiler.java:5531)
29: clojure.lang.Compiler$FnMethod.parse(Compiler.java:4967)
30: clojure.lang.Compiler$FnExpr.parse(Compiler.java:3588)
31: clojure.lang.Compiler.analyzeSeq(Compiler.java:6366)
32: clojure.lang.Compiler.analyze(Compiler.java:6175)
33: clojure.lang.Compiler.eval(Compiler.java:6421)
34: clojure.lang.Compiler.eval(Compiler.java:6390)
35: clojure.core$eval.invoke(core.clj:2795)
36: swank.commands.basic$eval_region.invoke(basic.clj:47)
37: swank.commands.basic$eval_region.invoke(basic.clj:37)
38: swank.commands.basic$eval764$listener_eval__765.invoke(basic.clj:71)
39: clojure.lang.Var.invoke(Var.java:401)
40: clojure.data.priority_map$eval2759.invoke(NO_SOURCE_FILE)
41: clojure.lang.Compiler.eval(Compiler.java:6424)
42: clojure.lang.Compiler.eval(Compiler.java:6390)
43: clojure.core$eval.invoke(core.clj:2795)
44: swank.core$eval_in_emacs_package.invoke(core.clj:92)
45: swank.core$eval_for_emacs.invoke(core.clj:239)
46: clojure.lang.Var.invoke(Var.java:409)
47: clojure.lang.AFn.applyToHelper(AFn.java:167)
48: clojure.lang.Var.applyTo(Var.java:518)
49: clojure.core$apply.invoke(core.clj:600)
50: swank.core$eval_from_control.invoke(core.clj:99)
[No Locals]
51: swank.core$eval_loop.invoke(core.clj:104)
52: swank.core$spawn_repl_thread$fn_529$fn_530.invoke(core.clj:309)
53: clojure.lang.AFn.applyToHelper(AFn.java:159)
54: clojure.lang.AFn.applyTo(AFn.java:151)
55: clojure.core$apply.invoke(core.clj:600)
56: swank.core$spawn_repl_thread$fn__529.doInvoke(core.clj:306)
57: clojure.lang.RestFn.invoke(RestFn.java:397)
58: clojure.lang.AFn.run(AFn.java:24)
59: java.lang.Thread.run(Thread.java:680)



 Comments   
Comment by Fogus [ 11/Sep/11 9:18 PM ]

The privileged names __meta and __extmap are implementation details of types and records and should probably not be viewed as real. In other words, since they are not published as part of the type/record docs, then they may not exist in the future. Having said that, the current Clojure implementation treats any name starting with __ as a special field and doesn't count them toward the field count. As a result, when emitting the MyType.create method (another implementation detail), the fields marked as __ throw off the field count and as a result the bytecode is not emitted properly. I know how to change the code to allow names prefixed with __ and fixing the validation error (not called by those two special names listed above), but I hesitate to say that is the solution
without further reflection.

Comment by Fogus [ 12/Sep/11 8:32 AM ]

I think the prudent fix is as follows:

  • Allow any legal name except those in the set #{__meta __extmap}
  • Fix the #create emission
  • Documentation should be changed to reflect that the aforementioned set is disallowed

Thankfully this is an easy fix. However, the "magic" of __ prefixed names will go away. That is, the following behavior will differ from v1.2:

(defrecord R [__a ___b])
(:__a (R. 42 9))
;=> 42

(deftype T [__a])
(.__a (T. 36))
;=> 36

(deftype T2 [a b __meta])
;; AssertionError

Any thoughts?

Comment by Fogus [ 12/Sep/11 9:44 AM ]

If anyone is interested, the changes reflected in the previous comment can be found on my own branch: https://github.com/fogus/clojure/tree/CLJ-837 Code-review would be nice.

Comment by Rich Hickey [ 12/Sep/11 10:09 AM ]

differing behavior of undocumented features is a non-problem

Comment by Fogus [ 12/Sep/11 12:05 PM ]

Got it. Thanks Rich.

Comment by Fogus [ 12/Sep/11 12:25 PM ]

Added patch file.

Comment by Steve Miner [ 12/Sep/11 2:11 PM ]

I suggest that the documentation should simply say that all fields beginning with double-underscore (__) are reserved. You don't have to enforce that (as the patch allows any field name except __meta and __extmap), but it's better to be conservative with the documentation so that you have some room to innovate.

Comment by Steve Miner [ 12/Sep/11 2:13 PM ]

Just by inspection, it looks like patch has an extra space before the 'b' in the line near the end:

+ 2 (:___ b r)

Comment by Stuart Halloway [ 02/Dec/11 1:53 PM ]

0837-fixed is same as original patch, but with fixed unit test.

Generated at Wed Apr 23 10:33:31 CDT 2014 using JIRA 4.4#649-r158309.