Skip to content

refactor(cli/rustup-mode): avoid redundant allocs in target_add() - #5054

Merged
rami3l merged 3 commits into
rust-lang:mainfrom
rami3l:refactor/target-add-allocs
Sep 7, 2026
Merged

refactor(cli/rustup-mode): avoid redundant allocs in target_add()#5054
rami3l merged 3 commits into
rust-lang:mainfrom
rami3l:refactor/target-add-allocs

Conversation

@rami3l

@rami3l rami3l commented Sep 4, 2026

Copy link
Copy Markdown
Member

Follow-up on a minor pre-existing issue discovered when reviewing #5042.

This patch avoids an extra .to_string() allocation and a redundant into_iter() -> collect() -> into_iter() round trip.

@rustbot

This comment has been minimized.

@rami3l
rami3l force-pushed the refactor/target-add-allocs branch 2 times, most recently from e699869 to 83f5d81 Compare September 4, 2026 14:21
@rami3l
rami3l requested a review from djc September 4, 2026 14:23
@rami3l
rami3l marked this pull request as ready for review September 4, 2026 14:23
@rami3l
rami3l force-pushed the refactor/target-add-allocs branch 3 times, most recently from 4b61a97 to cb9dc23 Compare September 4, 2026 14:48
Comment thread src/cli/rustup_mode.rs Outdated
@rami3l
rami3l force-pushed the refactor/target-add-allocs branch 6 times, most recently from 00055a8 to d8fd968 Compare September 4, 2026 15:24
@rami3l
rami3l force-pushed the refactor/target-add-allocs branch from d8fd968 to 81044d6 Compare September 7, 2026 08:59
@rustbot

rustbot commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

This PR was rebased onto a different main 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.

@djc djc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice, this seems much better.

The one other thing I played with was adding a Component::std(target) constructor wrapping Component::new("rust-std".to_string(), Some(target), false).

View changes since this review

Comment thread src/toolchain/distributable.rs
@rami3l

rami3l commented Sep 7, 2026

Copy link
Copy Markdown
Member Author

The one other thing I played with was adding a Component::std(target) constructor wrapping Component::new("rust-std".to_string(), Some(target), false).

@djc Sounds pretty much like a systematic replacement. I can do this as a follow-up.

@rami3l
rami3l enabled auto-merge September 7, 2026 09:20
@rami3l
rami3l added this pull request to the merge queue Sep 7, 2026
Merged via the queue into rust-lang:main with commit 17e3b80 Sep 7, 2026
31 checks passed
@rami3l
rami3l deleted the refactor/target-add-allocs branch September 7, 2026 09:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants