Allow duplicate set elements and map keys for all set and map constructors

Environment

None

Attachments

2
  • 09 Sep 2012, 09:07 AM
  • 08 Sep 2012, 09:39 PM

Activity

Show:

Andy FingerhutSeptember 9, 2012 at 9: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.

Andy FingerhutSeptember 8, 2012 at 9: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.

Rich HickeySeptember 8, 2012 at 1:10 PM

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.

Andy FingerhutSeptember 8, 2012 at 7: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).

Completed

Details

Assignee

Reporter

Approval

Ok

Patch

Code and Test

Priority

Affects versions

Fix versions

Created September 8, 2012 at 7:15 AM
Updated October 4, 2012 at 4:15 PM
Resolved October 4, 2012 at 4:15 PM

Flag notifications