Implement `push_mut` #135975

balt-dev · 2025-01-24T05:43:00Z

Implementation of #135974.

rustbot · 2025-01-24T05:43:08Z

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 (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

@rustbot author: the review is finished, PR author should check the comments and take action accordingly
@rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

balt-dev · 2025-01-24T05:44:26Z

Sorry if I did this wrong, it's my first time!

balt-dev · 2025-01-25T20:59:27Z

A single trailing whitespace. Oops.

ibraheemdev · 2025-01-25T22:56:36Z

I'm not sure we should be touching Vec::push here.. maybe just duplicate the implementation?

balt-dev · 2025-01-26T00:42:09Z

Alright, I'll do that soon.

balt-dev · 2025-02-17T22:24:29Z

I finally got around to duplicating the implementation. College has been eating my time like a child with Halloween candy, sorry.

library/alloc/src/vec/mod.rs

balt-dev · 2025-02-22T03:00:32Z

@rustbot review

library/alloc/src/vec/mod.rs

ibraheemdev · 2025-02-23T01:04:41Z

library/alloc/src/vec/mod.rs

+    ///
+    /// 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) };


I'm not sure I understand the point of this.

There should probably be a better example, yeah. Will update soon!

alex-semenyuk · 2025-04-06T10:35:10Z

@balt-dev
Thanks for your contribution
From wg-triage. Any updates on this PR?

balt-dev · 2025-04-07T12:23:41Z

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!

balt-dev · 2025-05-30T04:11:38Z

It's been a rough few months. ._.

balt-dev · 2025-05-31T05:01:25Z

hm.

balt-dev · 2025-05-31T05:05:46Z

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?

balt-dev · 2025-06-17T03:32:57Z

Now mind you, that's a MIR difference, so I'm not sure if it should be ignored or updated or what.

(cherry picked from commit b48ad8c)

balt-dev · 2025-06-17T04:31:03Z

Agh, is there not a way to change the branch you're pushing from?

tgross35 · 2025-06-17T04:31:52Z

Now mind you, that's a MIR difference, so I'm not sure if it should be ignored or updated or what.

Ah, yeah, those should be fine to update. ./x t tests/ui/stable-mir-print/basic_function.rs --bless should do it, or you can copy the diff from CI (sorry, it's a bit awkward to do if you're having trouble running locally)

balt-dev · 2025-06-17T04:32:35Z

God this commit history is a trainwreck. I am so sorry.

tgross35 · 2025-06-17T04:33:11Z

We request squashing before merge for a reason :) feel free to do that at any time too if it keeps things more organized for you.

balt-dev · 2025-06-17T05:05:31Z

./x t tests/ui/stable-mir-print/basic_function.rs --bless

I'd love to, but for some reason...

downloading https://static.rust-lang.org/dist/2025-05-26/rustc-beta-x86_64-unknown-linux-gnu.tar.xz                                                                                                                                                                                                                      
#######################################################################################################################                                                                                                                                                                                             39.0%
curl: (23) Failure writing output to destination, passed 7940 returned 311
Warning: Problem (retrying all errors). Will retry in 1 second. 3 retries left.
#######################################################################################################################                                                                                                                                                                                             39.1%
curl: (23) Failure writing output to destination, passed 7922 returned 331
Warning: Problem (retrying all errors). Will retry in 2 seconds. 2 retries left.
#######################################################################################################################                                                                                                                                                                                             39.0%
curl: (23) Failure writing output to destination, passed 8084 returned 3712
Warning: Problem (retrying all errors). Will retry in 4 seconds. 1 retry left.
#######################################################################################################################                                                                                                                                                                                             39.1%
curl: (23) Failure writing output to destination, passed 8192 returned 54

curl seems to hate me right now. Will try again later.

balt-dev · 2025-06-17T17:09:38Z

Figured it out - turns out my /tmp was full.

library/alloc/src/vec/mod.rs

balt-dev · 2025-06-17T17:11:18Z

library/alloc/src/vec/mod.rs

I've went ahead and done this.
Let me know if I should revert this change.

balt-dev · 2025-06-17T18:07:28Z

Thank goodness for that test :P
It was silently doing nothing instead of properly panicking. Added back the panic, tests should pass now.

balt-dev · 2025-06-17T18:53:50Z

Just did some assembly inspection, and got some interesting results:

Rust and assembly code blocks

#![crate_type = "lib"]
#![feature(push_mut)]

#[inline(never)]
pub fn push_and_get(vec: &mut Vec<i32>, value: i32) -> &mut i32 {
    vec.push(value);
    vec.last_mut().unwrap()
}

#[inline(never)]
pub fn push_and_unsafe_get(vec: &mut Vec<i32>, value: i32) -> &mut i32 {
    vec.push(value);
    unsafe { vec.last_mut().unwrap_unchecked() }
}

#[inline(never)]
pub fn push_mut(vec: &mut Vec<i32>, value: i32) -> &mut i32 {
    vec.push_mut(value)
}

sample::push_and_get:
	.cfi_startproc
	push	r15
	.cfi_def_cfa_offset 16
	push	r14
	.cfi_def_cfa_offset 24
	push	rbx
	.cfi_def_cfa_offset 32
	.cfi_offset rbx, -32
	.cfi_offset r14, -24
	.cfi_offset r15, -16
	mov	ebx, esi
	mov	r15, qword ptr [rdi + 16]
	cmp	r15, qword ptr [rdi]
	jne	.LBB2_2
	lea	rsi, [rip + .Lanon.b379c6d255dd21da763072d2c3ee59ab.1]
	mov	r14, rdi
	call	qword ptr [rip + alloc::raw_vec::RawVec<T,A>::grow_one@GOTPCREL]
	mov	rax, qword ptr [r14 + 8]
	mov	dword ptr [rax + 4*r15], ebx
	inc	r15
	mov	qword ptr [r14 + 16], r15
	jmp	.LBB2_3
.LBB2_2:
	mov	rax, qword ptr [rdi + 8]
	mov	dword ptr [rax + 4*r15], ebx
	inc	r15
	mov	qword ptr [rdi + 16], r15
	je	.LBB2_4
.LBB2_3:
	lea	rax, [rax + 4*r15]
	add	rax, -4
	pop	rbx
	.cfi_def_cfa_offset 24
	pop	r14
	.cfi_def_cfa_offset 16
	pop	r15
	.cfi_def_cfa_offset 8
	ret
.LBB2_4:
	.cfi_def_cfa_offset 32
	lea	rdi, [rip + .Lanon.b379c6d255dd21da763072d2c3ee59ab.2]
	call	qword ptr [rip + core::option::unwrap_failed@GOTPCREL]

sample::push_and_unsafe_get:
	.cfi_startproc
	push	rbp
	.cfi_def_cfa_offset 16
	push	r14
	.cfi_def_cfa_offset 24
	push	rbx
	.cfi_def_cfa_offset 32
	.cfi_offset rbx, -32
	.cfi_offset r14, -24
	.cfi_offset rbp, -16
	mov	ebp, esi
	mov	rbx, rdi
	mov	r14, qword ptr [rdi + 16]
	cmp	r14, qword ptr [rdi]
	jne	.LBB2_2
	lea	rsi, [rip + .Lanon.d8d47fb7bb295c1cef687eae3796c8a4.1]
	mov	rdi, rbx
	call	qword ptr [rip + alloc::raw_vec::RawVec<T,A>::grow_one@GOTPCREL]
.LBB2_2:
	mov	rax, qword ptr [rbx + 8]
	mov	dword ptr [rax + 4*r14], ebp
	lea	rcx, [r14 + 1]
	mov	qword ptr [rbx + 16], rcx
	lea	rax, [rax + 4*r14]
	pop	rbx
	.cfi_def_cfa_offset 24
	pop	r14
	.cfi_def_cfa_offset 16
	pop	rbp
	.cfi_def_cfa_offset 8
	ret

sample::push_mut:
	.cfi_startproc
	push	rbp
	.cfi_def_cfa_offset 16
	push	r14
	.cfi_def_cfa_offset 24
	push	rbx
	.cfi_def_cfa_offset 32
	.cfi_offset rbx, -32
	.cfi_offset r14, -24
	.cfi_offset rbp, -16
	mov	ebp, esi
	mov	rbx, rdi
	mov	r14, qword ptr [rdi + 16]
	cmp	r14, qword ptr [rdi]
	jne	.LBB3_2
	lea	rsi, [rip + .Lanon.b379c6d255dd21da763072d2c3ee59ab.3]
	mov	rdi, rbx
	call	qword ptr [rip + alloc::raw_vec::RawVec<T,A>::grow_one@GOTPCREL]
