Skip to content

Allow drivers to supply a list of extra symbols to intern - #138682

Merged
bors merged 1 commit into
rust-lang:masterfrom
Alexendoo:extra-symbols
Apr 11, 2025
Merged

bors merged 1 commit into
rust-lang:masterfrom
Alexendoo:extra-symbols

Conversation

@Alexendoo

@Alexendoo Alexendoo commented Mar 19, 2025 •

Copy link
Copy Markdown
Member

Allows adding new symbols as consts in external drivers, desirable in Clippy so we can use them in patterns to replace code like

if matches!(name.ident.as_str(), "read_unaligned" | "write_unaligned")

The Clippy change adds a couple symbols as a demo, the exact clippy_utils API and replacing other usages can be done on the Clippy side to minimise sync conflicts


try-job: aarch64-gnu

@rustbot

rustbot commented Mar 19, 2025

Copy link
Copy Markdown
Collaborator

r? @fee1-dead

rustbot has assigned @fee1-dead.
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-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Mar 19, 2025
@rust-log-analyzer

This comment has been minimized.

@rustbot

rustbot commented Mar 19, 2025

Copy link
Copy Markdown
Collaborator

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@rust-log-analyzer

This comment has been minimized.

@blyxyas

blyxyas commented Mar 19, 2025

Copy link
Copy Markdown
Member

This will help a lot when having symbols on Clippy. We usually just batched these and submitted a PR to add 3 or 4 symbols at a time.

Strong +1 ! 👀

@bors

bors commented Mar 20, 2025

Copy link
Copy Markdown
Collaborator

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

