Skip to content

Add Vec<T> into Arc/Rc<[T]> lint#16790

Open
landaire wants to merge 9 commits into
rust-lang:masterfrom
landaire-contrib:push-ywunxyxovqyn
Open

Add Vec<T> into Arc/Rc<[T]> lint#16790
landaire wants to merge 9 commits into
rust-lang:masterfrom
landaire-contrib:push-ywunxyxovqyn

Conversation

@landaire
Copy link
Copy Markdown

@landaire landaire commented Apr 2, 2026

View all comments

  • Followed [lint naming conventions][lint_naming]
  • Added passing UI tests (including committed .stderr file)
  • cargo test passes locally
  • Executed cargo dev update_lints
  • Added lint documentation
  • Run cargo dev fmt

Adds a lint detecting conversions from Vec<T> into Arc<[T]> and Rc<[T]>. When doing this conversion, the source Vec<T> allocation cannot be re-used because of incompatible layout with Arc<[T]> and Rc<[T]>, which may be a fairly expensive reallocation depending on the source vector's size.

In most cases, it's probably better to convert to an Arc<Box<[T]>> instead.

See discussion here for rationale: https://users.rust-lang.org/t/could-arc-t-from-vec-t-be-optimized-to-remove-the-copy/137532

changelog: [vec_to_rc_slice]: add perf lint for converting from Vec<[T]> to Arc<[T]>/Rc<[T]>

@rustbot rustbot added needs-fcp PRs that add, remove, or rename lints and need an FCP S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Apr 2, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 2, 2026

r? @dswij

rustbot has assigned @dswij.
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

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: 7 candidates
  • 7 candidates expanded to 7 candidates
  • Random selection from Jarcho, dswij, llogiq, samueltardieu

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 2, 2026

Lintcheck changes for a6eec8f

Lint Added Removed Changed
clippy::vec_to_rc_slice 4 0 0

This comment will be updated if you push new changes

@landaire landaire force-pushed the push-ywunxyxovqyn branch from 7e67b53 to fac19f9 Compare April 2, 2026 06:16
Comment thread CHANGELOG.md
@ada4a
Copy link
Copy Markdown
Contributor

ada4a commented Apr 5, 2026

the changelog message doesn't need to be enclosed in a code block:) I think the default PR comment is confusing people into thinking that, because you're not the first one to have done this^^

Copy link
Copy Markdown
Contributor

@ada4a ada4a left a comment

Choose a reason for hiding this comment

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

This is already looking very good -- just a few comments

View changes since this review

Comment thread clippy_lints/src/vec_to_rc_slice.rs Outdated
Comment thread clippy_lints/src/vec_to_rc_slice.rs Outdated
Comment thread clippy_lints/src/vec_to_rc_slice.rs Outdated
Comment thread clippy_lints/src/vec_to_rc_slice.rs Outdated
@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Apr 5, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 5, 2026

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@landaire landaire force-pushed the push-ywunxyxovqyn branch from fac19f9 to 5d5596e Compare April 6, 2026 17:38
@landaire
Copy link
Copy Markdown
Author

landaire commented Apr 6, 2026

@rustbot ready

I use jj and was trying to avoid force-pushing to preserve the original review but realized a bit too late I was applying changes on top of the existing commit rather than a new one. My apologies.

@rustbot 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 from the author. (Use `@rustbot ready` to update this status) labels Apr 6, 2026
@ada4a
Copy link
Copy Markdown
Contributor

ada4a commented Apr 6, 2026

I use jj and was trying to avoid force-pushing to preserve the original review but realized a bit too late I was applying changes on top of the existing commit rather than a new one. My apologies.

Hello fellow jj user:) I mean force-pushing is usually okay if you're only addressing smaller comments, but yeah in this case a separate commit would've been nice. Oh well^^

@ada4a
Copy link
Copy Markdown
Contributor

ada4a commented Apr 6, 2026

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Apr 6, 2026
@landaire
Copy link
Copy Markdown
Author

landaire commented Apr 6, 2026

@rustbot ready

@rustbot 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 from the author. (Use `@rustbot ready` to update this status) labels Apr 6, 2026
Comment thread tests/ui/vec_to_rc_slice.stderr Outdated
Comment thread clippy_lints/src/vec_to_rc_slice.rs Outdated
@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Apr 6, 2026
@rustbot

This comment has been minimized.

@landaire
Copy link
Copy Markdown
Author

@rustbot ready

@ada4a I ended up not rolling back the ascryption change. I think you're right it's a losing battle but the context I have it in working with atm is probably still useful. Sorry for the delay on this.

I also noted the conflict -- I'll wait until this is approved to avoid the force push.

@rustbot rustbot removed the S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) label Apr 14, 2026
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Apr 14, 2026
@ada4a
Copy link
Copy Markdown
Contributor

ada4a commented Apr 19, 2026

I'll wait until this is approved to avoid the force push.

Oh don't worry, that should be fine -- when you do a rebase, rustbot shows this nice range-diff thing, which hides the "all the new stuff from main" part of the changes and allows concentrating on the "the things that the rebase changed in this PR" part.

Sorry for the delay on this.

That's no problem either:) As you can see, I'm not perfect myself^^ In general you're more likely to need to wait for a reviewer than vice versa...

Adds a lint detecting conversions from `Vec<T>` into `Arc<[T]>` and
`Rc<[T]>`. When doing this conversion, the source `Vec<T>` allocation
cannot be re-used because of incompatible layout with `Arc<[T]>` and
`Rc<[T]>`, which may be a fairly expensive reallocation depending on the
source vector's size.

In most cases, it's probably better to convert to an `Arc<Box<[T]>>`
instead.

See discussion here for rationale: https://users.rust-lang.org/t/could-arc-t-from-vec-t-be-optimized-to-remove-the-copy/137532
Comment thread clippy_lints/src/vec_to_rc_slice.rs Outdated
Comment thread clippy_lints/src/vec_to_rc_slice.rs Outdated
Comment thread clippy_lints/src/vec_to_rc_slice.rs Outdated
Comment thread clippy_lints/src/vec_to_rc_slice.rs
@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Apr 19, 2026
@landaire landaire force-pushed the push-ywunxyxovqyn branch from e79c36a to 41904d2 Compare April 19, 2026 18:19
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 19, 2026

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.

Comment thread clippy_lints/src/vec_to_rc_slice.rs Outdated
@landaire
Copy link
Copy Markdown
Author

@rustbot ready

@rustbot 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 from the author. (Use `@rustbot ready` to update this status) labels Apr 20, 2026
@ada4a ada4a added lint-nominated Create an FCP-thread on Zulip for this PR and removed needs-fcp PRs that add, remove, or rename lints and need an FCP labels Apr 20, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 20, 2026

This lint has been nominated for inclusion.

A FCP topic has been created on Zulip.

@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 24, 2026

☔ The latest upstream changes (possibly #16025) made this pull request unmergeable. Please resolve the merge conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lint-nominated Create an FCP-thread on Zulip for this PR S-waiting-on-review Status: Awaiting review from the assignee but also interested parties

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants