Add Vec::remove_insert()
#635
Conversation
src/vec/mod.rs
Outdated
| assert!(remove_index < length); | ||
| assert!(insert_index < length); | ||
|
|
||
| unsafe { |
There was a problem hiding this comment.
- Can we have multiple unsafe blocks with each having as little scope as possible?
- Please adds
Safety:comments above each use ofunsafe, briefly describing how the invariants are being held in the block.
There was a problem hiding this comment.
Pushed a commit that does this (unsquashed, in case I overdid the explanations).
|
Do you know why this is not also available in the standard library's Vec? This would actually not even need to be implemented on |
|
I added this as a local trait implemented for Feel free to do with it whatever you want. The implementation using rotates is slower as noted a few days ago. So, only the original impl is actually useful performance wise. |
I understand that you yourself no longer need this but please consider the time reviewers put into this as well. It seems to me like all that's left now is reverting to the original implementation ( |
In patterns where an insertion directly follows a removal, this serves as being more efficient, since it only shifts/copies values as needed for the combined operation. This also comes with the added bonus of not needing to check for fullness, and thus, a `Result` is not needed as a return type. Signed-off-by: Mohammad AlSaleh <CE.Mohammad.AlSaleh@gmail.com>
|
@zeenix |
|
I'm a bit baffled by the CI failure. I can't reproduce the warning locally. 🤔 |
|
This is a bug in 1.92 I think, you may not have the latest compiler. |
|
Given that this is a feature that can be implemented on slices, I'm not fully certain it makes sense to merge it here? I'd rather try to get it implemented upstream in |
|
Hmm, no this does not reproduce for me either |
|
This might be related: rust-lang/rust#147648 |
4 participants
In patterns where an insertion directly follows a removal, this serves
as being more efficient, since it only shifts/copies values as needed
for the combined operation.
This also comes with the added bonus of not needing to check for
fullness, and thus, a
Resultis not needed as a return type.