Comment thread compiler/rustc_macros/src/symbols.rs Outdated
Interner::prefill(&[
#prefill_stream
])
pub(crate) fn fresh(extra: &[&'static str]) -> Self {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would recommend keeping fresh() but naming a new method for this. Perhaps new_with_extra_preinterned_symbols?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Renamed it to with_extra_symbols and added some docs

There's only one callsite so the current fresh would be unused

Comment thread compiler/rustc_macros/src/symbols.rs Outdated
let output = quote! {
const SYMBOL_DIGITS_BASE: u32 = #symbol_digits_base;
const PREINTERNED_SYMBOLS_COUNT: u32 = #preinterned_symbols_count;
pub const PREINTERNED_SYMBOLS_COUNT: u32 = #preinterned_symbols_count;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This needs a better name and we should document what it really means. Perhaps something like

Suggested change
pub const PREINTERNED_SYMBOLS_COUNT: u32 = #preinterned_symbols_count;
/// The number of predefined symbols; this is the the first index for
/// extra pre-interned symbols in an Interner created via
/// `new_with_extra_preinterned_symbols`.
pub const PREDEFINED_SYMBOLS_COUNT: u32 = #predefined_symbols_count;

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 24, 2025

@fee1-dead fee1-dead left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sorry for the delay! I saw that this was still S-waiting-on-author so I assumed that I didn't need to review this. For next time, it would be really nice if you could do @rustbot ready to let me know when this is ready.

I had some changes that I wanted to make, but overall I am happy enough for this to land. I'll work on the followup.

Comment thread compiler/rustc_span/src/symbol.rs Outdated
@@ -2730,8 +2728,8 @@ impl Symbol {
}

/// Is this symbol was interned in compiler's `symbols!` macro

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
/// Is this symbol was interned in compiler's `symbols!` macro
/// Was this symbol predefined in the compiler's `symbols!` macro

Comment on lines 894 to 896
// if symbol preinterned, emit tag and symbol index
if symbol.is_preinterned() {
if symbol.is_predefined() {
self.encoder.emit_u8(SYMBOL_PREINTERNED);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

should be changed to predefined too.

@fee1-dead

Copy link
Copy Markdown
Member

@bors r+ rollup

@bors

bors commented Apr 9, 2025

Copy link
Copy Markdown
Collaborator

📌 Commit abe6e88 has been approved by fee1-dead

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 9, 2025
Zalathar added a commit to Zalathar/rust that referenced this pull request Apr 10, 2025
Allow drivers to supply a list of extra symbols to intern

Allows adding new symbols as `const`s in external drivers, desirable in Clippy so we can use them in patterns to replace code like https://github.com/rust-lang/rust/blob/75530e9f72a1990ed2305e16fd51d02f47048f12/src/tools/clippy/clippy_lints/src/casts/cast_ptr_alignment.rs#L66

The Clippy change adds a couple symbols as a demo, the exact `clippy_utils` API and replacing other usages can be done on the Clippy side to minimise sync conflicts
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 10, 2025
Rollup of 18 pull requests

Successful merges:

 - rust-lang#137412 (Ensure `swap_nonoverlapping` is really always untyped)
 - rust-lang#138167 (Small code improvement in rustdoc hidden stripper)
 - rust-lang#138605 (Clean up librustdoc::html::render to be better encapsulated)
 - rust-lang#138682 (Allow drivers to supply a list of extra symbols to intern)
 - rust-lang#138904 (Test linking and running `no_std` binaries)
 - rust-lang#139423 (Suppress missing field error when autoderef bottoms out in infer)
 - rust-lang#139449 (match ergonomics: replace `peel_off_references` with a recursive call)
 - rust-lang#139507 (compiletest: Trim whitespace from environment variable names)
 - rust-lang#139530 (Remove some dead or leftover code related to rustc-intrinsic abi removal)
 - rust-lang#139560 (fix title of offset_of_enum feature)
 - rust-lang#139563 (emit a better error message for using the macro incorrectly)
 - rust-lang#139568 (Don't use empty trait names)
 - rust-lang#139580 (Temporarily leave the review rotation)
 - rust-lang#139589 (saethlin is back from vacation)
 - rust-lang#139592 (rustdoc: Enable Markdown extensions when looking for doctests)
 - rust-lang#139599 (Tracking issue template: fine-grained information on style update status)
 - rust-lang#139600 (Update `compiler-builtins` to 0.1.153)
 - rust-lang#139606 (Update compiletest to Edition 2024)

r? `@ghost`
`@rustbot` modify labels: rollup
Zalathar added a commit to Zalathar/rust that referenced this pull request Apr 10, 2025
Allow drivers to supply a list of extra symbols to intern

Allows adding new symbols as `const`s in external drivers, desirable in Clippy so we can use them in patterns to replace code like https://github.com/rust-lang/rust/blob/75530e9f72a1990ed2305e16fd51d02f47048f12/src/tools/clippy/clippy_lints/src/casts/cast_ptr_alignment.rs#L66

The Clippy change adds a couple symbols as a demo, the exact `clippy_utils` API and replacing other usages can be done on the Clippy side to minimise sync conflicts
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 10, 2025
Rollup of 17 pull requests

Successful merges:

 - rust-lang#137412 (Ensure `swap_nonoverlapping` is really always untyped)
 - rust-lang#138167 (Small code improvement in rustdoc hidden stripper)
 - rust-lang#138605 (Clean up librustdoc::html::render to be better encapsulated)
 - rust-lang#138682 (Allow drivers to supply a list of extra symbols to intern)
 - rust-lang#138904 (Test linking and running `no_std` binaries)
 - rust-lang#139423 (Suppress missing field error when autoderef bottoms out in infer)
 - rust-lang#139449 (match ergonomics: replace `peel_off_references` with a recursive call)
 - rust-lang#139507 (compiletest: Trim whitespace from environment variable names)
 - rust-lang#139530 (Remove some dead or leftover code related to rustc-intrinsic abi removal)
 - rust-lang#139560 (fix title of offset_of_enum feature)
 - rust-lang#139563 (emit a better error message for using the macro incorrectly)
 - rust-lang#139568 (Don't use empty trait names)
 - rust-lang#139580 (Temporarily leave the review rotation)
 - rust-lang#139589 (saethlin is back from vacation)
 - rust-lang#139592 (rustdoc: Enable Markdown extensions when looking for doctests)
 - rust-lang#139599 (Tracking issue template: fine-grained information on style update status)
 - rust-lang#139600 (Update `compiler-builtins` to 0.1.153)

r? `@ghost`
`@rustbot` modify labels: rollup
@Zalathar

Copy link
Copy Markdown
Member

Failed in rollup: #139613 (comment)

@bors r-

@bors bors removed the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Apr 10, 2025
@bors bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Apr 10, 2025
@fee1-dead

Copy link
Copy Markdown
Member

Well then, please address my review comments and fix the test failure when you have time :)

@Alexendoo

Copy link
Copy Markdown
Member 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 (such as code changes or more information) from the author. labels Apr 10, 2025
@Zalathar

Copy link
Copy Markdown
Member

@bors try

@bors

bors commented Apr 10, 2025

Copy link
Copy Markdown
Collaborator

⌛ Trying commit f740326 with merge 1fd0466...

bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 10, 2025
Allow drivers to supply a list of extra symbols to intern

Allows adding new symbols as `const`s in external drivers, desirable in Clippy so we can use them in patterns to replace code like https://github.com/rust-lang/rust/blob/75530e9f72a1990ed2305e16fd51d02f47048f12/src/tools/clippy/clippy_lints/src/casts/cast_ptr_alignment.rs#L66

The Clippy change adds a couple symbols as a demo, the exact `clippy_utils` API and replacing other usages can be done on the Clippy side to minimise sync conflicts

---

try-job: aarch64-gnu
@bors

bors commented Apr 10, 2025

Copy link
Copy Markdown
Collaborator

☀️ Try build successful - checks-actions
Build commit: 1fd0466 (1fd0466b59234905a1f4f191166204db03bcf7cb)

@fee1-dead

Copy link
Copy Markdown
Member

@bors r+ rollup

@bors

bors commented Apr 11, 2025

Copy link
Copy Markdown
Collaborator

📌 Commit f740326 has been approved by fee1-dead

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 Apr 11, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 11, 2025
Rollup of 12 pull requests

Successful merges:

 - rust-lang#137447 (add `core::intrinsics::simd::{simd_extract_dyn, simd_insert_dyn}`)
 - rust-lang#138182 (rustc_target: update x86_win64 to match the documented calling convention for f128)
 - rust-lang#138682 (Allow drivers to supply a list of extra symbols to intern)
 - rust-lang#138904 (Test linking and running `no_std` binaries)
 - rust-lang#138998 (Don't suggest the use of  `impl Trait` in closure parameter)
 - rust-lang#139447 (doc changes: debug assertions -> overflow checks)
 - rust-lang#139469 (Introduce a `//@ needs-crate-type` compiletest directive)
 - rust-lang#139564 (Deeply normalize obligations in `BestObligation` folder)
 - rust-lang#139574 (bootstrap: improve `channel` handling)
 - rust-lang#139600 (Update `compiler-builtins` to 0.1.153)
 - rust-lang#139641 (Allow parenthesis around inferred array lengths)
 - rust-lang#139654 (Improve `AssocItem::descr`.)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors
bors merged commit 0abc6c6 into rust-lang:master Apr 11, 2025
@rustbot rustbot added this to the 1.88.0 milestone Apr 11, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 11, 2025
Rollup merge of rust-lang#138682 - Alexendoo:extra-symbols, r=fee1-dead

Allow drivers to supply a list of extra symbols to intern

Allows adding new symbols as `const`s in external drivers, desirable in Clippy so we can use them in patterns to replace code like https://github.com/rust-lang/rust/blob/75530e9f72a1990ed2305e16fd51d02f47048f12/src/tools/clippy/clippy_lints/src/casts/cast_ptr_alignment.rs#L66

The Clippy change adds a couple symbols as a demo, the exact `clippy_utils` API and replacing other usages can be done on the Clippy side to minimise sync conflicts

---

try-job: aarch64-gnu
bors pushed a commit to rust-lang-ci/rust that referenced this pull request Apr 22, 2025
Allow drivers to supply a list of extra symbols to intern

Allows adding new symbols as `const`s in external drivers, desirable in Clippy so we can use them in patterns to replace code like https://github.com/rust-lang/rust/blob/75530e9f72a1990ed2305e16fd51d02f47048f12/src/tools/clippy/clippy_lints/src/casts/cast_ptr_alignment.rs#L66

The Clippy change adds a couple symbols as a demo, the exact `clippy_utils` API and replacing other usages can be done on the Clippy side to minimise sync conflicts

---

try-job: aarch64-gnu
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Sep 9, 2025
… r=fee1-dead

report duplicate symbols added by the driver

The panic message did not mention what symbols were duplicates, which made the panic hard to debug. This came up in [#t-compiler/help > Easiest way to find offending duplicate symbols](https://rust-lang.zulipchat.com/#narrow/channel/182449-t-compiler.2Fhelp/topic/Easiest.20way.20to.20find.20offending.20duplicate.20symbols/with/538295740).

This behavior was introduced in rust-lang#138682.

r? `@fee1-dead`
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Sep 9, 2025
… r=fee1-dead

report duplicate symbols added by the driver

The panic message did not mention what symbols were duplicates, which made the panic hard to debug. This came up in [#t-compiler/help > Easiest way to find offending duplicate symbols](https://rust-lang.zulipchat.com/#narrow/channel/182449-t-compiler.2Fhelp/topic/Easiest.20way.20to.20find.20offending.20duplicate.20symbols/with/538295740).

This behavior was introduced in rust-lang#138682.

r? ``@fee1-dead``
Zalathar added a commit to Zalathar/rust that referenced this pull request Sep 11, 2025
… r=fee1-dead

report duplicate symbols added by the driver

The panic message did not mention what symbols were duplicates, which made the panic hard to debug. This came up in [#t-compiler/help > Easiest way to find offending duplicate symbols](https://rust-lang.zulipchat.com/#narrow/channel/182449-t-compiler.2Fhelp/topic/Easiest.20way.20to.20find.20offending.20duplicate.20symbols/with/538295740).

This behavior was introduced in rust-lang#138682.

r? ```@fee1-dead```
rust-timer added a commit that referenced this pull request Sep 11, 2025
Rollup merge of #146347 - folkertdev:duplicate-symbol-panic, r=fee1-dead

report duplicate symbols added by the driver

The panic message did not mention what symbols were duplicates, which made the panic hard to debug. This came up in [#t-compiler/help > Easiest way to find offending duplicate symbols](https://rust-lang.zulipchat.com/#narrow/channel/182449-t-compiler.2Fhelp/topic/Easiest.20way.20to.20find.20offending.20duplicate.20symbols/with/538295740).

This behavior was introduced in #138682.

r? ```@fee1-dead```
@camsteffen camsteffen mentioned this pull request Feb 14, 2026
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-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants