Skip to content

impl. AppendTo and AsSlice#195

Merged
lemire merged 1 commit intobits-and-blooms:masterfrom
gaissmai:fb-append
Dec 16, 2024
Merged

impl. AppendTo and AsSlice#195
lemire merged 1 commit intobits-and-blooms:masterfrom
gaissmai:fb-append

Conversation

@gaissmai
Copy link
Copy Markdown
Contributor

added AsSlice() and AppendTo() as convenience functions, if you like it, merge it, otherwise drop the PR

thanks for bitset

@gaissmai
Copy link
Copy Markdown
Contributor Author

huch, wait a moment, a panic test isn't working as expected

@gaissmai
Copy link
Copy Markdown
Contributor Author

ready for merge, or drop it if you don't like

// AppendTo appends all set bits to buf and returns the (maybe extended) buf.
//
// See also [BitSet.AsSlice] and [BitSet.NextSetMany].
func (b *BitSet) AppendTo(buf []uint) []uint {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Theoretically, we can overflow uint. It would be nice to mention that what happens if someone has a gigantic bitset.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will do

@lemire
Copy link
Copy Markdown
Member

lemire commented Dec 16, 2024

@gaissmai Please see my comment.

@gaissmai
Copy link
Copy Markdown
Contributor Author

I'll will push an update in a few minutes

@gaissmai
Copy link
Copy Markdown
Contributor Author

gaissmai commented Dec 16, 2024

I'm not a native speaker, maybe you will polish my new docstring.

edited: not a native english speaker ;)

@lemire
Copy link
Copy Markdown
Member

lemire commented Dec 16, 2024

@gaissmai Let me explain what I meant.

On a 32-bit system, uint maxes out at 2**32 - 1.

Meanwhile, the bitset can represent values between 0 and (2**32 - 1) * 64.

Right?

So idx<<log2WordSize might overflow.

It won't overflow in practice. But it is worth saying why. Or, at least, just say 'won't overflow in practice' as a comment.

@gaissmai
Copy link
Copy Markdown
Contributor Author

hm, yes, but this is not different to NextSet and NetxSetMany

@lemire
Copy link
Copy Markdown
Member

lemire commented Dec 16, 2024

hm, yes, but this is not different to NextSet and NetxSetMany

Correct. Let me merge this and I will add comments.

@lemire lemire merged commit d231880 into bits-and-blooms:master Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants