Avoid some interning in bootstrap #121567
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
rustbot has assigned @albertlarsan68. Use r? to explicitly pick a reviewer |
|
This PR modifies If appropriate, please update |
rustbot
added
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
Noratrieb
force-pushed
the
less-interning
branch
from
83bd6b7
to
a29d4ee
Compare
This comment has been minimized.
This comment has been minimized.
Noratrieb
force-pushed
the
less-interning
branch
from
a29d4ee
to
a1d6331
Compare
bjorn3
reviewed
Feb 25, 2024
| @@ -457,6 +457,8 @@ impl std::str::FromStr for RustcLto { | |||
| } | |||
|
|
|||
| #[derive(Copy, Clone, Default, PartialEq, Eq, PartialOrd, Ord, Hash)] | |||
| // N.B.: This type is used everywhere, and the entire codebase relies on it being Copy. | |||
| // Making !Copy is highly nontrivial! | |||
| pub struct TargetSelection { | |||
| pub triple: Interned<String>, | |||
| file: Option<Interned<String>>, | |||
There was a problem hiding this comment.
Maybe use &'static str here by leaking the strings using Box::leak? There should only be a handful of unique TargetSelections anyway.
This comment has been minimized.
This comment has been minimized.
|
☔ The latest upstream changes (presumably #121591) made this pull request unmergeable. Please resolve the merge conflicts. |
Noratrieb
force-pushed
the
less-interning
branch
from
a1d6331
to
3b76463
Compare
This comment has been minimized.
This comment has been minimized.
This interning is pointless and only makes the code more complex. The only remaining use of interning is `TargetSelection`, for which I left a comment.
Noratrieb
force-pushed
the
less-interning
branch
from
3b76463
to
fbb97ed
Compare
|
Thanks for the PR! |
bors
added
S-waiting-on-bors
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Mar 9, 2024
…larsan68 Avoid some interning in bootstrap This interning is pointless and only makes the code more complex. The only remaining use of interning is `TargetSelection`, for which I left a comment.
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Mar 9, 2024
…iaskrgr Rollup of 11 pull requests Successful merges: - rust-lang#121567 (Avoid some interning in bootstrap) - rust-lang#121813 (Misc improvements to non local defs lint implementation) - rust-lang#121860 (Add a tidy check that checks whether the fluent slugs only appear once) - rust-lang#121907 (skip sanity check for non-host targets in `check` builds) - rust-lang#122160 (Eagerly translate `HelpUseLatestEdition` in parser diagnostics) - rust-lang#122178 (ci: add a runner for vanilla LLVM 18) - rust-lang#122186 (Remove a workaround for a bug) - rust-lang#122187 (Move metadata header and version checks together) - rust-lang#122215 (Some tweaks to the parallel query cycle handler) - rust-lang#122223 (Fix typo in `VisitorResult`) - rust-lang#122232 (library/core: fix a comment, and a cfg(miri) warning) r? `@ghost` `@rustbot` modify labels: rollup
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Mar 9, 2024
…larsan68 Avoid some interning in bootstrap This interning is pointless and only makes the code more complex. The only remaining use of interning is `TargetSelection`, for which I left a comment.
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Mar 10, 2024
…iaskrgr Rollup of 10 pull requests Successful merges: - rust-lang#121561 (Detect typos for compiletest test directives) - rust-lang#121567 (Avoid some interning in bootstrap) - rust-lang#121685 (Fixing shellcheck comments on lvi test script) - rust-lang#121833 (Suggest correct path in include_bytes!) - rust-lang#121860 (Add a tidy check that checks whether the fluent slugs only appear once) - rust-lang#121907 (skip sanity check for non-host targets in `check` builds) - rust-lang#122029 (When displaying multispans, ignore empty lines adjacent to `...`) - rust-lang#122221 (match lowering: define a convenient struct) - rust-lang#122244 (fix: LocalWaker memory leak and some stability attributes) - rust-lang#122251 (Add test to check unused_lifetimes don't duplicate "parameter is never used" error) r? `@ghost` `@rustbot` modify labels: rollup
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Mar 10, 2024
…iaskrgr Rollup of 14 pull requests Successful merges: - rust-lang#112136 (Add std::ffi::c_str module) - rust-lang#113525 (Dynamically size sigaltstk in std) - rust-lang#121567 (Avoid some interning in bootstrap) - rust-lang#121642 (Update a test to support Symbol Mangling V0) - rust-lang#121685 (Fixing shellcheck comments on lvi test script) - rust-lang#121860 (Add a tidy check that checks whether the fluent slugs only appear once) - rust-lang#121942 (std::rand: enable getrandom for dragonflybsd too.) - rust-lang#122125 (Revert back to Git-for-Windows for MinGW CI builds) - rust-lang#122221 (match lowering: define a convenient struct) - rust-lang#122244 (fix: LocalWaker memory leak and some stability attributes) - rust-lang#122251 (Add test to check unused_lifetimes don't duplicate "parameter is never used" error) - rust-lang#122264 (add myself to rotation) - rust-lang#122269 (doc/rustc: Move loongarch64-unknown-linux-musl to Tier 3) - rust-lang#122271 (Fix legacy numeric constant diag items) r? `@ghost` `@rustbot` modify labels: rollup
rust-timer
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Mar 10, 2024
Rollup merge of rust-lang#121567 - Nilstrieb:less-interning, r=albertlarsan68 Avoid some interning in bootstrap This interning is pointless and only makes the code more complex. The only remaining use of interning is `TargetSelection`, for which I left a comment.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This interning is pointless and only makes the code more complex.
The only remaining use of interning is
TargetSelection, for which I left a comment.