Skip to content

Conversation

BenjaminBrienen
Copy link
Contributor

@BenjaminBrienen BenjaminBrienen commented Sep 7, 2025

if index < my_vector.len() { Some(my_vector.remove(index)) } else { None } is annoying to write and non-panicking functions are broadly useful.

APC: rust-lang/libs-team#649

Tracking issue: #146954

@rustbot
Copy link
Collaborator

rustbot commented Sep 7, 2025

r? @workingjubilee

rustbot has assigned @workingjubilee.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot 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 Sep 7, 2025
@BenjaminBrienen BenjaminBrienen changed the title feat: non-panicking Vec::try_remove feat: non-panicking Vec::try_remove Sep 7, 2025
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@BenjaminBrienen
Copy link
Contributor Author

Looks like I inadvertently repeated #77480

I'd still like to reopen the discussion about this.

Copy link
Contributor

@folkertdev folkertdev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additions to the standard library APIs will need an ACP first, see https://github.com/rust-lang/libs-team/issues > new issue > Api Change Proposal

View changes since this review

@rustbot

This comment has been minimized.

@BenjaminBrienen BenjaminBrienen force-pushed the try_remove branch 2 times, most recently from 5bf4f7d to c2ffa8a Compare September 7, 2025 11:19
@BenjaminBrienen
Copy link
Contributor Author

BenjaminBrienen commented Sep 7, 2025

Additions to the standard library APIs will need an ACP first, see https://github.com/rust-lang/libs-team/issues > new issue > Api Change Proposal

View changes since this review

Added the tracking issue to the description. Is that sufficient?

Edit: added an ACP

@tgross35 tgross35 added the S-waiting-on-ACP Status: PR has an ACP and is waiting for the ACP to complete. label Sep 7, 2025
@workingjubilee
Copy link
Member

r? libs

Copy link
Member

@joboet joboet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a small nit, but after that this looks good to go.

View changes since this review

@joboet joboet removed the S-waiting-on-ACP Status: PR has an ACP and is waiting for the ACP to complete. label Sep 24, 2025
@joboet joboet assigned joboet and unassigned ibraheemdev Sep 24, 2025
@joboet
Copy link
Member

joboet commented Sep 24, 2025

Oh, and could you squash the commits, please?

@rustbot
Copy link
Collaborator

rustbot commented Sep 24, 2025

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

Vec::try_remove is a non-panicking version of Vec::remove
@joboet
Copy link
Member

joboet commented Sep 25, 2025

Great, thank you!
@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Sep 25, 2025

📌 Commit 5673449 has been approved by joboet

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 25, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Sep 25, 2025
feat: non-panicking `Vec::try_remove`

`if index < my_vector.len() { Some(my_vector.remove(index)) } else { None }` is annoying to write and non-panicking functions are broadly useful.

APC: rust-lang/libs-team#649

Tracking issue: rust-lang#146954
bors added a commit that referenced this pull request Sep 25, 2025
Rollup of 8 pull requests

Successful merges:

 - #116882 (rustdoc: hide `#[repr]` if it isn't part of the public ABI)
 - #135771 ([rustdoc] Add support for associated items in "jump to def" feature)
 - #141032 (avoid violating `slice::from_raw_parts` safety contract in `Vec::extract_if`)
 - #142401 (Add proper name mangling for pattern types)
 - #146293 (feat: non-panicking `Vec::try_remove`)
 - #146859 (BTreeMap: Don't leak allocators when initializing nodes)
 - #146924 (Add doc for `NonZero*` const creation)
 - #146933 (Make `render_example_with_highlighting` return an `impl fmt::Display`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit fea9196 into rust-lang:master Sep 25, 2025
10 checks passed
@rustbot rustbot added this to the 1.92.0 milestone Sep 25, 2025
rust-timer added a commit that referenced this pull request Sep 25, 2025
Rollup merge of #146293 - BenjaminBrienen:try_remove, r=joboet

feat: non-panicking `Vec::try_remove`

`if index < my_vector.len() { Some(my_vector.remove(index)) } else { None }` is annoying to write and non-panicking functions are broadly useful.

APC: rust-lang/libs-team#649

Tracking issue: #146954
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Sep 26, 2025
Rollup of 8 pull requests

Successful merges:

 - rust-lang/rust#116882 (rustdoc: hide `#[repr]` if it isn't part of the public ABI)
 - rust-lang/rust#135771 ([rustdoc] Add support for associated items in "jump to def" feature)
 - rust-lang/rust#141032 (avoid violating `slice::from_raw_parts` safety contract in `Vec::extract_if`)
 - rust-lang/rust#142401 (Add proper name mangling for pattern types)
 - rust-lang/rust#146293 (feat: non-panicking `Vec::try_remove`)
 - rust-lang/rust#146859 (BTreeMap: Don't leak allocators when initializing nodes)
 - rust-lang/rust#146924 (Add doc for `NonZero*` const creation)
 - rust-lang/rust#146933 (Make `render_example_with_highlighting` return an `impl fmt::Display`)

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants