Skip to content

Allow Clippy to define duplicate extra symbols #143596

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion compiler/rustc_driver_impl/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ pub fn run_compiler(at_args: &[String], callbacks: &mut (dyn Callbacks + Send))
hash_untracked_state: None,
register_lints: None,
override_queries: None,
extra_symbols: Vec::new(),
preinterned_symbols: None,
make_codegen_backend: None,
registry: diagnostics_registry(),
using_internal_features: &USING_INTERNAL_FEATURES,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ fn def_path_hash_depends_on_crate_id() {
// the crate by changing the crate disambiguator (e.g. via bumping the
// crate's version number).

create_session_globals_then(Edition::Edition2024, &[], None, || {
create_session_globals_then(Edition::Edition2024, None, None, || {
let id0 = StableCrateId::new(Symbol::intern("foo"), false, vec!["1".to_string()], "");
let id1 = StableCrateId::new(Symbol::intern("foo"), false, vec!["2".to_string()], "");

Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_interface/src/interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -350,9 +350,9 @@ pub struct Config {
/// the list of queries.
pub override_queries: Option<fn(&Session, &mut Providers)>,

/// An extra set of symbols to add to the symbol interner, the symbol indices
/// will start at [`PREDEFINED_SYMBOLS_COUNT`](rustc_span::symbol::PREDEFINED_SYMBOLS_COUNT)
pub extra_symbols: Vec<&'static str>,
/// Replaces the default list of preinterned symbols, should be set to the `PREINTERNED_SYMBOLS`
/// expanded from [`rustc_span::extra_symbols`]
pub preinterned_symbols: Option<&'static [&'static str]>,

/// This is a callback from the driver that is called to create a codegen backend.
///
Expand Down Expand Up @@ -418,7 +418,7 @@ pub fn run_compiler<R: Send>(config: Config, f: impl FnOnce(&Compiler) -> R + Se
&early_dcx,
config.opts.edition,
config.opts.unstable_opts.threads,
&config.extra_symbols,
config.preinterned_symbols,
SourceMapInputs { file_loader, path_mapping, hash_kind, checksum_hash_kind },
|current_gcx, jobserver_proxy| {
// The previous `early_dcx` can't be reused here because it doesn't
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_interface/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ where
checksum_hash_kind,
});

rustc_span::create_session_globals_then(DEFAULT_EDITION, &[], sm_inputs, || {
rustc_span::create_session_globals_then(DEFAULT_EDITION, None, sm_inputs, || {
let temps_dir = sessopts.unstable_opts.temps_dir.as_deref().map(PathBuf::from);
let io = CompilerIO {
input: Input::Str { name: FileName::Custom(String::new()), input: String::new() },
Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_interface/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ fn run_in_thread_with_globals<F: FnOnce(CurrentGcx, Arc<Proxy>) -> R + Send, R:
thread_stack_size: usize,
edition: Edition,
sm_inputs: SourceMapInputs,
extra_symbols: &[&'static str],
driver_symbols: Option<&[&'static str]>,
f: F,
) -> R {
// The "thread pool" is a single spawned thread in the non-parallel
Expand All @@ -148,7 +148,7 @@ fn run_in_thread_with_globals<F: FnOnce(CurrentGcx, Arc<Proxy>) -> R + Send, R:
.spawn_scoped(s, move || {
rustc_span::create_session_globals_then(
edition,
extra_symbols,
driver_symbols,
Some(sm_inputs),
|| f(CurrentGcx::new(), Proxy::new()),
)
Expand All @@ -170,7 +170,7 @@ pub(crate) fn run_in_thread_pool_with_globals<
thread_builder_diag: &EarlyDiagCtxt,
edition: Edition,
threads: usize,
extra_symbols: &[&'static str],
driver_symbols: Option<&[&'static str]>,
sm_inputs: SourceMapInputs,
f: F,
) -> R {
Expand All @@ -191,7 +191,7 @@ pub(crate) fn run_in_thread_pool_with_globals<
thread_stack_size,
edition,
sm_inputs,
extra_symbols,
driver_symbols,
|current_gcx, jobserver_proxy| {
// Register the thread for use with the `WorkerLocal` type.
registry.register();
Expand Down Expand Up @@ -259,7 +259,7 @@ pub(crate) fn run_in_thread_pool_with_globals<
// pool. Upon creation, each worker thread created gets a copy of the
// session globals in TLS. This is possible because `SessionGlobals` impls
// `Send` in the parallel compiler.
rustc_span::create_session_globals_then(edition, extra_symbols, Some(sm_inputs), || {
rustc_span::create_session_globals_then(edition, driver_symbols, Some(sm_inputs), || {
rustc_span::with_session_globals(|session_globals| {
let session_globals = FromDyn::from(session_globals);
builder
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@ pub fn symbols(input: TokenStream) -> TokenStream {
symbols::symbols(input.into()).into()
}

#[proc_macro]
pub fn extra_symbols_impl(input: TokenStream) -> TokenStream {
symbols::extra_symbols(input.into()).into()
}

/// Derive an extension trait for a given impl block. The trait name
/// goes into the parenthesized args of the macro, for greppability.
/// For example:
Expand Down
190 changes: 150 additions & 40 deletions compiler/rustc_macros/src/symbols.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,13 @@
//! ```

use std::collections::HashMap;
use std::collections::hash_map::Entry;

use proc_macro2::{Span, TokenStream};
use quote::quote;
use syn::parse::{Parse, ParseStream, Result};
use syn::punctuated::Punctuated;
use syn::{Expr, Ident, Lit, LitStr, Macro, Token, braced};
use syn::{Expr, Ident, Lit, LitStr, Macro, Token, braced, bracketed};

#[cfg(test)]
mod tests;
Expand Down Expand Up @@ -147,25 +148,44 @@ struct Predefined {
span_of_name: Span,
}

struct Duplicate {
name: String,
span_of_name: Span,
}

struct Entries {
map: HashMap<String, Predefined>,
prefill_stream: TokenStream,
}

impl Entries {
fn with_capacity(capacity: usize) -> Self {
Entries { map: HashMap::with_capacity(capacity) }
Entries { map: HashMap::with_capacity(capacity), prefill_stream: TokenStream::new() }
}

fn try_insert(&mut self, span: Span, s: String) -> (u32, Option<Duplicate>) {
let len = self.len();
match self.map.entry(s) {
Entry::Occupied(entry) => {
let Predefined { idx, span_of_name } = *entry.get();
(idx, Some(Duplicate { name: entry.key().clone(), span_of_name }))
}
Entry::Vacant(entry) => {
let s = entry.key().as_str();
self.prefill_stream.extend(quote! { #s, });
entry.insert(Predefined { idx: len, span_of_name: span });
(len, None)
}
}
}

fn insert(&mut self, span: Span, s: &str, errors: &mut Errors) -> u32 {
if let Some(prev) = self.map.get(s) {
errors.error(span, format!("Symbol `{s}` is duplicated"));
errors.error(prev.span_of_name, "location of previous definition".to_string());
prev.idx
} else {
let idx = self.len();
self.map.insert(s.to_string(), Predefined { idx, span_of_name: span });
idx
fn insert(&mut self, span: Span, s: String, errors: &mut Errors) -> u32 {
let (idx, duplicate) = self.try_insert(span, s);
if let Some(Duplicate { name, span_of_name }) = duplicate {
errors.error(span, format!("Symbol `{name}` is duplicated"));
errors.error(span_of_name, "location of previous definition".to_string());
}
idx
}

fn len(&self) -> u32 {
Expand All @@ -188,18 +208,13 @@ fn symbols_with_errors(input: TokenStream) -> (TokenStream, Vec<syn::Error>) {

let mut keyword_stream = quote! {};
let mut symbols_stream = quote! {};
let mut prefill_stream = quote! {};
let mut entries = Entries::with_capacity(input.keywords.len() + input.symbols.len() + 10);

// Generate the listed keywords.
for keyword in input.keywords.iter() {
let name = &keyword.name;
let value = &keyword.value;
let value_string = value.value();
let idx = entries.insert(keyword.name.span(), &value_string, &mut errors);
prefill_stream.extend(quote! {
#value,
});
let value_string = keyword.value.value();
let idx = entries.insert(keyword.name.span(), value_string, &mut errors);
keyword_stream.extend(quote! {
pub const #name: Symbol = Symbol::new(#idx);
});
Expand All @@ -224,23 +239,15 @@ fn symbols_with_errors(input: TokenStream) -> (TokenStream, Vec<syn::Error>) {
continue;
}
};
let idx = entries.insert(symbol.name.span(), &value, &mut errors);

prefill_stream.extend(quote! {
#value,
});
let idx = entries.insert(symbol.name.span(), value, &mut errors);
symbols_stream.extend(quote! {
pub const #name: Symbol = Symbol::new(#idx);
});
}

// Generate symbols for the strings "0", "1", ..., "9".
for n in 0..10 {
let n = n.to_string();
entries.insert(Span::call_site(), &n, &mut errors);
prefill_stream.extend(quote! {
#n,
});
entries.insert(Span::call_site(), n.to_string(), &mut errors);
}

// Symbols whose value comes from an environment variable. It's allowed for
Expand All @@ -267,23 +274,16 @@ fn symbols_with_errors(input: TokenStream) -> (TokenStream, Vec<syn::Error>) {
}
};

let idx = if let Some(prev) = entries.map.get(&value) {
prev.idx
} else {
prefill_stream.extend(quote! {
#value,
});
entries.insert(symbol.name.span(), &value, &mut errors)
};

let name = &symbol.name;
let (idx, _) = entries.try_insert(name.span(), value);
symbols_stream.extend(quote! {
pub const #name: Symbol = Symbol::new(#idx);
});
}

let symbol_digits_base = entries.map["0"].idx;
let predefined_symbols_count = entries.len();
let prefill_stream = entries.prefill_stream;
let output = quote! {
const SYMBOL_DIGITS_BASE: u32 = #symbol_digits_base;

Expand All @@ -309,14 +309,124 @@ fn symbols_with_errors(input: TokenStream) -> (TokenStream, Vec<syn::Error>) {
impl Interner {
/// Creates an `Interner` with the predefined symbols from the `symbols!` macro and
/// any extra symbols provided by external drivers such as Clippy
pub(crate) fn with_extra_symbols(extra_symbols: &[&'static str]) -> Self {
pub(crate) fn new(driver_symbols: Option<&[&'static str]>) -> Self {
Interner::prefill(
&[#prefill_stream],
extra_symbols,
driver_symbols.unwrap_or(&[#prefill_stream])
)
}
}

/// Allows drivers to define extra preinterned symbols, the expanded `PREINTERNED_SYMBOLS`
/// is to be provided to `rustc_interface::Config`
#[macro_export]
macro_rules! extra_symbols {
($(#[macro_export] $further_symbols:ident;)? Symbols { $($tt:tt)* }) => {
rustc_macros::extra_symbols_impl! {
$($further_symbols)? [#prefill_stream] $($tt)*
}
};
}
};

(output, errors.list)
}

#[derive(Default)]
struct ExtraSymbols {
macro_ident: Option<Ident>,
predefined: Punctuated<LitStr, Token![,]>,
extra_symbols: Punctuated<Symbol, Token![,]>,
}

impl Parse for ExtraSymbols {
fn parse(input: ParseStream<'_>) -> Result<Self> {
let macro_ident: Option<Ident> = input.parse()?;

let content;
bracketed!(content in input);
let predefined = Punctuated::parse_terminated(&content)?;

let extra_symbols = Punctuated::parse_terminated(&input)?;

Ok(ExtraSymbols { macro_ident, predefined, extra_symbols })
}
}

pub(super) fn extra_symbols(input: TokenStream) -> TokenStream {
let mut errors = Errors::default();

let input: ExtraSymbols = match syn::parse2(input) {
Ok(input) => input,
Err(e) => {
errors.list.push(e);
Default::default()
}
};

let mut symbol_stream = TokenStream::new();
let mut duplicate_symbols = TokenStream::new();

let mut entries = Entries::with_capacity(input.predefined.len() + input.extra_symbols.len());
for lit in &input.predefined {
entries.insert(lit.span(), lit.value(), &mut errors);
}

for symbol in input.extra_symbols {
let value = match symbol.value {
Value::SameAsName => symbol.name.to_string(),
Value::String(lit) => lit.value(),
_ => {
errors.error(
symbol.name.span(),
"unsupported expression for extra symbol value".to_string(),
);
continue;
}
};

let name = &symbol.name;
let (idx, duplicate) = entries.try_insert(name.span(), value);
if duplicate.is_some() {
duplicate_symbols.extend(quote! { #name, });
}
symbol_stream.extend(quote! {
pub const #name: Symbol = Symbol::new(#idx);
});
}

let prefill_stream = entries.prefill_stream;

let further_symbols = if let Some(macro_ident) = input.macro_ident {
quote! {
#[macro_export]
macro_rules! #macro_ident {
($(#[macro_export] $further_symbols:ident;)? Symbols { $($tt:tt)* }) => {
rustc_macros::extra_symbols_impl! {
$($further_symbols)? [#prefill_stream] $($tt)*
}
};
}
}
} else {
TokenStream::new()
};

let mut output = quote! {
/// To be supplied to `rustc_interface::Config`
pub const PREINTERNED_SYMBOLS: &[&str] = &[
#prefill_stream
];

pub const DUPLICATE_SYMBOLS: &[Symbol] = &[
#duplicate_symbols
];

#symbol_stream

#further_symbols
};

output.extend(errors.list.into_iter().map(|e| e.into_compile_error()));

output
}
Loading
Loading