[CLJ-1065] Allow duplicate set elements and map keys for all set and map constructors Created: 08/Sep/12 Updated: 04/Oct/12 Resolved: 04/Oct/12
|Affects Version/s:||Release 1.4|
|Fix Version/s:||Release 1.5|
|Patch:||Code and Test|
See description here http://dev.clojure.org/display/design/Allow+duplicate+map+keys+and+set+elements
|Comment by Andy Fingerhut [ 08/Sep/12 1:19 AM ]|
I think that clj-1065-set-map-constructors-allow-duplicates-v1.txt dated Sep 7 2012 updates Clojure for the behavior Rich recommends on the dev Wiki page in the description. I may have missed something, though. Perhaps the most questionable part is the way I've chosen to implement array-map always taking the last key's value. It is no less efficient than the PersistentArrayMap createWithCheck method, i.e. O(n^2).
|Comment by Rich Hickey [ 08/Sep/12 7:10 AM ]|
array-map is ok. I would prefer that the docs strings say "as if by repeated assoc" (or conj for sets). "eliminates" and "last" are less precise e.g. what if you pass equal things, but with different metadata, to hash-set? "Eliminates dupes" doesn't say.
|Comment by Andy Fingerhut [ 08/Sep/12 3:39 PM ]|
clj-1065-set-map-constructors-allow-duplicates-v2.txt dated Sep 8 2012 supersedes yesterday's -v1.txt patch, which I will remove.
This one updates doc strings per Rich's suggestion.
Also, his mention of metadata and "as if by assoc" led me to beef up the new test cases to check that metadata is correct, and I thus found that my new array-map constructor was not doing the right thing. This one does.
There is still no implementation of Rich's #3 yet. Just wanted to get this one up there in case someone else builds on it before I do.
|Comment by Andy Fingerhut [ 09/Sep/12 3:07 AM ]|
Patch clj-1065-do-map-constant-duplicate-key-checks-compile-time-only-v1.txt only makes changes that should eliminate run-time checks for duplicate map keys, if they are compile-time constants.
It is currently separate from the changes to those for the set/map constructor functions, since I am less sure about these changes. I'm not terribly familiar with the compiler internals, and I might be going about this the wrong way. Better to get separate feedback on these changes alone. I'm happy to merge them all into one patch if both parts look good.