Implement append and split_off for BitSet (RFC 509)
#24934
Conversation
|
r? @brson (rust_highfive has picked a reviewer for you, use r? to override) |
jooert
force-pushed
the
bitset-append-split_off
branch
from
f3667ec to
9b69d47
Compare
|
Renamed the feature gate using the same scheme as in #24890. |
|
Woah! I totally missed this. Thanks for the reminder! |
| if at == 0 { | ||
| swap(self, &mut other); | ||
| return other; | ||
| } else if at > self.bit_vec.len() { |
There was a problem hiding this comment.
Same issue as the other PR; this should be a panic. == has this behaviour, though.
There was a problem hiding this comment.
I thought a set is more like a map than a vector and VecMap doesn't panic either if at is behind the last element.
There was a problem hiding this comment.
I mean, we also don't panic if you try to access a non-existant element of a set -- it's just not there. But I don't know what semantics we want here so I'm fine with both.
There was a problem hiding this comment.
Oh, good point! This check should still be >=?
|
review done |
jooert
force-pushed
the
bitset-append-split_off
branch
from
9b69d47 to
ef2e116
Compare
|
Addressed your comments up to the behaviour of |
| // Pad `other` with `w` zero blocks, | ||
| // append `self`'s blocks in the range from `w` to the end to `other` | ||
| other.bit_vec.storage.extend(repeat(0u32).take(w) | ||
| .chain(self.bit_vec.storage[w..].iter().map(|&el| el))); |
There was a problem hiding this comment.
this map can be replaced with .cloned().
There was a problem hiding this comment.
Ah, great, this map seemed rather odd when I was writing it.
jooert
force-pushed
the
bitset-append-split_off
branch
from
ef2e116 to
f95c812
Compare
|
I fixed the last remaining points, thanks for your review again! |
|
@bors r+ Thanks! |
|
📌 Commit f95c812 has been approved by |
|
⌛ Testing commit f95c812 with merge fc83c55... |
|
⛄ The build was interrupted to prioritize another pull request. |
|
⌛ Testing commit f95c812 with merge 895ff4f... |
|
⛄ The build was interrupted to prioritize another pull request. |
|
⌛ Testing commit f95c812 with merge 3d425b5... |
|
⛄ The build was interrupted to prioritize another pull request. |
|
⌛ Testing commit f95c812 with merge bef0b4b... |
cc #19986