[CLJS-461] repeated assoc into map eventually drops meta data Created: 22/Jan/13 Updated: 27/Jan/13 Resolved: 27/Jan/13 |
|
| Status: | Resolved |
| Project: | ClojureScript |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Defect | Priority: | Major |
| Reporter: | Michael van Acken | Assignee: | Michał Marczyk |
| Resolution: | Completed | Votes: | 0 |
| Labels: | None | ||
| Environment: |
[org.clojure/clojurescript "0.0-1552"] |
||
| Attachments: |
|
| Description |
|
When running this loop, then the map's meta is lost after 32 assocs: (loop [i 0, m (with-meta {} {:foo :bar})]
(when (<= i 34)
(.log js/console i (pr-str (meta m)))
(recur (inc i) (assoc m (str i) i))))
The last 4 lines of output read 31 {:foo :bar}
32 {:foo :bar}
33 nil
34 nil
cljs.core.ObjMap/HASHMAP_THRESHOLD happens to be 32, so I guess |
| Comments |
| Comment by Michał Marczyk [ 24/Jan/13 12:51 AM ] |
|
obj-map->hash-map failed to take into account the fact that transients don't support metadata. The attached patch fixes this. |
| Comment by Michał Marczyk [ 25/Jan/13 6:11 AM ] |
|
Just realized that this also happens with PAMs. New patch addresses the issue in both places. (Also tweaks the PAM->PHM conversion – no more transient creation for a single assoc.) |
| Comment by David Nolen [ 26/Jan/13 11:06 AM ] |
|
I tried the patch and it did not seem to resolve the case above. Could we get a patch that includes a test case? Thanks! |
| Comment by Michał Marczyk [ 26/Jan/13 10:13 PM ] |
|
I tried it again at a Rhino REPL and for me it resolves both problems (OM and PAM). New patch with test cases based on the form from the ticket attached. (The tests do fail on master.) |
| Comment by Michał Marczyk [ 26/Jan/13 10:20 PM ] |
|
Tweaked patch with ticket number mentioned in a comment above the new test cases. |
| Comment by David Nolen [ 27/Jan/13 5:31 PM ] |
|
fixed, http://github.com/clojure/clojurescript/commit/3ed16f32c6532d30eccd76ed41f90c956051ac58 |