diff options
author | Paul Phillips <paulp@improving.org> | 2010-11-09 03:13:17 +0000 |
---|---|---|
committer | Paul Phillips <paulp@improving.org> | 2010-11-09 03:13:17 +0000 |
commit | e46a39797744d1fb4bda637f31034be5d25a6338 (patch) | |
tree | 24166bae78a78982d0ae30187761d7b22ba85f91 /src | |
parent | 6cb5c25802ec68d31fccde6bf9ee2fd2e8e3ff45 (diff) | |
download | scala-e46a39797744d1fb4bda637f31034be5d25a6338.tar.gz scala-e46a39797744d1fb4bda637f31034be5d25a6338.tar.bz2 scala-e46a39797744d1fb4bda637f31034be5d25a6338.zip |
While investigating the cause of #3984, discove...
While investigating the cause of #3984, discovered that set hashcodes
were being calculated like:
elems map (_.hashCode) sum
This looks totally correct, except that because of set+map semantics it
drops all the duplicate hashcodes and therefore calculates the wrong
sum anytime there are unequal elements with equal hashcodes. If we're
married to the set+map semantics (and I don't know what could be done
about it) I had better get a proper -Xlint going because this is one of
those traps which shoots bees out of its mouth. No review.
Diffstat (limited to 'src')
-rw-r--r-- | src/library/scala/collection/SetLike.scala | 15 | ||||
-rw-r--r-- | src/library/scala/collection/mutable/SetLike.scala | 3 |
2 files changed, 13 insertions, 5 deletions
diff --git a/src/library/scala/collection/SetLike.scala b/src/library/scala/collection/SetLike.scala index fbbd77d8aa..5a3b9734ed 100644 --- a/src/library/scala/collection/SetLike.scala +++ b/src/library/scala/collection/SetLike.scala @@ -11,6 +11,7 @@ package scala.collection import generic._ import mutable.{Builder, AddingBuilder} +import scala.annotation.migration /** A template trait for sets. * @@ -72,6 +73,12 @@ self => */ override protected[this] def newBuilder: Builder[A, This] = new AddingBuilder[A, This](empty) + // note: this is only overridden here to add the migration annotation, + // which I hope to turn into an Xlint style warning as the migration aspect + // is not central to its importance. + @migration(2, 8, "Set.map now returns a Set, so it will discard duplicate values.") + override def map[B, That](f: A => B)(implicit bf: CanBuildFrom[This, B, That]): That = super.map(f)(bf) + /** Tests if some element is contained in this set. * * @param elem the element to test for membership. @@ -182,9 +189,13 @@ self => * Unless overridden this is simply `"Set"`. */ override def stringPrefix: String = "Set" - override def toString = super[IterableLike].toString - override def hashCode() = this map (_.hashCode) sum + + // Careful! Don't write a Set's hashCode like: + // override def hashCode() = this map (_.hashCode) sum + // Calling map on a set drops duplicates: any hashcode collisions would + // then be dropped before they can be added. + override def hashCode() = this.foldLeft(0)(_ + _.hashCode) /** Compares this set with another object for equality. * diff --git a/src/library/scala/collection/mutable/SetLike.scala b/src/library/scala/collection/mutable/SetLike.scala index 6264d1fbc2..26f19555cd 100644 --- a/src/library/scala/collection/mutable/SetLike.scala +++ b/src/library/scala/collection/mutable/SetLike.scala @@ -68,9 +68,6 @@ trait SetLike[A, +This <: SetLike[A, This] with Set[A]] */ override protected[this] def newBuilder: Builder[A, This] = empty - @migration(2, 8, "Set.map now returns a Set, so it will discard duplicate values.") - override def map[B, That](f: A => B)(implicit bf: CanBuildFrom[This, B, That]): That = super.map(f)(bf) - /** Adds an element to this $coll. * * @param elem the element to be added |