Details
-
Type:
Defect
-
Status:
Closed
-
Priority:
Minor
-
Resolution: Completed
-
Affects Version/s: None
-
Fix Version/s: None
-
Component/s: None
-
Labels:
-
Environment:Happens on cljsfiddle, among other environments.
Description
In commit b8681e8 the implementation is
ICollection
(-conj [coll entry]
(if (vector? entry)
(-assoc coll (-nth entry 0) (-nth entry 1))
(reduce -conj coll entry)))
Thus, e.g., (-conj {} "foo") results in an infinite recursion, and a stack overflow. This causes things like (merge {} "foo") to fail for the same reason.
Not sure what the purpose of the not-vector branch could be. I can't think of a situation where it would give a useful result. Maybe it could throw a more helpful error message.
This actually applies to all three map types. (In fact, in (-conj {} "foo"), the map is an array map.) In Clojure, conj on a map works with a number of argument types:
1. map entries;
2. two-element vectors;
3. seqables of map entries.
The final case is, perhaps surprisingly, the oldest one. Merging maps falls under it, since for map arguments it boils down to merge minus special treatment of nil (merge uses conj to merge pairs of maps); but arbitrary seqables of map entries are supported. (NB. these must be actual map entries, not two-element vectors!) This allows one, for example, to filter a map and conj the result of that into another map.
So, we want to support the legitimate use cases while maybe complaining about code that wouldn't work in Clojure if it's not too much of a problem performance-wise. An example of a call that we'd probably like to throw: {{(conj {} (list (list [:foo 1])))}}.
The attached patch makes the -conj implementations in all the map types use an explicit loop in the non-vector branch and adds some test for the resulting behaviour.