rust-lang / libs-team

The home of the library team
Apache License 2.0
124 stars 19 forks source link

Make Vec::push return a &mut reference to the pushed value #464

Closed balt-dev closed 2 hours ago

balt-dev commented 2 hours ago

Proposal

Problem statement

Getting a reference to the value you just pushed to a vector requires an extra call and unwrap:

let mut v = Vec::new();
v.push(42);
*v.last_mut().unwrap() = 43;

Motivating examples or use cases

I'm currently porting https://github.com/Naruyoko/OmegaNum.js/ to Rust, and this is a common pattern all over: x.array[i+1]=(x.array[i+1]||0)+1; This is directly translated as

if x.is_empty() {
    x.push(0);
}
let last_mut = x.last_mut().unwrap();
*last_mut += 1;

With this, this could be the following:

let last_mut = x.last_mut().unwrap_or(x.push(0));
*last_mut += 1;

which is much nicer.

Solution sketch

Change Vec::push to return a reference to the pushed value:

pub fn push(&mut self, value: T) -> &mut T {
    let len = self.len;
    if len == self.buf.capacity() {
        self.buf.grow_one();
    }
    unsafe {
        let end = self.as_mut_ptr().add(len);
        ptr::write(end, value);
        self.len = len + 1;
        &mut *end
    }
}

Alternatives

Alternatively, Vec::last_or_push(default: T) -> &T, along with 3 other variations for mutability and laziness, could make the above translate in only one line - maybe it would be better, but both would be nice. See issue #465.

balt-dev commented 2 hours ago

I'm willing to make a PR with this if everyone's okay with these changes as-is.

balt-dev commented 2 hours ago

Closing in favor of #465

scottmcm commented 1 hour ago

I think this is 100% more correct, but sadly it's a breaking change today.

I'd love for someone to come up with enough language features that it would no longer be breaking!

balt-dev commented 1 hour ago

What do you mean? No matter what, this - changing the return type of a function - is always a breaking change.

scottmcm commented 1 hour ago

That's true of Rust today, but maybe in a future Rust it wouldn't need to be.

Imagine if you could coerce a fn(&mut Vec<T>, T) -> &mut T to fn(&mut Vec<T>, T) -> (), for example -- that'd remove one way that this a change like this is breaking. Then it'd "just" be a matter of doing changes like that for all the reasons that something might be breaking.