Activity

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).

Andy Fingerhut
added a comment - 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).

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.

Rich Hickey
added a comment - 08/Sep/12 7:10 AM Thanks!
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.

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.

Andy Fingerhut
added a comment - 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.

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.

Andy Fingerhut
added a comment - 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.