.LBB3_2:
	mov	rcx, qword ptr [rbx + 8]
	lea	rax, [rcx + 4*r14]
	mov	dword ptr [rcx + 4*r14], ebp
	inc	r14
	mov	qword ptr [rbx + 16], r14
	pop	rbx
	.cfi_def_cfa_offset 24
	pop	r14
	.cfi_def_cfa_offset 16
	pop	rbp
	.cfi_def_cfa_offset 8
	ret

Doing a .push(val) followed immediately by a .last_mut().unwrap_unchecked() optimizes to the exact same assembly on opt level 3, but the compiler cannot optimize away the unwrap in .last_mut().unwrap(), incurring a runtime cost.
This is exactly the kind of thing I wanted to add this for! This optimzation makes this not only a single call, but also abstracts away something that used to be unsafe into a safe function call.

balt-dev · 2025-06-18T01:12:37Z

@rustbot review

balt-dev · 2025-06-18T05:39:26Z

library/alloc/src/vec/mod.rs

-        fn assert_failed(index: usize, len: usize) -> ! {
-            panic!("insertion index (is {index}) should be <= len (is {len})");
-        }
+        assert!(self.insert_mut(index, element).is_some(), "index out of bounds");


Might be losing some optimization here. I'm not well-versed enough to know what the hell some of these attributes do.

rust-log-analyzer · 2025-06-19T02:43:46Z

The job x86_64-gnu-llvm-19 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

#19 exporting to docker image format
#19 sending tarball 26.8s done
#19 DONE 34.9s
##[endgroup]
Setting extra environment values for docker:  --env ENABLE_GCC_CODEGEN=1 --env GCC_EXEC_PREFIX=/usr/lib/gcc/
[CI_JOB_NAME=x86_64-gnu-llvm-19]
[CI_JOB_NAME=x86_64-gnu-llvm-19]
debug: `DISABLE_CI_RUSTC_IF_INCOMPATIBLE` configured.
---
sccache: Listening on address 127.0.0.1:4226
##[group]Configure the build
configure: processing command line
configure: 
configure: build.configure-args := ['--build=x86_64-unknown-linux-gnu', '--llvm-root=/usr/lib/llvm-19', '--enable-llvm-link-shared', '--set', 'rust.randomize-layout=true', '--set', 'rust.thin-lto-import-instr-limit=10', '--set', 'build.print-step-timings', '--enable-verbose-tests', '--set', 'build.metrics', '--enable-verbose-configure', '--enable-sccache', '--disable-manage-submodules', '--enable-locked-deps', '--enable-cargo-native-static', '--set', 'rust.codegen-units-std=1', '--set', 'dist.compression-profile=balanced', '--dist-compression-formats=xz', '--set', 'rust.lld=false', '--disable-dist-src', '--release-channel=nightly', '--enable-debug-assertions', '--enable-overflow-checks', '--enable-llvm-assertions', '--set', 'rust.verify-llvm-ir', '--set', 'rust.codegen-backends=llvm,cranelift,gcc', '--set', 'llvm.static-libstdcpp', '--set', 'gcc.download-ci-gcc=true', '--enable-new-symbol-mangling']
configure: build.build          := x86_64-unknown-linux-gnu
configure: target.x86_64-unknown-linux-gnu.llvm-config := /usr/lib/llvm-19/bin/llvm-config
configure: llvm.link-shared     := True
configure: rust.randomize-layout := True
configure: rust.thin-lto-import-instr-limit := 10
---
[RUSTC-TIMING] rustc_std_workspace_core test:false 0.049
error: redundant argument
    --> library/alloc/src/vec/mod.rs:2036:80
     |
2036 |             panic!("insertion index (is {index}) should be <= len (is {len})", index, self.len())
     |                                                                                ^^^^^--
     |                                                                                |
     |                                                                                help: this can be removed
     |
note: the formatting specifier is referencing the binding already
    --> library/alloc/src/vec/mod.rs:2036:42
     |
2036 |             panic!("insertion index (is {index}) should be <= len (is {len})", index, self.len())
     |                                          ^^^^^

error[E0425]: cannot find value `len` in this scope
    --> library/alloc/src/vec/mod.rs:2036:72
     |
2036 |             panic!("insertion index (is {index}) should be <= len (is {len})", index, self.len())
     |                                                                        ^^^
