Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HashSet (and maybe Map?) optimizations backported to 2.12.x don't respect possiblity of custom elemHashCode #675

Closed
retronym opened this issue Mar 3, 2020 · 8 comments
Labels
Milestone

Comments

@retronym
Copy link
Member

retronym commented Mar 3, 2020

See: scala/scala#8589 (comment)

@retronym retronym added this to the 2.12.11 milestone Mar 3, 2020
@retronym
Copy link
Member Author

retronym commented Mar 3, 2020

Maybe this isn't a real problem because the base classes HashMap and HashSet are sealed. So the only way to customise elemHashCode is to extend HashMap1, HashTrieMap etc.

They are marked as:

@deprecatedInheritance("This class will be made final in a future release.", "2.12.2")
  class HashTrieMap[A, +B](

@retronym
Copy link
Member Author

retronym commented Mar 3, 2020

The choices are to add a defensive check that the operands of the optimized union are one of the known subclasses, or to agree this isn't a supported extension point.

@mkeskells
Copy link

Also the pre-existing implementations of HashMaps union/diff/intersect/merge/filter/++/-- …. and any other methods that traverse the tree of HashMap objects that assume the tree structure and bit representation of the hashcode is consistent between the receiver and the parameter

The same problem for HashSet, but fewer methods

@mkeskells
Copy link

the optimisations for the builders (in scala/scala#8722 and scala/scala#8726) may need consideration for the ++ optimisations

@retronym
Copy link
Member Author

retronym commented Mar 4, 2020

I'm leaning towards release noting the known breakage because I believe its more a theoretical problem than something that exists in the wild.

@lrytz
Copy link
Member

lrytz commented Mar 4, 2020

HashTrieSet is missing the deprecatedInheritance, but still, I agree this is extremely unlikely to break anything.

@mkeskells
Copy link

it would also affect HashSet/HashMap equals and HashSet subsetOf

I agree that it seems more a theoretical issue than a practical one

@mkeskells
Copy link

This also was an existing issue in 2.12 HashMap union/diff/intersect/merge as they were already tree based. The difference is that its now exposed via ++ -- filter etc

for merge this issue dates back to 2.9! https://github.com/scala/scala/blob/2.9.x/src/library/scala/collection/immutable/HashMap.scala#L392

@lrytz lrytz closed this as completed Mar 11, 2020
@scala scala deleted a comment Mar 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants