diff --git a/arraycontainer.go b/arraycontainer.go index 84075c68..c03b135e 100644 --- a/arraycontainer.go +++ b/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 { diff --git a/inplace_ownership_test.go b/inplace_ownership_test.go new file mode 100644 index 00000000..eef0f7bb --- /dev/null +++ b/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) + } + } + } +} diff --git a/roaring64/inplace_ownership_test.go b/roaring64/inplace_ownership_test.go new file mode 100644 index 00000000..aaec3fc6 --- /dev/null +++ b/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) + } + } + } +} diff --git a/roaring64/roaring64.go b/roaring64/roaring64.go index 143209de..cea0da3e 100644 --- a/roaring64/roaring64.go +++ b/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++ diff --git a/runcontainer.go b/runcontainer.go index 7c369b75..7df434cd 100644 --- a/runcontainer.go +++ b/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 {