From ee1ca4ffc4daac3e266c00d2310f526cc41d1829 Mon Sep 17 00:00:00 2001
From: Alan Donovan <alan@alandonovan.net>
Date: Mon, 27 Dec 2021 11:04:36 -0500
Subject: [PATCH] container/intsets: fix bug in UnionWith 'changed' result

This change corrects the computation of the 'changed' result
of the x.UnionWith(y) method in the case where a block of y is
a subset of the corresponding block of x. It also adds a test.

I audited for similar problems and found none.

Fixes golang/go#50352

Change-Id: I5224211a3cab06ce8986cdaa7b75c3b0531b3a9e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/374455
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
---
 container/intsets/sparse.go      |  5 +++--
 container/intsets/sparse_test.go | 35 ++++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/container/intsets/sparse.go b/container/intsets/sparse.go
index e9f19e62f7..2f1a0eaf31 100644
--- a/container/intsets/sparse.go
+++ b/container/intsets/sparse.go
@@ -666,8 +666,9 @@ func (s *Sparse) UnionWith(x *Sparse) bool {
 	for xb != &none {
 		if sb != &none && sb.offset == xb.offset {
 			for i := range xb.bits {
-				if sb.bits[i] != xb.bits[i] {
-					sb.bits[i] |= xb.bits[i]
+				union := sb.bits[i] | xb.bits[i]
+				if sb.bits[i] != union {
+					sb.bits[i] = union
 					changed = true
 				}
 			}
diff --git a/container/intsets/sparse_test.go b/container/intsets/sparse_test.go
index 7481a06b89..cd8ec6e084 100644
--- a/container/intsets/sparse_test.go
+++ b/container/intsets/sparse_test.go
@@ -460,6 +460,41 @@ func TestSetOperations(t *testing.T) {
 	}
 }
 
+// TestUnionWithChanged checks the 'changed' result of UnionWith.
+func TestUnionWithChanged(t *testing.T) {
+	setOf := func(elems ...int) *intsets.Sparse {
+		s := new(intsets.Sparse)
+		for _, elem := range elems {
+			s.Insert(elem)
+		}
+		return s
+	}
+
+	checkUnionWith := func(x, y *intsets.Sparse) {
+		xstr := x.String()
+		prelen := x.Len()
+		changed := x.UnionWith(y)
+		if (x.Len() > prelen) != changed {
+			t.Errorf("%s.UnionWith(%s) => %s, changed=%t", xstr, y, x, changed)
+		}
+	}
+
+	// The case marked "!" is a regression test for Issue 50352,
+	// which spuriously returned true when y ⊂ x.
+
+	// same block
+	checkUnionWith(setOf(1, 2), setOf(1, 2))
+	checkUnionWith(setOf(1, 2, 3), setOf(1, 2)) // !
+	checkUnionWith(setOf(1, 2), setOf(1, 2, 3))
+	checkUnionWith(setOf(1, 2), setOf())
+
+	// different blocks
+	checkUnionWith(setOf(1, 1000000), setOf(1, 1000000))
+	checkUnionWith(setOf(1, 2, 1000000), setOf(1, 2))
+	checkUnionWith(setOf(1, 2), setOf(1, 2, 1000000))
+	checkUnionWith(setOf(1, 1000000), setOf())
+}
+
 func TestIntersectionWith(t *testing.T) {
 	// Edge cases: the pairs (1,1), (1000,2000), (8000,4000)
 	// exercise the <, >, == cases in IntersectionWith that the