Skip to content

Conversation

Zalathar
Copy link
Contributor

One of the discussion points that came up in #119917 is that some people use Vec::split_off in cases where they probably shouldn't, because the alternatives (like mem::take) are hard to discover.

This PR adds some suggestions to the documentation of split_off that should point people towards alternatives that might be more appropriate for their use-case.

I've deliberately tried to keep these changes as simple and uncontroversial as possible, so that they don't depend on how the team decides to handle the concerns raised in #119917. That's why I haven't touched the existing documentation for split_off, and haven't added links to split_off to the documentation of other methods.

@rustbot
Copy link
Collaborator

rustbot commented Jan 21, 2024

r? @Mark-Simulacrum

(rustbot has picked a reviewer for you, use r? to override)

@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 Jan 21, 2024
@dtolnay dtolnay assigned dtolnay and unassigned Mark-Simulacrum Jan 21, 2024
Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

Nice! Thank you.

This is conveniently independent of which way #119917 goes.

@dtolnay
Copy link
Member

dtolnay commented Jan 21, 2024

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Jan 21, 2024

📌 Commit 6f1944d has been approved by dtolnay

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 Jan 21, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 21, 2024
…s, r=dtolnay

Document some alternatives to `Vec::split_off`

One of the discussion points that came up in rust-lang#119917 is that some people use `Vec::split_off` in cases where they probably shouldn't, because the alternatives (like `mem::take`) are hard to discover.

This PR adds some suggestions to the documentation of `split_off` that should point people towards alternatives that might be more appropriate for their use-case.

I've deliberately tried to keep these changes as simple and uncontroversial as possible, so that they don't depend on how the team decides to handle the concerns raised in rust-lang#119917. That's why I haven't touched the existing documentation for `split_off`, and haven't added links to `split_off` to the documentation of other methods.
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 21, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#120005 (Update Readme)
 - rust-lang#120045 (Un-hide `iter::repeat_n`)
 - rust-lang#120128 (Make stable_mir::with_tables sound)
 - rust-lang#120145 (fix: Drop guard was deallocating with the incorrect size)
 - rust-lang#120158 (`rustc_mir_dataflow`: Restore removed exports)
 - rust-lang#120167 (Capture the rationale for `-Zallow-features=` in bootstrap.py)
 - rust-lang#120174 (Warn users about limited review for tier 2 and 3 code)
 - rust-lang#120180 (Document some alternatives to `Vec::split_off`)

Failed merges:

 - rust-lang#120171 (Fix assume and assert in jump threading)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 21, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#120005 (Update Readme)
 - rust-lang#120045 (Un-hide `iter::repeat_n`)
 - rust-lang#120128 (Make stable_mir::with_tables sound)
 - rust-lang#120145 (fix: Drop guard was deallocating with the incorrect size)
 - rust-lang#120158 (`rustc_mir_dataflow`: Restore removed exports)
 - rust-lang#120167 (Capture the rationale for `-Zallow-features=` in bootstrap.py)
 - rust-lang#120174 (Warn users about limited review for tier 2 and 3 code)
 - rust-lang#120180 (Document some alternatives to `Vec::split_off`)

Failed merges:

 - rust-lang#120171 (Fix assume and assert in jump threading)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 21, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#120005 (Update Readme)
 - rust-lang#120045 (Un-hide `iter::repeat_n`)
 - rust-lang#120128 (Make stable_mir::with_tables sound)
 - rust-lang#120145 (fix: Drop guard was deallocating with the incorrect size)
 - rust-lang#120158 (`rustc_mir_dataflow`: Restore removed exports)
 - rust-lang#120167 (Capture the rationale for `-Zallow-features=` in bootstrap.py)
 - rust-lang#120174 (Warn users about limited review for tier 2 and 3 code)
 - rust-lang#120180 (Document some alternatives to `Vec::split_off`)

Failed merges:

 - rust-lang#120171 (Fix assume and assert in jump threading)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 21, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#120005 (Update Readme)
 - rust-lang#120045 (Un-hide `iter::repeat_n`)
 - rust-lang#120128 (Make stable_mir::with_tables sound)
 - rust-lang#120145 (fix: Drop guard was deallocating with the incorrect size)
 - rust-lang#120158 (`rustc_mir_dataflow`: Restore removed exports)
 - rust-lang#120167 (Capture the rationale for `-Zallow-features=` in bootstrap.py)
 - rust-lang#120174 (Warn users about limited review for tier 2 and 3 code)
 - rust-lang#120180 (Document some alternatives to `Vec::split_off`)

Failed merges:

 - rust-lang#120171 (Fix assume and assert in jump threading)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 22, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#120005 (Update Readme)
 - rust-lang#120045 (Un-hide `iter::repeat_n`)
 - rust-lang#120128 (Make stable_mir::with_tables sound)
 - rust-lang#120145 (fix: Drop guard was deallocating with the incorrect size)
 - rust-lang#120158 (`rustc_mir_dataflow`: Restore removed exports)
 - rust-lang#120167 (Capture the rationale for `-Zallow-features=` in bootstrap.py)
 - rust-lang#120174 (Warn users about limited review for tier 2 and 3 code)
 - rust-lang#120180 (Document some alternatives to `Vec::split_off`)

Failed merges:

 - rust-lang#120171 (Fix assume and assert in jump threading)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 3eb7fe3 into rust-lang:master Jan 22, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 22, 2024
Rollup merge of rust-lang#120180 - Zalathar:vec-split-off-alternatives, r=dtolnay

Document some alternatives to `Vec::split_off`

One of the discussion points that came up in rust-lang#119917 is that some people use `Vec::split_off` in cases where they probably shouldn't, because the alternatives (like `mem::take`) are hard to discover.

This PR adds some suggestions to the documentation of `split_off` that should point people towards alternatives that might be more appropriate for their use-case.

I've deliberately tried to keep these changes as simple and uncontroversial as possible, so that they don't depend on how the team decides to handle the concerns raised in rust-lang#119917. That's why I haven't touched the existing documentation for `split_off`, and haven't added links to `split_off` to the documentation of other methods.
@rustbot rustbot added this to the 1.77.0 milestone Jan 22, 2024
@Zalathar Zalathar deleted the vec-split-off-alternatives branch January 22, 2024 11:21
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.

5 participants