rustonaut / vec1

Rust `Vec<T>` wrapper that gurantees to contain at least 1 element
Apache License 2.0
90 stars 15 forks source link

`Vec1::drain` can empty a `Vec1`. #36

Open olson-sean-k opened 2 months ago

olson-sean-k commented 2 months ago

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 commented 1 month ago

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 :shrug: