-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Allow drivers to supply a list of extra symbols to intern #138682
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
r? @fee1-dead rustbot has assigned @fee1-dead. Use |
This comment has been minimized.
This comment has been minimized.
d0ed3d0
to
6547c4d
Compare
Some changes occurred in src/tools/clippy cc @rust-lang/clippy |
This comment has been minimized.
This comment has been minimized.
6547c4d
to
8ab6d63
Compare
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 ! 👀 |
☔ The latest upstream changes (presumably #137930) made this pull request unmergeable. Please resolve the merge conflicts. |
8ab6d63
to
d12c235
Compare
compiler/rustc_macros/src/symbols.rs
Outdated
Interner::prefill(&[ | ||
#prefill_stream | ||
]) | ||
pub(crate) fn fresh(extra: &[&'static str]) -> Self { |
There was a problem hiding this comment.
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
?
There was a problem hiding this comment.
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
compiler/rustc_macros/src/symbols.rs
Outdated
@@ -298,7 +298,7 @@ fn symbols_with_errors(input: TokenStream) -> (TokenStream, Vec<syn::Error>) { | |||
let preinterned_symbols_count = entries.len(); | |||
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; |
There was a problem hiding this comment.
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
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; |
d12c235
to
abe6e88
Compare
There was a problem hiding this comment.
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.
compiler/rustc_span/src/symbol.rs
Outdated
@@ -2730,8 +2728,8 @@ impl Symbol { | |||
} | |||
|
|||
/// Is this symbol was interned in compiler's `symbols!` macro |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// Is this symbol was interned in compiler's `symbols!` macro | |
/// Was this symbol predefined in the compiler's `symbols!` macro |
// if symbol preinterned, emit tag and symbol index | ||
if symbol.is_preinterned() { | ||
if symbol.is_predefined() { | ||
self.encoder.emit_u8(SYMBOL_PREINTERNED); |
There was a problem hiding this comment.
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.
@bors r+ 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 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
Failed in rollup: #139613 (comment) @bors r- |
Well then, please address my review comments and fix the test failure when you have time :) |
abe6e88
to
f740326
Compare
@rustbot ready |
@bors try |
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
☀️ Try build successful - checks-actions |
@bors r+ rollup |
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
Allows adding new symbols as
const
s in external drivers, desirable in Clippy so we can use them in patterns to replace code likerust/src/tools/clippy/clippy_lints/src/casts/cast_ptr_alignment.rs
Line 66 in 75530e9
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 conflictstry-job: aarch64-gnu