Clojure

missing field munging when recreating deftypes serialized into byte code

Details

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

Description

deftypes with fields whose names get munged fail when constructed in data reader functions.

user=> (deftype Foo [hello-world])
user.Foo
user=> (alter-var-root #'default-data-readers assoc 'foo (fn [x] (->Foo x)))
{inst #'clojure.instant/read-instant-date, uuid #'clojure.uuid/default-uuid-reader, foo #object[user$eval12$fn__13 0x23c89df9 "user$eval12$fn__13@23c89df9"]}
user=> #foo "1"
CompilerException java.lang.IllegalArgumentException: No matching field found: hello-world for class user.Foo, compiling:(NO_SOURCE_PATH:0:0)

Cause: To embed deftypes in the bytecode the compiler emits the value of each field, then emits a call to the deftypes underlying class's constructor. To get a list of fields the compiler calls .getBasis. The getBasis fields are the "clojure" level field names of the deftype, which the actual "jvm" level field names have been munged (replacing - with _, etc), so the compiler tries to generate code to set values on non-existent fields.

Approach: Munge the field name before emitting it in bytecode.
Patch: clj-1399-with-test.diff
Screened by: Alex Miller

  1. clj-1399.diff
    02/Apr/14 4:39 PM
    0.8 kB
    Kevin Downey
  2. clj-1399-with-test.diff
    29/Apr/15 1:20 PM
    1 kB
    Kevin Downey

Activity

Hide
Kevin Downey added a comment -

reproducing case

$ rlwrap java -server -Xmx1G -Xms1G -jar /Users/hiredman/src/clojure/target/clojure-1.6.0-master-SNAPSHOT.jar
Clojure 1.6.0-master-SNAPSHOT
user=> (deftype Foo [hello-world])
user.Foo
user=> (alter-var-root #'default-data-readers assoc 'foo (fn [x] (Foo. x)))
{foo #<user$eval6$fn__7 user$eval6$fn__7@2f953efd>, inst #'clojure.instant/read-instant-date, uuid #'clojure.uuid/default-uuid-reader}
user=> #foo "1"
CompilerException java.lang.IllegalArgumentException: No matching field found: hello-world for class user.Foo, compiling:(NO_SOURCE_PATH:0:0)
user=>
Show
Kevin Downey added a comment - reproducing case
$ rlwrap java -server -Xmx1G -Xms1G -jar /Users/hiredman/src/clojure/target/clojure-1.6.0-master-SNAPSHOT.jar
Clojure 1.6.0-master-SNAPSHOT
user=> (deftype Foo [hello-world])
user.Foo
user=> (alter-var-root #'default-data-readers assoc 'foo (fn [x] (Foo. x)))
{foo #<user$eval6$fn__7 user$eval6$fn__7@2f953efd>, inst #'clojure.instant/read-instant-date, uuid #'clojure.uuid/default-uuid-reader}
user=> #foo "1"
CompilerException java.lang.IllegalArgumentException: No matching field found: hello-world for class user.Foo, compiling:(NO_SOURCE_PATH:0:0)
user=>
Hide
Kevin Downey added a comment -

this patch fixes the issue on the latest master for me

Show
Kevin Downey added a comment - this patch fixes the issue on the latest master for me
Kevin Downey made changes -
Field Original Value New Value
Attachment clj-1399.diff [ 12912 ]
Hide
Chas Emerick added a comment -

FWIW, this was precipitated by real experience (I think I created the refheap paste). The workaround is easy (don't use dashes in field names of deftypes you want to return from data reader functions), but I wouldn't expect anyone to guess that that wasn't already oversensitized to munging edge cases.

Show
Chas Emerick added a comment - FWIW, this was precipitated by real experience (I think I created the refheap paste). The workaround is easy (don't use dashes in field names of deftypes you want to return from data reader functions), but I wouldn't expect anyone to guess that that wasn't already oversensitized to munging edge cases.
Kevin Downey made changes -
Patch Code [ 10001 ]
Alex Miller made changes -
Approval Triaged [ 10120 ]
Labels compiler deftype
Alex Miller made changes -
Priority Major [ 3 ] Critical [ 2 ]
Hide
Alex Miller added a comment -

Could the patch have a test?

Show
Alex Miller added a comment - Could the patch have a test?
Hide
Kevin Downey added a comment - - edited

clj-1399-with-test.diff adds a test

Show
Kevin Downey added a comment - - edited clj-1399-with-test.diff adds a test
Kevin Downey made changes -
Attachment clj-1399-with-test.diff [ 14089 ]
Alex Miller made changes -
Patch Code [ 10001 ] Code and Test [ 10002 ]
Description to embed deftypes in the bytecode the compiler emits the value of each field, then emits a call to the deftypes underlying class's constructor.

to get a list of fields the compiler calls .getBasis.

the getBasis fields are the "clojure" level field names of the deftype, which the actual "jvm" level field names have been munged (replacing - with _, etc), so the compiler tries to generate code to set values on non-existent fields

https://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/Compiler.java#L4579

https://www.refheap.com/70731

you can work around this by using field names that don't require munging. a solution might be just calling munge in the emission of field sets of ITypes
to embed deftypes in the bytecode the compiler emits the value of each field, then emits a call to the deftypes underlying class's constructor.

to get a list of fields the compiler calls .getBasis.

the getBasis fields are the "clojure" level field names of the deftype, which the actual "jvm" level field names have been munged (replacing - with _, etc), so the compiler tries to generate code to set values on non-existent fields

https://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/Compiler.java#L4579

https://www.refheap.com/70731

you can work around this by using field names that don't require munging. a solution might be just calling munge in the emission of field sets of ITypes

*Patch:* clj-1399-with-test.diff

*Screened by:* Alex Miller
Labels compiler deftype compiler deftype ft
Alex Miller made changes -
Description to embed deftypes in the bytecode the compiler emits the value of each field, then emits a call to the deftypes underlying class's constructor.

to get a list of fields the compiler calls .getBasis.

the getBasis fields are the "clojure" level field names of the deftype, which the actual "jvm" level field names have been munged (replacing - with _, etc), so the compiler tries to generate code to set values on non-existent fields

https://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/Compiler.java#L4579

https://www.refheap.com/70731

you can work around this by using field names that don't require munging. a solution might be just calling munge in the emission of field sets of ITypes

*Patch:* clj-1399-with-test.diff

*Screened by:* Alex Miller
deftypes with fields whose names get munged fail when constructed in data reader functions.

{code}
user=> (deftype Foo [hello-world])
user.Foo
user=> (alter-var-root #'default-data-readers assoc 'foo (fn [x] (->Foo x)))
{inst #'clojure.instant/read-instant-date, uuid #'clojure.uuid/default-uuid-reader, foo #object[user$eval12$fn__13 0x23c89df9 "user$eval12$fn__13@23c89df9"]}
user=> #foo "1"
CompilerException java.lang.IllegalArgumentException: No matching field found: hello-world for class user.Foo, compiling:(NO_SOURCE_PATH:0:0)
{code}

*Cause:* To embed deftypes in the bytecode the compiler emits the value of each field, then emits a call to the deftypes underlying class's constructor. To get a list of fields the compiler calls .getBasis. The getBasis fields are the "clojure" level field names of the deftype, which the actual "jvm" level field names have been munged (replacing - with _, etc), so the compiler tries to generate code to set values on non-existent fields.

*Approach:* Munge the field name before emitting it in bytecode.
*Patch:* clj-1399-with-test.diff
*Screened by:* Alex Miller
Summary missing field munging when recreating deftypes serialized in to byte code missing field munging when recreating deftypes serialized into byte code
Rich Hickey made changes -
Approval Triaged [ 10120 ] Vetted [ 10003 ]
Affects Version/s Release 1.7 [ 10250 ]
Fix Version/s Release 1.8 [ 10254 ]
Alex Miller made changes -
Approval Vetted [ 10003 ] Screened [ 10004 ]
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 (4)

Dates

  • Created:
    Updated:
    Resolved: