Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vec1::drain can empty a Vec1. #36

Open
olson-sean-k opened this issue Aug 20, 2024 · 1 comment
Open

Vec1::drain can empty a Vec1. #36

olson-sean-k opened this issue Aug 20, 2024 · 1 comment

Comments

@olson-sean-k
Copy link

Vec1 does not uphold its non-empty guarantee in its Vec1::drain API. Here's an example:

use std::mem;
use vec1::vec1;

fn main() {
    let mut xs1 = vec1![0i32, 1, 2, 3];
    println!("xs1 = {:?}", xs1.as_slice());
    let rtail = xs1.drain(..3).expect("range is not a strict subset");
    mem::forget(rtail);
    println!("xs1 = {:?}", xs1.as_slice());
}

This examples prints:

xs1 = [0, 1, 2, 3]
xs1 = []

The API is still sound, because vec1 does not use unsafe code and Vec1 APIs will panic if a Vec1 is empty. 👍🏽 This is a niche situation and I think it's reasonable to accept this behavior as is for the vec1 crate! For what it's worth though, I believe this can be avoided by taking advantage of the non-empty guarantee and swapping items when the drain range is a prefix. See the SwapDrainSegment implementation in the mitsein crate for an example (and the Vec::drain documentation).

@rustonaut
Copy link
Owner

thanks for the feedback, just writing to tell you I did read the issue (the day you opened it) I just haven't found any time to work on it

I originally tried to avoid writing a custom drain as it either comes with performance penalties or unsafe code. Through I guess for use cases where the perf. penalty matters there is anyway reason not to use vec1 🤷

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

No branches or pull requests

2 participants