Copy: bug: destination is smaller than source
paralin opened this issue · 10 comments
Line 535 in 5a82924
It seems that here it assumes that the destination c.set will have len >= len(b.set).
Otherwise the copy() will only copy the length of the smaller, c.set.
Shouldn't c.set be extended here if it's shorter??
This function in general seems broken: it doesn't set the length of the target set. So length will be wrong? It just returns the shorter of the two lengths...
The function is...
// Copy into a destination BitSet
// Returning the size of the destination BitSet
// like array copy
func (b *BitSet) Copy(c *BitSet) (count uint) {
if c == nil {
return
}
if b.set != nil { // Copy should not modify current object
copy(c.set, b.set)
}
count = c.length
if b.length < c.length {
count = b.length
}
return
}If you believe that there is a bug, please provide a reproducing test case.
@lemire In the code snippet you sent, please explain what happens if len(c.set) < len(b.set)??
... because what will happen is it will copy only len(c.set) int64s and drop the remainder.
Is this intended?
The copy function is not externally accessible.
What? The one I linked to is the one you pasted here. Copy(). Please click the link.
Is this intended?
Yes.
If you copy a large bitset into a smaller one, the leftover content is discarded, just like what happens in standard Go when copying arrays. Admittedly, the comment could be clearer, but that's what 'like array copy' means... we use the semantics of an array copy in Go. The final destination size is returned.
I will improve the comment in a commit.
If you think that there is a bug, please elaborate.
I am going to close this for now. In Go, an array copy does not automatically extend the destination. We follow this expected semantics.
If you have further ideas, I encourage you to consider producing a pull request, and to elaborate.
We generally want to avoid 'silent' memory allocation in Go.
Thanks for the feedback.