Allow drivers to supply a list of extra symbols to intern by Alexendoo · Pull Request #138682 · rust-lang/rust

Alexendoo · 2025-03-19T02:54:53Z

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

rust/src/tools/clippy/clippy_lints/src/casts/cast_ptr_alignment.rs

Line 66 in 75530e9

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 · 2025-03-19T02:54:57Z

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 · 2025-03-19T03:45:35Z

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

blyxyas · 2025-03-19T14:47:42Z

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 · 2025-03-20T08:36:16Z

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

fee1-dead · 2025-03-24T11:06:51Z

compiler/rustc_macros/src/symbols.rs

-                Interner::prefill(&[
-                    #prefill_stream
-                ])
+            pub(crate) fn fresh(extra: &[&'static str]) -> Self {


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

Renamed it to with_extra_symbols and added some docs

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

fee1-dead · 2025-03-24T11:09:00Z

compiler/rustc_macros/src/symbols.rs

    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;


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;

fee1-dead

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.

fee1-dead · 2025-04-09T09:03:58Z

compiler/rustc_span/src/symbol.rs

@@ -2730,8 +2728,8 @@ impl Symbol {
    }

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


Suggested change

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

/// Was this symbol predefined in the compiler's `symbols!` macro

fee1-dead · 2025-04-09T09:04:30Z

compiler/rustc_middle/src/query/on_disk_cache.rs

        // if symbol preinterned, emit tag and symbol index
-        if symbol.is_preinterned() {
+        if symbol.is_predefined() {
            self.encoder.emit_u8(SYMBOL_PREINTERNED);


should be changed to predefined too.

fee1-dead · 2025-04-09T09:08:07Z

@bors r+ rollup

bors · 2025-04-09T09:08:10Z

📌 Commit abe6e88 has been approved by fee1-dead

It is now in the queue for this repository.

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

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

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

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 · 2025-04-10T09:19:04Z

Failed in rollup: #139613 (comment)

@bors r-

fee1-dead · 2025-04-10T09:59:08Z

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

Alexendoo · 2025-04-10T13:56:31Z

@rustbot ready

Zalathar · 2025-04-10T14:05:07Z

@bors try

bors · 2025-04-10T14:06:20Z

⌛ Trying commit f740326 with merge 1fd0466...

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 · 2025-04-10T16:01:18Z

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

fee1-dead · 2025-04-11T01:12:01Z

@bors r+ rollup

bors · 2025-04-11T01:12:04Z

📌 Commit f740326 has been approved by fee1-dead

It is now in the queue for this repository.

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

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

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

… 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`

… 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``

… 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```

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```

rustbot assigned fee1-dead Mar 19, 2025

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

This comment has been minimized.

Sign in to view

Alexendoo force-pushed the extra-symbols branch from d0ed3d0 to 6547c4d Compare March 19, 2025 03:45

This comment has been minimized.

Sign in to view

Alexendoo force-pushed the extra-symbols branch from 6547c4d to 8ab6d63 Compare March 19, 2025 14:13

Alexendoo force-pushed the extra-symbols branch from 8ab6d63 to d12c235 Compare March 20, 2025 14:04

fee1-dead requested changes Mar 24, 2025

View reviewed changes

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

Alexendoo force-pushed the extra-symbols branch from d12c235 to abe6e88 Compare March 24, 2025 15:45

fee1-dead approved these changes Apr 9, 2025

View reviewed changes

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 mentioned this pull request Apr 10, 2025

Rollup of 18 pull requests #139612

Closed

Zalathar mentioned this pull request Apr 10, 2025

Rollup of 17 pull requests #139613

Closed

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 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

Allow drivers to supply a list of extra symbols to intern

f740326

Alexendoo force-pushed the extra-symbols branch from abe6e88 to f740326 Compare April 10, 2025 13:55

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

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

Zalathar mentioned this pull request Apr 11, 2025

Rollup of 12 pull requests #139657

Merged

bors merged commit 0abc6c6 into rust-lang:master Apr 11, 2025
7 checks passed

rustbot added this to the 1.88.0 milestone Apr 11, 2025

BD103 mentioned this pull request Apr 28, 2025

Pre-intern extra symbols to avoid Symbol::intern() and sym!() TheBevyFlock/bevy_cli#398

Closed

blyxyas mentioned this pull request Apr 29, 2025

Optimizing Clippy & linting rust-lang/rust-project-goals#114

Closed

2 tasks

RalfJung mentioned this pull request Jun 12, 2025

intrinsics: rename min_align_of to align_of #142410

Merged

folkertdev mentioned this pull request Sep 8, 2025

report duplicate symbols added by the driver #146347

Merged

-        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;

		@@ -2730,8 +2728,8 @@ impl Symbol {
		}

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

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

Uh oh!

Conversation

Alexendoo commented Mar 19, 2025 • edited by Zalathar Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Mar 19, 2025

Uh oh!

This comment has been minimized.

rustbot commented Mar 19, 2025

Uh oh!

This comment has been minimized.

blyxyas commented Mar 19, 2025

Uh oh!

bors commented Mar 20, 2025

Uh oh!

fee1-dead Mar 24, 2025

Choose a reason for hiding this comment

Uh oh!

Alexendoo Mar 24, 2025

Choose a reason for hiding this comment

Uh oh!

fee1-dead Mar 24, 2025

Choose a reason for hiding this comment

Uh oh!

fee1-dead left a comment

Choose a reason for hiding this comment

Uh oh!

fee1-dead Apr 9, 2025

Choose a reason for hiding this comment

Uh oh!

fee1-dead Apr 9, 2025

Choose a reason for hiding this comment

Uh oh!

fee1-dead commented Apr 9, 2025

Uh oh!

bors commented Apr 9, 2025

Uh oh!

Zalathar commented Apr 10, 2025

Uh oh!

fee1-dead commented Apr 10, 2025

Uh oh!

Alexendoo commented Apr 10, 2025

Uh oh!

Zalathar commented Apr 10, 2025

Uh oh!

bors commented Apr 10, 2025

Uh oh!

bors commented Apr 10, 2025

Uh oh!

fee1-dead commented Apr 11, 2025

Uh oh!

bors commented Apr 11, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Alexendoo commented Mar 19, 2025 •

edited by Zalathar

Loading