Fix Xor modifying its argument and 64-bit self-XOR

arraycontainer.go

@@ -645,7 +645,7 @@ func (ac *arrayContainer) ixorArray(value2 *arrayContainer) container {
 }
 
 func (ac *arrayContainer) ixorBitmap(value2 *bitmapContainer) container {
-	return value2.ixor(ac)
+	return value2.xorArray(ac)
 }
 
 func (ac *arrayContainer) ixorRun16(value2 *runContainer16) container {

inplace_ownership_test.go

@@ -0,0 +1,86 @@
+package roaring
+
+import "testing"
+
+// Every in-place set operation must leave its argument untouched, and the
+// two bitmaps must not share storage afterwards.
+func TestInPlaceOpsPreserveArgument(t *testing.T) {
+	const hi = 1 << 16
+	dense := func(base uint32) *Bitmap {
+		b := New()
+		for i := uint32(0); i < 65536; i += 2 {
+			b.Add(base | i)
+		}
+		return b
+	}
+	run := func(base uint32) *Bitmap {
+		b := New()
+		b.AddRange(uint64(base)+100, uint64(base)+60000)
+		return b
+	}
+	twoKeys := func(b *Bitmap) *Bitmap {
+		b = b.Clone()
+		b.AddRange(hi, hi+5)
+		return b
+	}
+	shapes := []struct {
+		name string
+		bm   *Bitmap
+	}{
+		{"empty", New()}, {"array", BitmapOf(1, 3, 65535)}, {"bitmap", dense(0)}, {"run", run(0)},
+		{"array+key", twoKeys(BitmapOf(2, 4))}, {"run+key", twoKeys(run(0))},
+		{"hi-array", BitmapOf(hi|1, hi|3)}, {"hi-bitmap", dense(hi)}, {"hi-run", run(hi)},
+	}
+	ops := []struct {
+		name    string
+		inPlace func(a, b *Bitmap)
+		pure    func(a, b *Bitmap) *Bitmap
+	}{
+		{"Or", func(a, b *Bitmap) { a.Or(b) }, Or},
+		{"And", func(a, b *Bitmap) { a.And(b) }, And},
+		{"AndNot", func(a, b *Bitmap) { a.AndNot(b) }, AndNot},
+		{"Xor", func(a, b *Bitmap) { a.Xor(b) }, Xor},
+	}
+	for _, op := range ops {
+		for _, left := range shapes {
+			for _, right := range shapes {
+				l, r := left.bm, right.bm
+				for _, cow := range []bool{false, true} {
+					name := op.name + "/" + left.name + "/" + right.name
+					a, b := l.Clone(), r.Clone()
+					a.SetCopyOnWrite(cow)
+					b.SetCopyOnWrite(cow)
+					snapshot := a.Clone()
+					want := op.pure(l, r)
+					op.inPlace(a, b)
+					if !snapshot.Equals(l) {
+						t.Fatalf("%s: a clone of the receiver changed (copy-on-write %v)", name, cow)
+					}
+					if !a.Equals(want) {
+						t.Fatalf("%s: wrong result", name)
+					}
+					if err := a.Validate(); err != nil {
+						t.Fatalf("%s: %v", name, err)
+					}
+					if !b.Equals(r) {
+						t.Fatalf("%s: argument modified (copy-on-write %v)", name, cow)
+					}
+					it := r.Iterator()
+					for n := 0; n < 50 && it.HasNext(); n++ {
+						a.Remove(it.Next())
+					}
+					a.Add(65533)
+					a.AddRange(hi+70000, hi+70050)
+					if !b.Equals(r) {
+						t.Fatalf("%s: editing the result changed the argument (copy-on-write %v)", name, cow)
+					}
+				}
+			}
+			a := left.bm.Clone()
+			op.inPlace(a, a)
+			if !a.Equals(op.pure(left.bm, left.bm)) {
+				t.Fatalf("%s/%s: wrong result for a bitmap applied to itself", op.name, left.name)
+			}
+		}
+	}
+}

roaring64/inplace_ownership_test.go

@@ -0,0 +1,86 @@
+package roaring64
+
+import "testing"
+
+// Every in-place set operation must leave its argument untouched, and the
+// two bitmaps must not share storage afterwards.
+func TestInPlaceOpsPreserveArgument(t *testing.T) {
+	const hi = 1 << 32
+	dense := func(base uint64) *Bitmap {
+		b := New()
+		for i := uint64(0); i < 65536; i += 2 {
+			b.Add(base | i)
+		}
+		return b
+	}
+	run := func(base uint64) *Bitmap {
+		b := New()
+		b.AddRange(base+100, base+60000)
+		return b
+	}
+	twoKeys := func(b *Bitmap) *Bitmap {
+		b = b.Clone()
+		b.AddRange(hi, hi+5)
+		return b
+	}
+	shapes := []struct {
+		name string
+		bm   *Bitmap
+	}{
+		{"empty", New()}, {"array", BitmapOf(1, 3, 65535)}, {"bitmap", dense(0)}, {"run", run(0)},
+		{"array+key", twoKeys(BitmapOf(2, 4))}, {"run+key", twoKeys(run(0))},
+		{"hi-array", BitmapOf(hi|1, hi|3)}, {"hi-bitmap", dense(hi)}, {"hi-run", run(hi)},
+	}
+	ops := []struct {
+		name    string
+		inPlace func(a, b *Bitmap)
+		pure    func(a, b *Bitmap) *Bitmap
+	}{
+		{"Or", func(a, b *Bitmap) { a.Or(b) }, Or},
+		{"And", func(a, b *Bitmap) { a.And(b) }, And},
+		{"AndNot", func(a, b *Bitmap) { a.AndNot(b) }, AndNot},
+		{"Xor", func(a, b *Bitmap) { a.Xor(b) }, Xor},
+	}
+	for _, op := range ops {
+		for _, left := range shapes {
+			for _, right := range shapes {
+				l, r := left.bm, right.bm
+				for _, cow := range []bool{false, true} {
+					name := op.name + "/" + left.name + "/" + right.name
+					a, b := l.Clone(), r.Clone()
+					a.SetCopyOnWrite(cow)
+					b.SetCopyOnWrite(cow)
+					snapshot := a.Clone()
+					want := op.pure(l, r)
+					op.inPlace(a, b)
+					if !snapshot.Equals(l) {
+						t.Fatalf("%s: a clone of the receiver changed (copy-on-write %v)", name, cow)
+					}
+					if !a.Equals(want) {
+						t.Fatalf("%s: wrong result", name)
+					}
+					if err := a.Validate(); err != nil {
+						t.Fatalf("%s: %v", name, err)
+					}
+					if !b.Equals(r) {
+						t.Fatalf("%s: argument modified (copy-on-write %v)", name, cow)
+					}
+					it := r.Iterator()
+					for n := 0; n < 50 && it.HasNext(); n++ {
+						a.Remove(it.Next())
+					}
+					a.Add(65533)
+					a.AddRange(hi+70000, hi+70050)
+					if !b.Equals(r) {
+						t.Fatalf("%s: editing the result changed the argument (copy-on-write %v)", name, cow)
+					}
+				}
+			}
+			a := left.bm.Clone()
+			op.inPlace(a, a)
+			if !a.Equals(op.pure(left.bm, left.bm)) {
+				t.Fatalf("%s/%s: wrong result for a bitmap applied to itself", op.name, left.name)
+			}
+		}
+	}
+}

roaring64/roaring64.go

@@ -632,6 +632,10 @@ main:
 
 // Xor computes the symmetric difference between two bitmaps and stores the result in the current bitmap
 func (rb *Bitmap) Xor(x2 *Bitmap) {
+	if rb == x2 {
+		rb.Clear()
+		return
+	}
 	pos1 := 0
 	pos2 := 0
 	length1 := rb.highlowcontainer.size()
@@ -646,7 +650,7 @@ func (rb *Bitmap) Xor(x2 *Bitmap) {
 					break
 				}
 			} else if s1 > s2 {
-				c := x2.highlowcontainer.getWritableContainerAtIndex(pos2)
+				c := x2.highlowcontainer.getContainerAtIndex(pos2).Clone()
 				rb.highlowcontainer.insertNewKeyValueAt(pos1, x2.highlowcontainer.getKeyAtIndex(pos2), c)
 				length1++
 				pos1++

runcontainer.go

@@ -2452,7 +2452,7 @@ func (rc *runContainer16) ixorArray(value2 *arrayContainer) container {
 }
 
 func (rc *runContainer16) ixorBitmap(value2 *bitmapContainer) container {
-	return value2.ixor(rc)
+	return rc.toBitmapContainer().ixorBitmap(value2)
 }
 
 func (rc *runContainer16) ixorRunContainer16(value2 *runContainer16) container {