diff --git a/api_test.go b/api_test.go index 24ac6c4..1589013 100644 --- a/api_test.go +++ b/api_test.go @@ -581,3 +581,68 @@ func TestIteratorReadRanges(t *testing.T) { t.Error("expected zero ranges for an empty buffer") } } + +// The in-place C routines assert that their operands are distinct; applying a +// bitmap to itself used to abort the process. +func TestSelfAliasedOperations(t *testing.T) { + fresh := func() *Bitmap { return New(1, 2, 3, 100000) } + + rb := fresh() + rb.Xor(rb) + if !rb.IsEmpty() { + t.Errorf("x XOR x should be empty, got %v", rb.ToArray()) + } + + rb = fresh() + rb.AndNot(rb) + if !rb.IsEmpty() { + t.Errorf("x ANDNOT x should be empty, got %v", rb.ToArray()) + } + + rb = fresh() + rb.And(rb) + if !reflect.DeepEqual(rb.ToArray(), fresh().ToArray()) { + t.Errorf("x AND x should be x, got %v", rb.ToArray()) + } + + rb = fresh() + rb.Or(rb) + if !reflect.DeepEqual(rb.ToArray(), fresh().ToArray()) { + t.Errorf("x OR x should be x, got %v", rb.ToArray()) + } + + rb = fresh() + rb.LazyOrInplace(rb, false) + rb.RepairAfterLazy() + if !reflect.DeepEqual(rb.ToArray(), fresh().ToArray()) { + t.Errorf("lazy x OR x should be x, got %v", rb.ToArray()) + } + + rb = fresh() + rb.LazyXorInplace(rb) + rb.RepairAfterLazy() + if !rb.IsEmpty() { + t.Errorf("lazy x XOR x should be empty, got %v", rb.ToArray()) + } + + rb = fresh() + if !rb.Assign(rb) { + t.Error("self-assignment should succeed") + } + if !reflect.DeepEqual(rb.ToArray(), fresh().ToArray()) { + t.Errorf("self-assignment should be a no-op, got %v", rb.ToArray()) + } +} + +func TestFromRangeEmpty(t *testing.T) { + for _, c := range []struct{ min, max uint64 }{{5, 5}, {10, 3}, {0, 0}} { + rb := FromRange(c.min, c.max, 1) + if !rb.IsEmpty() { + t.Errorf("FromRange(%d, %d, 1) should be empty, got %v", c.min, c.max, rb.ToArray()) + } + } + // The 32-bit range saturates at 2^32, so this is not an empty range. + if got := FromRange(0xFFFFFFFE, 1<<40, 1).Cardinality(); got != 2 { + t.Errorf("expected 2 values, got %d", got) + } +} diff --git a/gocroaring.go b/gocroaring.go index ca25cb4..57a1eb8 100644 --- a/gocroaring.go +++ b/gocroaring.go @@ -314,12 +314,20 @@ func NewWithCapacity(capacity uint32) *Bitmap { } // FromRange creates a bitmap containing min, min+step, min+2*step... up to but -// not including max. The step must be strictly positive. +// not including max. An empty range yields an empty bitmap. The step must be +// strictly positive. // This function may panic if the allocation failed. func FromRange(min, max uint64, step uint32) *Bitmap { if step == 0 { panic("gocroaring: FromRange requires a strictly positive step") } + // C returns a null pointer for an empty range, which is not an error. + if max > 0x100000000 { + max = 0x100000000 + } + if max <= min { + return New() + } return wrap(C.roaring_bitmap_from_range(C.uint64_t(min), C.uint64_t(max), C.uint32_t(step))) } @@ -333,6 +341,9 @@ func (rb *Bitmap) Clone() *Bitmap { // Assign copies x2 over rb, returning false if the copy failed. func (rb *Bitmap) Assign(x2 *Bitmap) bool { + if rb.aliases(x2) { + return true // already a copy of itself + } answer := bool(C.roaring_bitmap_overwrite(rb.cpointer, x2.cpointer)) runtime.KeepAlive(rb) runtime.KeepAlive(x2) @@ -732,9 +743,19 @@ func (rb *Bitmap) SetCopyOnWrite(cow bool) { // In-place set operations //////////////////////////////////////////////////////////////////////////////// +// aliases reports whether the two wrappers refer to the same C bitmap. Several +// of the in-place C routines assert that their operands are distinct, so we +// answer the aliased cases ourselves rather than let the C library abort. +func (rb *Bitmap) aliases(x2 *Bitmap) bool { + return rb == x2 || rb.cpointer == x2.cpointer +} + // And computes the intersection between two bitmaps and stores the result in // the current bitmap. func (rb *Bitmap) And(x2 *Bitmap) { + if rb.aliases(x2) { + return // x AND x is x + } C.roaring_bitmap_and_inplace(rb.cpointer, x2.cpointer) runtime.KeepAlive(rb) runtime.KeepAlive(x2) @@ -743,6 +764,10 @@ func (rb *Bitmap) And(x2 *Bitmap) { // Xor computes the symmetric difference between two bitmaps and stores the // result in the current bitmap. func (rb *Bitmap) Xor(x2 *Bitmap) { + if rb.aliases(x2) { + rb.Clear() // x XOR x is empty + return + } C.roaring_bitmap_xor_inplace(rb.cpointer, x2.cpointer) runtime.KeepAlive(rb) runtime.KeepAlive(x2) @@ -751,6 +776,9 @@ func (rb *Bitmap) Xor(x2 *Bitmap) { // Or computes the union between two bitmaps and stores the result in the // current bitmap. func (rb *Bitmap) Or(x2 *Bitmap) { + if rb.aliases(x2) { + return // x OR x is x + } C.roaring_bitmap_or_inplace(rb.cpointer, x2.cpointer) runtime.KeepAlive(rb) runtime.KeepAlive(x2) @@ -759,6 +787,10 @@ func (rb *Bitmap) Or(x2 *Bitmap) { // AndNot computes the difference between two bitmaps and stores the result in // the current bitmap. func (rb *Bitmap) AndNot(x2 *Bitmap) { + if rb.aliases(x2) { + rb.Clear() // x ANDNOT x is empty + return + } C.roaring_bitmap_andnot_inplace(rb.cpointer, x2.cpointer) runtime.KeepAlive(rb) runtime.KeepAlive(x2) @@ -768,15 +800,21 @@ func (rb *Bitmap) AndNot(x2 *Bitmap) { // invalid state until RepairAfterLazy is called. Set bitsetconversion to true // to eagerly convert containers to bitsets when it might help. func (rb *Bitmap) LazyOrInplace(x2 *Bitmap, bitsetconversion bool) { + if rb.aliases(x2) { + return // x OR x is x + } C.roaring_bitmap_lazy_or_inplace(rb.cpointer, x2.cpointer, C.bool(bitsetconversion)) runtime.KeepAlive(rb) runtime.KeepAlive(x2) } // LazyXorInplace computes the symmetric difference with x2 in place, leaving -// the bitmap in an invalid state until RepairAfterLazy is called. The two -// bitmaps must be distinct. +// the bitmap in an invalid state until RepairAfterLazy is called. func (rb *Bitmap) LazyXorInplace(x2 *Bitmap) { + if rb.aliases(x2) { + rb.Clear() // x XOR x is empty + return + } C.roaring_bitmap_lazy_xor_inplace(rb.cpointer, x2.cpointer) runtime.KeepAlive(rb) runtime.KeepAlive(x2) diff --git a/gocroaring64.go b/gocroaring64.go index afb93cb..666f18c 100644 --- a/gocroaring64.go +++ b/gocroaring64.go @@ -240,18 +240,28 @@ func New64(x ...uint64) *Bitmap64 { } // FromRange64 creates a bitmap containing min, min+step, min+2*step... up to -// but not including max. The step must be strictly positive. +// but not including max. An empty range yields an empty bitmap. The step must +// be strictly positive. // This function may panic if the allocation failed. func FromRange64(min, max, step uint64) *Bitmap64 { if step == 0 { panic("gocroaring: FromRange64 requires a strictly positive step") } + // C returns a null pointer for an empty range, which is not an error. + if max <= min { + return New64() + } return wrap64(C.roaring64_bitmap_from_range(C.uint64_t(min), C.uint64_t(max), C.uint64_t(step))) } // MoveFrom32 builds a 64-bit bitmap by moving the containers out of a 32-bit // bitmap. This avoids copying the container data, but it leaves the source // bitmap empty. +// +// The containers are taken without regard for copy-on-write sharing, so do not +// use it on a bitmap whose containers are shared with another one (that is, a +// bitmap that has copy-on-write enabled and has been cloned). Clone first if +// you need the source intact. // This function may panic if the allocation failed. func MoveFrom32(from *Bitmap) *Bitmap64 { b := wrap64(C.roaring64_bitmap_move_from_roaring32(from.cpointer)) @@ -269,6 +279,9 @@ func (rb *Bitmap64) Clone() *Bitmap64 { // Assign copies x2 over rb. func (rb *Bitmap64) Assign(x2 *Bitmap64) { + if rb.aliases(x2) { + return // already a copy of itself + } C.roaring64_bitmap_overwrite(rb.cpointer, x2.cpointer) runtime.KeepAlive(rb) runtime.KeepAlive(x2) @@ -628,9 +641,19 @@ func (rb *Bitmap64) InternalValidate() error { // In-place set operations //////////////////////////////////////////////////////////////////////////////// +// aliases reports whether the two wrappers refer to the same C bitmap. Several +// of the in-place C routines assert that their operands are distinct, so we +// answer the aliased cases ourselves rather than let the C library abort. +func (rb *Bitmap64) aliases(x2 *Bitmap64) bool { + return rb == x2 || rb.cpointer == x2.cpointer +} + // And computes the intersection between two bitmaps and stores the result in // the current bitmap. func (rb *Bitmap64) And(x2 *Bitmap64) { + if rb.aliases(x2) { + return // x AND x is x + } C.roaring64_bitmap_and_inplace(rb.cpointer, x2.cpointer) runtime.KeepAlive(rb) runtime.KeepAlive(x2) @@ -639,6 +662,10 @@ func (rb *Bitmap64) And(x2 *Bitmap64) { // Xor computes the symmetric difference between two bitmaps and stores the // result in the current bitmap. func (rb *Bitmap64) Xor(x2 *Bitmap64) { + if rb.aliases(x2) { + rb.Clear() // x XOR x is empty + return + } C.roaring64_bitmap_xor_inplace(rb.cpointer, x2.cpointer) runtime.KeepAlive(rb) runtime.KeepAlive(x2) @@ -647,6 +674,9 @@ func (rb *Bitmap64) Xor(x2 *Bitmap64) { // Or computes the union between two bitmaps and stores the result in the // current bitmap. func (rb *Bitmap64) Or(x2 *Bitmap64) { + if rb.aliases(x2) { + return // x OR x is x + } C.roaring64_bitmap_or_inplace(rb.cpointer, x2.cpointer) runtime.KeepAlive(rb) runtime.KeepAlive(x2) @@ -655,6 +685,10 @@ func (rb *Bitmap64) Or(x2 *Bitmap64) { // AndNot computes the difference between two bitmaps and stores the result in // the current bitmap. func (rb *Bitmap64) AndNot(x2 *Bitmap64) { + if rb.aliases(x2) { + rb.Clear() // x ANDNOT x is empty + return + } C.roaring64_bitmap_andnot_inplace(rb.cpointer, x2.cpointer) runtime.KeepAlive(rb) runtime.KeepAlive(x2) @@ -905,8 +939,9 @@ func AlignedBuffer64(size int) []byte { // ReadFrozenView64 reads a frozen serialized version of the bitmap, as written // by Bitmap64.WriteFrozen. The result is immutable: attempting to mutate it // will fail catastrophically. The buffer must be aligned on a -// Frozen64Alignment boundary (see AlignedBuffer64). A reference to the buffer -// is retained for the lifetime of the view. +// Frozen64Alignment boundary (see AlignedBuffer64) and its length must be +// exactly the length that was written. A reference to the buffer is retained +// for the lifetime of the view. func ReadFrozenView64(b []byte) (*Bitmap64, error) { if len(b) == 0 { return nil, ErrEmptyBuffer diff --git a/gocroaring64_test.go b/gocroaring64_test.go index 65466c5..8781eb4 100644 --- a/gocroaring64_test.go +++ b/gocroaring64_test.go @@ -590,3 +590,46 @@ func TestIterator64ReadRanges(t *testing.T) { t.Error("expected zero ranges for an empty buffer") } } + +func TestSelfAliasedOperations64(t *testing.T) { + fresh := func() *Bitmap64 { return New64(1, 2, big, big+1) } + + rb := fresh() + rb.Xor(rb) + if !rb.IsEmpty() { + t.Errorf("x XOR x should be empty, got %v", rb.ToArray()) + } + + rb = fresh() + rb.AndNot(rb) + if !rb.IsEmpty() { + t.Errorf("x ANDNOT x should be empty, got %v", rb.ToArray()) + } + + rb = fresh() + rb.And(rb) + if !reflect.DeepEqual(rb.ToArray(), fresh().ToArray()) { + t.Errorf("x AND x should be x, got %v", rb.ToArray()) + } + + rb = fresh() + rb.Or(rb) + if !reflect.DeepEqual(rb.ToArray(), fresh().ToArray()) { + t.Errorf("x OR x should be x, got %v", rb.ToArray()) + } + + rb = fresh() + rb.Assign(rb) + if !reflect.DeepEqual(rb.ToArray(), fresh().ToArray()) { + t.Errorf("self-assignment should be a no-op, got %v", rb.ToArray()) + } +} + +func TestFromRange64Empty(t *testing.T) { + for _, c := range []struct{ min, max uint64 }{{5, 5}, {10, 3}, {big, big}} { + rb := FromRange64(c.min, c.max, 1) + if !rb.IsEmpty() { + t.Errorf("FromRange64(%d, %d, 1) should be empty, got %v", c.min, c.max, rb.ToArray()) + } + } +}