Skip to content

Commit 9657efd

Browse files
authored
Merge pull request scala/scala#9311 from retronym/topic/sorted-map-set-bincompat
Avoid use of binary incompatible super accessors in SortedMap/Set
2 parents 0d3ce98 + f94d559 commit 9657efd

File tree

6 files changed

+41
-27
lines changed

6 files changed

+41
-27
lines changed

library/src/scala/collection/GenMap.scala

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ package scala
1414
package collection
1515

1616
import generic._
17+
import scala.runtime.AbstractFunction1
1718

1819
/** A trait for all traversable collections which may possibly
1920
* have their operations implemented in parallel.
@@ -39,4 +40,24 @@ object GenMap extends GenMapFactory[GenMap] {
3940
ReusableCBF.asInstanceOf[CanBuildFrom[Coll, (K, V), GenMap[K, V]]]
4041
private[this] val ReusableCBF = new MapCanBuildFrom[Nothing, Nothing]
4142

43+
private[collection] def mapEquals[K1, V, K2](thisMap: GenMapLike[K1, V, _], thatMap: GenMap[K2, _]): Boolean = {
44+
(thisMap eq thatMap) ||
45+
(thatMap canEqual thisMap) &&
46+
(thisMap.size == thatMap.size) && {
47+
try {
48+
val checker = new AbstractFunction1[(K1, V),Boolean] with Function0[V]{
49+
override def apply(kv: (K1,V)): Boolean = {
50+
// Note: uncurry optimizes this to `get.getOrElse(..., this: Function0)`; there is no extra lambda allocated.
51+
val v2 = thatMap.getOrElse(kv._1.asInstanceOf[K2], this.apply())
52+
// A mis-behaving user-defined equals method might not expect the sentinel value, and we should try to limit
53+
// the chance of it escaping. Its also probably quicker to avoid the virtual call to equals.
54+
(v2.asInstanceOf[AnyRef] ne this) && v2 == kv._2
55+
}
56+
override def apply(): V = this.asInstanceOf[V]
57+
}
58+
thisMap forall checker
59+
} catch {
60+
case ex: ClassCastException => false
61+
}}
62+
}
4263
}

library/src/scala/collection/GenMapLike.scala

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,6 @@
1313
package scala
1414
package collection
1515

16-
import scala.runtime.AbstractFunction1
17-
1816
/** A trait for all maps upon which operations may be
1917
* implemented in parallel.
2018
*
@@ -117,25 +115,9 @@ trait GenMapLike[K, +V, +Repr] extends GenIterableLike[(K, V), Repr] with Equals
117115
* same mappings, `false` otherwise.
118116
*/
119117
override def equals(that: Any): Boolean = that match {
118+
// copy/pasted to immutable.SortedMap.equals for binary compat reasons!
120119
case that: GenMap[b, _] =>
121-
(this eq that) ||
122-
(that canEqual this) &&
123-
(this.size == that.size) && {
124-
try {
125-
val checker = new AbstractFunction1[(K, V),Boolean] with Function0[V]{
126-
override def apply(kv: (K,V)): Boolean = {
127-
// Note: uncurry optimizes this to `get.getOrElse(..., this: Function0)`; there is no extra lambda allocated.
128-
val v2 = that.getOrElse(kv._1.asInstanceOf[b], this.apply())
129-
// A mis-behaving user-defined equals method might not expect the sentinel value, and we should try to limit
130-
// the chance of it escaping. Its also probably quicker to avoid the virtual call to equals.
131-
(v2.asInstanceOf[AnyRef] ne this) && v2 == kv._2
132-
}
133-
override def apply(): V = this.asInstanceOf[V]
134-
}
135-
this forall checker
136-
} catch {
137-
case ex: ClassCastException => false
138-
}}
120+
GenMap.mapEquals(this, that)
139121
case _ =>
140122
false
141123
}

library/src/scala/collection/GenSet.scala

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,5 +37,12 @@ extends GenSetLike[A, GenSet[A]]
3737
object GenSet extends GenTraversableFactory[GenSet] {
3838
implicit def canBuildFrom[A] = ReusableCBF.asInstanceOf[GenericCanBuildFrom[A]]
3939
def newBuilder[A] = Set.newBuilder
40+
private[collection] def setEquals[A1, A2](thisSet: GenSetLike[A1, _], thatSet: GenSet[A2]): Boolean = {
41+
(thisSet eq thatSet) ||
42+
(thatSet canEqual thisSet) &&
43+
(thisSet.size == thatSet.size) &&
44+
(try thisSet subsetOf thatSet.asInstanceOf[GenSet[A1]]
45+
catch { case ex: ClassCastException => false })
46+
}
4047
}
4148

library/src/scala/collection/GenSetLike.scala

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -117,12 +117,9 @@ extends GenIterableLike[A, Repr]
117117
* as this set.
118118
*/
119119
override def equals(that: Any): Boolean = that match {
120+
// copy/pasted to immutable.SortedSet.equals for binary compat reasons!
120121
case that: GenSet[_] =>
121-
(this eq that) ||
122-
(that canEqual this) &&
123-
(this.size == that.size) &&
124-
(try this subsetOf that.asInstanceOf[GenSet[A]]
125-
catch { case ex: ClassCastException => false })
122+
GenSet.setEquals(this, that)
126123
case _ =>
127124
false
128125
}

library/src/scala/collection/immutable/SortedMap.scala

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,11 @@ self =>
114114
allEqual = i1.next() == i2.next()
115115
allEqual
116116
}
117-
case _ => super.equals(that)
117+
// copy/pasted from super.equals for binary compat reasons!
118+
case that: GenMap[b, _] =>
119+
GenMap.mapEquals(this, that)
120+
case _ =>
121+
false && super.equals(that) // generate unused super accessor for binary compatibility (scala/scala#9311)
118122
}
119123
}
120124

library/src/scala/collection/immutable/SortedSet.scala

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,11 @@ trait SortedSet[A] extends Set[A] with scala.collection.SortedSet[A] with Sorted
4141
allEqual = i1.next() == i2.next
4242
allEqual
4343
}
44+
// copy/pasted from super.equals for binary compat reasons!
45+
case that: GenSet[_] =>
46+
GenSet.setEquals(this, that)
4447
case _ =>
45-
super.equals(that)
48+
false && super.equals(that) // generate unused super accessor for binary compatibility (scala/scala#9311)
4649
}
4750
}
4851
/** $factoryInfo

0 commit comments

Comments
 (0)