...
2830 |     pub const fn len(&self) -> usize {
     |                  --- a method by that name is available on `Self` here
     |
     = help: you might have meant to use the available field in a format string: `"{}", self.len`

[RUSTC-TIMING] core test:false 35.273
   Compiling cfg-if v1.0.1
[RUSTC-TIMING] cfg_if test:false 0.055
   Compiling adler2 v2.0.1

balt-dev · 2025-06-19T05:52:12Z

This is why I shouldn't program after 9 PM. Will fix when I wake up tomorrow.

rustbot assigned ibraheemdev Jan 24, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jan 24, 2025

balt-dev mentioned this pull request Jan 24, 2025

Tracking Issue for Vec::push_mut #135974

Open

3 tasks

This comment has been minimized.

Sign in to view

ibraheemdev added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 31, 2025

balt-dev commented Feb 17, 2025

View reviewed changes

library/alloc/src/vec/mod.rs Outdated Show resolved Hide resolved

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 22, 2025

ibraheemdev requested changes Feb 23, 2025

View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 23, 2025

This comment has been minimized.

Sign in to view

rustbot added the has-merge-commits PR has merge commits, merge with caution. label May 31, 2025

balt-dev closed this May 31, 2025

balt-dev force-pushed the master branch from 149aab8 to 7a7bcbb Compare May 31, 2025 05:06

This comment has been minimized.

Sign in to view

balt-dev and others added 2 commits June 16, 2025 23:23

Update mod.rs

cbd069e

(cherry picked from commit b48ad8c)

Ah screw it, I'm just copy/pasting this

3bf443c

balt-dev closed this Jun 17, 2025

balt-dev force-pushed the master branch from b48ad8c to 55d4364 Compare June 17, 2025 04:30

balt-dev reopened this Jun 17, 2025

balt-dev added 2 commits June 16, 2025 23:35

Add back new thing that got accidentally nuked

71e37a7

Deduplicate code (can be reverted if needed)

d44c088

This comment has been minimized.

Sign in to view

balt-dev commented Jun 17, 2025

View reviewed changes

Update tests

01ee4b7

This comment has been minimized.

Sign in to view

Add forgotten bounds check

fcfc17f

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 18, 2025

balt-dev commented Jun 18, 2025

View reviewed changes

Update mod.rs

a0c757f

Implement push_mut #135975

Are you sure you want to change the base?

Implement push_mut #135975

Conversation

balt-dev commented Jan 24, 2025 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Jan 24, 2025

Uh oh!

balt-dev commented Jan 24, 2025

Uh oh!

This comment has been minimized.

balt-dev commented Jan 25, 2025

Uh oh!

This comment has been minimized.

ibraheemdev commented Jan 25, 2025

Uh oh!

balt-dev commented Jan 26, 2025

Uh oh!

balt-dev commented Feb 17, 2025

Uh oh!

Uh oh!

balt-dev commented Feb 22, 2025

Uh oh!

Uh oh!

ibraheemdev Feb 23, 2025

Choose a reason for hiding this comment

Uh oh!

balt-dev Feb 25, 2025

Choose a reason for hiding this comment

Uh oh!

alex-semenyuk commented Apr 6, 2025

Uh oh!

balt-dev commented Apr 7, 2025

Uh oh!

balt-dev commented May 30, 2025

Uh oh!

This comment has been minimized.

balt-dev commented May 31, 2025

Uh oh!

balt-dev commented May 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

balt-dev commented Jun 17, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

balt-dev commented Jun 17, 2025

Uh oh!

tgross35 commented Jun 17, 2025

Uh oh!

balt-dev commented Jun 17, 2025

Uh oh!

tgross35 commented Jun 17, 2025

Uh oh!

This comment has been minimized.

balt-dev commented Jun 17, 2025

Uh oh!

balt-dev commented Jun 17, 2025

Uh oh!

Uh oh!

balt-dev Jun 17, 2025

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

balt-dev commented Jun 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

balt-dev commented Jun 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

balt-dev commented Jun 18, 2025

Uh oh!

balt-dev Jun 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rust-log-analyzer commented Jun 19, 2025

Uh oh!

balt-dev commented Jun 19, 2025 via email

Implement `push_mut` #135975

Implement `push_mut` #135975

balt-dev commented Jan 24, 2025 •

edited by rustbot

Loading

balt-dev commented May 31, 2025 •

edited

Loading

balt-dev commented Jun 17, 2025 •

edited

Loading

balt-dev commented Jun 17, 2025 •

edited

Loading

balt-dev Jun 18, 2025 •

edited

Loading