Implement push_mut
#135975
Implement push_mut
#135975
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ibraheemdev (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
rustbot
added
S-waiting-on-review
|
Sorry if I did this wrong, it's my first time! |
This comment has been minimized.
This comment has been minimized.
|
A single trailing whitespace. Oops. |
This comment has been minimized.
This comment has been minimized.
|
I'm not sure we should be touching |
|
Alright, I'll do that soon. |
ibraheemdev
added
S-waiting-on-author
|
I finally got around to duplicating the implementation. College has been eating my time like a child with Halloween candy, sorry. |
|
@rustbot review |
rustbot
added
S-waiting-on-review
| /// Takes *O*(1) time. | ||
| #[inline] | ||
| #[unstable(feature = "push_mut", issue = "135974")] | ||
| // #[unstable(feature = "vec_push_within_capacity", issue = "100486")] |
There was a problem hiding this comment.
Should this comment be removed?
There was a problem hiding this comment.
Well, both features apply, so I wasn't quite sure what to do. If push_mut is stabilized before vec_push_within_capacity, this should still be unstable, and same for the reverse.
library/alloc/src/vec/mod.rs
Outdated
| /// | ||
| /// let mut vec = vec![]; | ||
| /// // Due to current borrow checker limitations (see -Zpolonius), this is the only way to spell this right now. | ||
| /// let last = if let Some(v) = vec.last_mut() { v } else { vec.push_mut(0) }; |
There was a problem hiding this comment.
I'm not sure I understand the point of this.
There was a problem hiding this comment.
There should probably be a better example, yeah. Will update soon!
rustbot
added
S-waiting-on-author
|
@balt-dev |
|
I'm sorry, college has been quite a time sink for me and I haven't had the time to follow up as of recent - I will when I can, promise! |
|
It's been a rough few months. ._. |
This comment has been minimized.
This comment has been minimized.
|
hm. |
|
I think I messed up my fork so bad I'll have to reset it and put my code in from scratch in order to get rid of the merge commit. At least it's copy/pasteable? |
|
What the hell is going on!? |
|
For the record: |
src/tools/cargo
Outdated
There was a problem hiding this comment.
I have no idea why this happened and no idea how to revert it. I don't even know if I should revert it. Could someone advise?
This comment has been minimized.
This comment has been minimized.
|
@rustbot review I need help fixing this mess. |
rustbot
added
the
S-waiting-on-review
|
I've fixed your branch so as to not touch any submodules. |
|
Thank you!! |
|
Should rust-lang/libs-team#579 be lumped in with this implementation? |
|
Yes, I'd expect that should be OK. (It can always be broken out later if the reviewer prefers.) |
| /// #![feature(push_mut)] | ||
| /// | ||
| /// # #[allow(unused)] | ||
| /// #[derive(PartialEq, Eq, Debug)] | ||
| /// struct Item { identifier: &'static str, count: usize } | ||
| /// | ||
| /// impl Default for Item { | ||
| /// fn default() -> Self { | ||
| /// return Self { identifier: "stone", count: 64 } | ||
| /// } | ||
| /// } | ||
| /// | ||
| /// let mut items = vec![]; | ||
| /// | ||
| /// // We can mutate the just-pushed value without having to fetch it again | ||
| /// for count in [15, 35, 61] { | ||
| /// let item = items.push_mut(Item::default()); | ||
| /// item.count = count; | ||
| /// } | ||
| /// | ||
| /// assert_eq!( | ||
| /// items, | ||
| /// [Item { identifier: "stone", count: 15 }, Item { identifier: "stone", count: 35 }, Item { identifier: "stone", count: 61 }] | ||
| /// ); |
There was a problem hiding this comment.
This example is quite long. Documentation examples should generally be short and to the point.
| /// #![feature(push_mut)] | |
| /// | |
| /// # #[allow(unused)] | |
| /// #[derive(PartialEq, Eq, Debug)] | |
| /// struct Item { identifier: &'static str, count: usize } | |
| /// | |
| /// impl Default for Item { | |
| /// fn default() -> Self { | |
| /// return Self { identifier: "stone", count: 64 } | |
| /// } | |
| /// } | |
| /// | |
| /// let mut items = vec![]; | |
| /// | |
| /// // We can mutate the just-pushed value without having to fetch it again | |
| /// for count in [15, 35, 61] { | |
| /// let item = items.push_mut(Item::default()); | |
| /// item.count = count; | |
| /// } | |
| /// | |
| /// assert_eq!( | |
| /// items, | |
| /// [Item { identifier: "stone", count: 15 }, Item { identifier: "stone", count: 35 }, Item { identifier: "stone", count: 61 }] | |
| /// ); | |
| /// #![feature(push_mut)] | |
| /// | |
| /// let mut vec = vec![1, 2]; | |
| /// let last = vec.push(3); | |
| /// assert_eq!(vec, [1, 2, 3]); | |
| /// *last = 4; | |
| /// assert_eq!(vec, [1, 2, 4]); | |
| /// ``` |
|
I'm happy to review |
ibraheemdev
added
S-waiting-on-author
|
|
...again? |
|
Sigh.... |
|
@traviscross How'd you fix the submodules? |
|
|
|
The job Click to see the possible cause of the failure (guessed by this bot) |
The easiest way is something like: # Make sure you have rust-lang's `master` available
git remote add upstream https://github.com/rust-lang/rust.git
git fetch upstream master
# Do a soft reset to the point where you forked off from master.
# Mild warning: this will keep the diff but get rid of the new
# commits (the files will show up as having unstaged changes)
git reset "$(git merge-base HEAD upstream/master)"
# Just reset submodules
git submodule deinit --all --force
# Add the changes you actually want and create a new commit
git add library
git commit
# Update this PR, which will replace the three commits here with
# the one new commit
git push --force-with-lease
For future reference, it's usually better practice to always work on branches rather than your fork's |
Implementation of #135974.