Skip to content

Commit

Permalink
Rework recent, buggy change to immutable.TreeMap
Browse files Browse the repository at this point in the history
We need to favour keys from the left that equivalent (but ne),
while favouring values from the right.

(cherry picked from commit abd82a2)
  • Loading branch information
retronym committed Jun 24, 2020
1 parent 72e2d77 commit 493168b
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 17 deletions.
34 changes: 17 additions & 17 deletions src/library/scala/collection/immutable/RedBlackTree.scala
Expand Up @@ -820,17 +820,17 @@ private[collection] object RedBlackTree {
} else mkTree(isRedTree(tl) || isRedTree(tr), k, v, tl, tr)
}

private[this] def split[A, B](t: Tree[A, B], k: A)(implicit ordering: Ordering[A]): (Tree[A, B], Tree[A, B], Tree[A, B]) =
if(t eq null) (null, null, null)
private[this] def split[A, B](t: Tree[A, B], k2: A)(implicit ordering: Ordering[A]): (Tree[A, B], Tree[A, B], Tree[A, B], A) =
if(t eq null) (null, null, null, k2)
else {
val cmp = ordering.compare(k, t.key)
if(cmp == 0) (t.left, t, t.right)
val cmp = ordering.compare(k2, t.key)
if(cmp == 0) (t.left, t, t.right, t.key)
else if(cmp < 0) {
val (ll, b, lr) = split(t.left, k)
(ll, b, join(lr, t.key, t.value, t.right))
val (ll, b, lr, k1) = split(t.left, k2)
(ll, b, join(lr, t.key, t.value, t.right), k1)
} else {
val (rl, b, rr) = split(t.right, k)
(join(t.left, t.key, t.value, rl), b, rr)
val (rl, b, rr, k1) = split(t.right, k2)
(join(t.left, t.key, t.value, rl), b, rr, k1)
}
}

Expand All @@ -853,28 +853,28 @@ private[collection] object RedBlackTree {
if((t1 eq null) || (t1 eq t2)) t2
else if(t2 eq null) t1
else {
val (l2, _, r2) = split(t2, t1.key)
val tl = _union(t1.left, l2)
val tr = _union(t1.right, r2)
join(tl, t1.key, t1.value, tr)
val (l1, _, r1, k1) = split(t1, t2.key)
val tl = _union(l1, t2.left)
val tr = _union(r1, t2.right)
join(tl, k1, t2.value, tr)
}

private[this] def _intersect[A, B](t1: Tree[A, B], t2: Tree[A, B])(implicit ordering: Ordering[A]): Tree[A, B] =
if((t1 eq null) || (t2 eq null)) null
else if (t1 eq t2) t1
else {
val (l2, b, r2) = split(t2, t1.key)
val tl = _intersect(t1.left, l2)
val tr = _intersect(t1.right, r2)
if(b ne null) join(tl, t1.key, t1.value, tr)
val (l1, b, r1, k1) = split(t1, t2.key)
val tl = _intersect(l1, t2.left)
val tr = _intersect(r1, t2.right)
if(b ne null) join(tl, k1, t2.value, tr)
else join2(tl, tr)
}

private[this] def _difference[A, B](t1: Tree[A, B], t2: Tree[A, B])(implicit ordering: Ordering[A]): Tree[A, B] =
if((t1 eq null) || (t2 eq null)) t1
else if (t1 eq t2) null
else {
val (l1, _, r1) = split(t1, t2.key)
val (l1, _, r1, k1) = split(t1, t2.key)
val tl = _difference(l1, t2.left)
val tr = _difference(r1, t2.right)
join2(tl, tr)
Expand Down
9 changes: 9 additions & 0 deletions test/junit/scala/collection/immutable/TreeMapTest.scala
Expand Up @@ -200,4 +200,13 @@ class TreeMapTest extends AllocationTest {
assertIdenticalKeys(Map((c0l, ())), TreeMap.newBuilder[C, Unit].++=(TreeMap((c0l, ()))).++=(HashMap((c0r, ()))).result())
assertIdenticalKeys(Map((c0l, ())), TreeMap.newBuilder[C, Unit].++=(TreeMap((c0l, ()))).++=(TreeMap((c0r, ()))).result())
}

@Test
def overwriteEntryRegression(): Unit = {
val x = TreeMap(1 -> "herring", 2 -> "cod", 3 -> "salmon")
val y = TreeMap(3 -> "wish")
val r1 = x ++ y
val r2 = (x.toSeq ++ y.toSeq).toMap
assertEquals(r1, r2)
}
}

0 comments on commit 493168b

Please sign in to comment.