arcadia-unity / Arcadia

Clojure in Unity
http://arcadia-unity.github.io/
Apache License 2.0
1.68k stars 108 forks source link

edn serialization of arrays seems broken #305

Open timsgardner opened 5 years ago

timsgardner commented 5 years ago

With *print-dup* set to false, arrays serialize as lists:

user=> (binding [*print-dup* false] (pr-str (into-array Object (list 1 2 3))))
"(1 2 3)"

With *print-dup* set to true, arrays serialize as follows:

user=> (binding [*print-dup* true] (pr-str (into-array Object (list 1 2 3))))
"#=(System.Object[]. [1 2 3])"

This doesn't survive edn/read-string:

user=> (clojure.edn/read-string (binding [*print-dup* true] (pr-str (into-array Object (list 1 2 3)))))
InvalidOperationException No dispatch macro for: =
clojure.lang.EdnReader+DispatchReader.Read (:0)
clojure.lang.EdnReader+ReaderBase.invoke (:0)
clojure.lang.EdnReader.read (:0)
clojure.lang.EdnReader.read (:0)
clojure.lang.EdnReader.readString (:0)
clojure/edn$read_string__19066.invokeStatic (:0)
clojure/edn$read_string__19066.invoke (:0)
clojure/edn$read_string__19066.invokeStatic (:0)
clojure/edn$read_string__19066.invoke (:0)
user$eval__14222__14235.invokeStatic (:0)
user$eval__14222__14235.invoke (:0)
clojure.lang.Compiler.eval (:0)
clojure.lang.Compiler.eval (:0)
clojure/core$eval__3112.invokeStatic (:0)
clojure/core$eval__3112.invoke (:0)
arcadia/socket_repl$game_thread_evalfn__14050fn__14055__14067.invoke (:0)
nasser commented 5 years ago

On ClojureJVM

user=> (binding [*print-dup* false] (pr-str (into-array Object (list 1 2 3))))
"#object[\"[Ljava.lang.Object;\" 0x42954c45 \"[Ljava.lang.Object;@42954c45\"]"
user=> (binding [*print-dup* true] (pr-str (into-array Object (list 1 2 3))))

IllegalArgumentException No method in multimethod 'print-dup' for dispatch value: class [Ljava.lang.Object;  clojure.lang.MultiFn.getFn (MultiFn.java:156)
timsgardner commented 5 years ago

On the language level, the bug seems to be that (binding [*print-dup* true] (pr-str (into-array Object (list 1 2 3)))) emits anything at all rather than an error. Alternatively, the bug is that ClojureJVM doesn't print arrays readably (and should), and ClojureCLR's version doesn't work (this representation doesn't work with clojure.core/read-string either).

Whatever the case, we want to be able to serialize arrays. If we're not expressing this as a language patch we have to express it as an intervention on *print-method* and *data-readers*. If we do this globally we're effectively patching the language at runtime for our own little project, which isn't great. If we do this locally, we have multiple edn serialization techniques drifting about in the same runtime, which is inherently a bit sketchy.

We're probably going to do the latter (we probably have to anyway). At the very least we should document the dickens out of this because it will lead to great confusion and weird user bugs.