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

pushed for returning a mutable reference to the pushed element. #1821

Open
ticki opened this issue Dec 18, 2016 · 10 comments
Open

pushed for returning a mutable reference to the pushed element. #1821

ticki opened this issue Dec 18, 2016 · 10 comments
Labels
T-libs-api Relevant to the library API team, which will review and decide on the RFC.

Comments

@ticki
Copy link
Contributor

ticki commented Dec 18, 2016

It is annoying to manually fetch the last element, which is unsurprisingly quite common after a push. The most obvious thing would be to make Vec::<T>::push() return a &mut T, but this is strictly speaking a breaking change. It would be cool to get some crater data on whether this in practice is an issue (i.e. if people don't put the ; after push() in functions returning ()).

The more realistic change is to add a method, push_ref, returning &mut T.

@KalitaAlexey
Copy link

Hi @ticki,

Why not push_mut_ref according to conventions?

@comex
Copy link

comex commented Dec 18, 2016

I'd really like this too, but bikeshed: I don't like the name push_ref; it sounds like you're pushing a reference. Maybe pushed_ref?

@ticki
Copy link
Contributor Author

ticki commented Dec 18, 2016

@KalitaAlexey Two reasons:

  1. The actual idiom is to use push_mut (like AsMut::as_mut).

  2. Using _mut makes no sense when there is no non-mutuable alternative.

@ticki
Copy link
Contributor Author

ticki commented Dec 18, 2016

@comex what about pushed()? It's simpler and nicer IMO.

@comex
Copy link

comex commented Dec 18, 2016

@ticki Sure.

@nagisa
Copy link
Member

nagisa commented Dec 18, 2016

The emplacement implementation could be made to return the reference to an element that has been just emplaced. I wonder, though, which is more useful – returning the index of the element or the reference to the element itself.

@ticki
Copy link
Contributor Author

ticki commented Dec 18, 2016

@nagisa

Returning a reference allows working around the implicit boundcheck, and honestly, getting the index isn't that hard (it's free too).

@ticki ticki changed the title push_ref for returning a mutable reference to the pushed element. pushed for returning a mutable reference to the pushed element. Dec 18, 2016
@eddyb
Copy link
Member

eddyb commented Dec 18, 2016

IIRC, v.back() <- elem was supposed to produce a &mut T to the element.

@nrc nrc added the T-libs-api Relevant to the library API team, which will review and decide on the RFC. label Dec 19, 2016
@fiveseven-lambda
Copy link

Making Vec::push() return a &mut T will be a breaking change; it is natural that someone use push() in match arm like:

match something() {
    0 => println!("zero"),
    value => vec.push(value),
}

@JohnScience
Copy link

You can use push_mut for now. It feels like a left-pad and thing belonging to the standard library but anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-libs-api Relevant to the library API team, which will review and decide on the RFC.
Projects
None yet
Development

No branches or pull requests

8 participants