From 2def03d8c8ab0443312cf14f51a54ce5320360f7 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 18 Dec 2024 18:28:08 +0000 Subject: [PATCH 1/2] Properly record metavar spans for other expansions other than TT --- compiler/rustc_expand/src/mbe/transcribe.rs | 4 +++ compiler/rustc_parse/src/validate_attr.rs | 8 ++---- .../lint-if-let-rescope-with-macro.stderr | 2 +- tests/ui/expr/if/if-let.stderr | 4 +-- tests/ui/for-loop-while/while-let-2.stderr | 4 +-- tests/ui/lint/wide_pointer_comparisons.stderr | 2 +- ...sue-65122-mac-invoc-in-mut-patterns.stderr | 4 +-- .../unsafe-attributes-fix.fixed | 11 ++++++++ .../unsafe-attributes-fix.rs | 11 ++++++++ .../unsafe-attributes-fix.stderr | 27 +++++++++++++++---- 10 files changed, 58 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_expand/src/mbe/transcribe.rs b/compiler/rustc_expand/src/mbe/transcribe.rs index 4fb1eadd486ec..1010437fcfc12 100644 --- a/compiler/rustc_expand/src/mbe/transcribe.rs +++ b/compiler/rustc_expand/src/mbe/transcribe.rs @@ -282,11 +282,13 @@ pub(super) fn transcribe<'a>( } MatchedSingle(ParseNtResult::Ident(ident, is_raw)) => { marker.visit_span(&mut sp); + with_metavar_spans(|mspans| mspans.insert(ident.span, sp)); let kind = token::NtIdent(*ident, *is_raw); TokenTree::token_alone(kind, sp) } MatchedSingle(ParseNtResult::Lifetime(ident, is_raw)) => { marker.visit_span(&mut sp); + with_metavar_spans(|mspans| mspans.insert(ident.span, sp)); let kind = token::NtLifetime(*ident, *is_raw); TokenTree::token_alone(kind, sp) } @@ -295,6 +297,8 @@ pub(super) fn transcribe<'a>( // `Delimiter::Invisible` to maintain parsing priorities. // `Interpolated` is currently used for such groups in rustc parser. marker.visit_span(&mut sp); + let use_span = nt.use_span(); + with_metavar_spans(|mspans| mspans.insert(use_span, sp)); TokenTree::token_alone(token::Interpolated(Lrc::clone(nt)), sp) } MatchedSeq(..) => { diff --git a/compiler/rustc_parse/src/validate_attr.rs b/compiler/rustc_parse/src/validate_attr.rs index 8b6b37c0f8f52..86f673c100c19 100644 --- a/compiler/rustc_parse/src/validate_attr.rs +++ b/compiler/rustc_parse/src/validate_attr.rs @@ -11,7 +11,7 @@ use rustc_session::errors::report_lit_error; use rustc_session::lint::BuiltinLintDiag; use rustc_session::lint::builtin::{ILL_FORMED_ATTRIBUTE_INPUT, UNSAFE_ATTR_OUTSIDE_UNSAFE}; use rustc_session::parse::ParseSess; -use rustc_span::{BytePos, Span, Symbol, sym}; +use rustc_span::{Span, Symbol, sym}; use crate::{errors, parse_in}; @@ -164,11 +164,7 @@ pub fn check_attribute_safety(psess: &ParseSess, safety: AttributeSafety, attr: // wrapping it in `unsafe(...)`. Otherwise, we suggest putting the // `unsafe(`, `)` right after and right before the opening and closing // square bracket respectively. - let diag_span = if attr_item.span().can_be_used_for_suggestions() { - attr_item.span() - } else { - attr.span.with_lo(attr.span.lo() + BytePos(2)).with_hi(attr.span.hi() - BytePos(1)) - }; + let diag_span = attr_item.span(); if attr.span.at_least_rust_2024() { psess.dcx().emit_err(errors::UnsafeAttrOutsideUnsafe { diff --git a/tests/ui/drop/lint-if-let-rescope-with-macro.stderr b/tests/ui/drop/lint-if-let-rescope-with-macro.stderr index d73a878c74f41..029d5c7492936 100644 --- a/tests/ui/drop/lint-if-let-rescope-with-macro.stderr +++ b/tests/ui/drop/lint-if-let-rescope-with-macro.stderr @@ -2,7 +2,7 @@ error: `if let` assigns a shorter lifetime since Edition 2024 --> $DIR/lint-if-let-rescope-with-macro.rs:12:12 | LL | if let $p = $e { $($conseq)* } else { $($alt)* } - | ^^^ + | ^^^^^^^^^^^ ... LL | / edition_2021_if_let! { LL | | Some(_value), diff --git a/tests/ui/expr/if/if-let.stderr b/tests/ui/expr/if/if-let.stderr index c4bba3cb1a8a0..5f77909f9810c 100644 --- a/tests/ui/expr/if/if-let.stderr +++ b/tests/ui/expr/if/if-let.stderr @@ -2,7 +2,7 @@ warning: irrefutable `if let` pattern --> $DIR/if-let.rs:6:16 | LL | if let $p = $e $b - | ^^^ + | ^^^^^^^^^^^ ... LL | / foo!(a, 1, { LL | | println!("irrefutable pattern"); @@ -18,7 +18,7 @@ warning: irrefutable `if let` pattern --> $DIR/if-let.rs:6:16 | LL | if let $p = $e $b - | ^^^ + | ^^^^^^^^^^^ ... LL | / bar!(a, 1, { LL | | println!("irrefutable pattern"); diff --git a/tests/ui/for-loop-while/while-let-2.stderr b/tests/ui/for-loop-while/while-let-2.stderr index 1b1cf67924333..b93a216ac6500 100644 --- a/tests/ui/for-loop-while/while-let-2.stderr +++ b/tests/ui/for-loop-while/while-let-2.stderr @@ -2,7 +2,7 @@ warning: irrefutable `while let` pattern --> $DIR/while-let-2.rs:7:19 | LL | while let $p = $e $b - | ^^^ + | ^^^^^^^^^^^ ... LL | / foo!(_a, 1, { LL | | println!("irrefutable pattern"); @@ -18,7 +18,7 @@ warning: irrefutable `while let` pattern --> $DIR/while-let-2.rs:7:19 | LL | while let $p = $e $b - | ^^^ + | ^^^^^^^^^^^ ... LL | / bar!(_a, 1, { LL | | println!("irrefutable pattern"); diff --git a/tests/ui/lint/wide_pointer_comparisons.stderr b/tests/ui/lint/wide_pointer_comparisons.stderr index 7fe382393d7e1..78548e308ed57 100644 --- a/tests/ui/lint/wide_pointer_comparisons.stderr +++ b/tests/ui/lint/wide_pointer_comparisons.stderr @@ -615,7 +615,7 @@ warning: ambiguous wide pointer comparison, the comparison includes metadata whi --> $DIR/wide_pointer_comparisons.rs:169:37 | LL | ($a:expr, $b:expr) => { $a == $b } - | ^^ + | ^^^^^^^^ ... LL | cmp!(&a, &b); | ------------ in this macro invocation diff --git a/tests/ui/parser/issues/issue-65122-mac-invoc-in-mut-patterns.stderr b/tests/ui/parser/issues/issue-65122-mac-invoc-in-mut-patterns.stderr index 76259b40a9332..dda37d83282c2 100644 --- a/tests/ui/parser/issues/issue-65122-mac-invoc-in-mut-patterns.stderr +++ b/tests/ui/parser/issues/issue-65122-mac-invoc-in-mut-patterns.stderr @@ -30,7 +30,7 @@ error: `mut` must be followed by a named binding --> $DIR/issue-65122-mac-invoc-in-mut-patterns.rs:13:13 | LL | let mut $eval = (); - | ^^^ + | ^^^^ ... LL | mac2! { does_not_exist!() } | --------------------------- in this macro invocation @@ -40,7 +40,7 @@ LL | mac2! { does_not_exist!() } help: remove the `mut` prefix | LL - let mut $eval = (); -LL + let $eval = (); +LL + let $eval = (); | error: cannot find macro `does_not_exist` in this scope diff --git a/tests/ui/rust-2024/unsafe-attributes/unsafe-attributes-fix.fixed b/tests/ui/rust-2024/unsafe-attributes/unsafe-attributes-fix.fixed index 586881d180767..9b4c9de031b9d 100644 --- a/tests/ui/rust-2024/unsafe-attributes/unsafe-attributes-fix.fixed +++ b/tests/ui/rust-2024/unsafe-attributes/unsafe-attributes-fix.fixed @@ -40,6 +40,15 @@ macro_rules! meta2 { } } +macro_rules! with_cfg_attr { + () => { + #[cfg_attr(all(), unsafe(link_section = ".custom_section"))] + //~^ ERROR: unsafe attribute used without unsafe + //~| WARN this is accepted in the current edition + pub extern "C" fn abc() {} + }; +} + tt!([unsafe(no_mangle)]); //~^ ERROR: unsafe attribute used without unsafe //~| WARN this is accepted in the current edition @@ -52,6 +61,8 @@ meta2!(unsafe(export_name = "baw")); //~| WARN this is accepted in the current edition ident2!(export_name, "bars"); +with_cfg_attr!(); + #[unsafe(no_mangle)] //~^ ERROR: unsafe attribute used without unsafe //~| WARN this is accepted in the current edition diff --git a/tests/ui/rust-2024/unsafe-attributes/unsafe-attributes-fix.rs b/tests/ui/rust-2024/unsafe-attributes/unsafe-attributes-fix.rs index 03e122c7d57e7..75f9262590041 100644 --- a/tests/ui/rust-2024/unsafe-attributes/unsafe-attributes-fix.rs +++ b/tests/ui/rust-2024/unsafe-attributes/unsafe-attributes-fix.rs @@ -40,6 +40,15 @@ macro_rules! meta2 { } } +macro_rules! with_cfg_attr { + () => { + #[cfg_attr(all(), link_section = ".custom_section")] + //~^ ERROR: unsafe attribute used without unsafe + //~| WARN this is accepted in the current edition + pub extern "C" fn abc() {} + }; +} + tt!([no_mangle]); //~^ ERROR: unsafe attribute used without unsafe //~| WARN this is accepted in the current edition @@ -52,6 +61,8 @@ meta2!(export_name = "baw"); //~| WARN this is accepted in the current edition ident2!(export_name, "bars"); +with_cfg_attr!(); + #[no_mangle] //~^ ERROR: unsafe attribute used without unsafe //~| WARN this is accepted in the current edition diff --git a/tests/ui/rust-2024/unsafe-attributes/unsafe-attributes-fix.stderr b/tests/ui/rust-2024/unsafe-attributes/unsafe-attributes-fix.stderr index 87330d2693d05..15a48fb71594a 100644 --- a/tests/ui/rust-2024/unsafe-attributes/unsafe-attributes-fix.stderr +++ b/tests/ui/rust-2024/unsafe-attributes/unsafe-attributes-fix.stderr @@ -1,5 +1,5 @@ error: unsafe attribute used without unsafe - --> $DIR/unsafe-attributes-fix.rs:43:6 + --> $DIR/unsafe-attributes-fix.rs:52:6 | LL | tt!([no_mangle]); | ^^^^^^^^^ usage of unsafe attribute @@ -34,7 +34,7 @@ LL | #[unsafe($e)] | +++++++ + error: unsafe attribute used without unsafe - --> $DIR/unsafe-attributes-fix.rs:47:7 + --> $DIR/unsafe-attributes-fix.rs:56:7 | LL | meta!(no_mangle); | ^^^^^^^^^ usage of unsafe attribute @@ -47,7 +47,7 @@ LL | meta!(unsafe(no_mangle)); | +++++++ + error: unsafe attribute used without unsafe - --> $DIR/unsafe-attributes-fix.rs:50:8 + --> $DIR/unsafe-attributes-fix.rs:59:8 | LL | meta2!(export_name = "baw"); | ^^^^^^^^^^^ usage of unsafe attribute @@ -77,7 +77,24 @@ LL | #[unsafe($e = $l)] | +++++++ + error: unsafe attribute used without unsafe - --> $DIR/unsafe-attributes-fix.rs:55:3 + --> $DIR/unsafe-attributes-fix.rs:45:27 + | +LL | #[cfg_attr(all(), link_section = ".custom_section")] + | ^^^^^^^^^^^^ usage of unsafe attribute +... +LL | with_cfg_attr!(); + | ---------------- in this macro invocation + | + = warning: this is accepted in the current edition (Rust 2015) but is a hard error in Rust 2024! + = note: for more information, see + = note: this error originates in the macro `with_cfg_attr` (in Nightly builds, run with -Z macro-backtrace for more info) +help: wrap the attribute in `unsafe(...)` + | +LL | #[cfg_attr(all(), unsafe(link_section = ".custom_section"))] + | +++++++ + + +error: unsafe attribute used without unsafe + --> $DIR/unsafe-attributes-fix.rs:66:3 | LL | #[no_mangle] | ^^^^^^^^^ usage of unsafe attribute @@ -89,5 +106,5 @@ help: wrap the attribute in `unsafe(...)` LL | #[unsafe(no_mangle)] | +++++++ + -error: aborting due to 6 previous errors +error: aborting due to 7 previous errors From 4d766b56df2b19420272e35226f59e42ced2e0ce Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 18 Dec 2024 23:28:27 +0000 Subject: [PATCH 2/2] Hash only the spans that we care ended up reading in Span::try_metavars --- compiler/rustc_expand/src/mbe/transcribe.rs | 23 +++++++++++------ compiler/rustc_middle/src/hir/map/mod.rs | 18 ++++++++++++- compiler/rustc_span/src/lib.rs | 28 +++++++++++++++++---- 3 files changed, 56 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_expand/src/mbe/transcribe.rs b/compiler/rustc_expand/src/mbe/transcribe.rs index 1010437fcfc12..1892ddcf05a1a 100644 --- a/compiler/rustc_expand/src/mbe/transcribe.rs +++ b/compiler/rustc_expand/src/mbe/transcribe.rs @@ -6,6 +6,7 @@ use rustc_ast::token::{self, Delimiter, IdentIsRaw, Lit, LitKind, Nonterminal, T use rustc_ast::tokenstream::{DelimSpacing, DelimSpan, Spacing, TokenStream, TokenTree}; use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::sync::Lrc; +use rustc_data_structures::unord::UnordMap; use rustc_errors::{Diag, DiagCtxtHandle, PResult, pluralize}; use rustc_parse::lexer::nfc_normalize; use rustc_parse::parser::ParseNtResult; @@ -282,13 +283,17 @@ pub(super) fn transcribe<'a>( } MatchedSingle(ParseNtResult::Ident(ident, is_raw)) => { marker.visit_span(&mut sp); - with_metavar_spans(|mspans| mspans.insert(ident.span, sp)); + with_metavar_spans(|mspans| { + mspans.write().insert(ident.span, (sp, false)) + }); let kind = token::NtIdent(*ident, *is_raw); TokenTree::token_alone(kind, sp) } MatchedSingle(ParseNtResult::Lifetime(ident, is_raw)) => { marker.visit_span(&mut sp); - with_metavar_spans(|mspans| mspans.insert(ident.span, sp)); + with_metavar_spans(|mspans| { + mspans.write().insert(ident.span, (sp, false)) + }); let kind = token::NtLifetime(*ident, *is_raw); TokenTree::token_alone(kind, sp) } @@ -298,7 +303,9 @@ pub(super) fn transcribe<'a>( // `Interpolated` is currently used for such groups in rustc parser. marker.visit_span(&mut sp); let use_span = nt.use_span(); - with_metavar_spans(|mspans| mspans.insert(use_span, sp)); + with_metavar_spans(|mspans| { + mspans.write().insert(use_span, (sp, false)) + }); TokenTree::token_alone(token::Interpolated(Lrc::clone(nt)), sp) } MatchedSeq(..) => { @@ -414,16 +421,17 @@ fn maybe_use_metavar_location( return orig_tt.clone(); } - let insert = |mspans: &mut FxHashMap<_, _>, s, ms| match mspans.try_insert(s, ms) { + let insert = |mspans: &mut UnordMap<_, _>, s, ms| match mspans.try_insert(s, (ms, false)) { Ok(_) => true, - Err(err) => *err.entry.get() == ms, // Tried to insert the same span, still success + Err(err) => err.entry.get().0 == ms, // Tried to insert the same span, still success }; marker.visit_span(&mut metavar_span); let no_collision = match orig_tt { TokenTree::Token(token, ..) => { - with_metavar_spans(|mspans| insert(mspans, token.span, metavar_span)) + with_metavar_spans(|mspans| insert(&mut mspans.write(), token.span, metavar_span)) } TokenTree::Delimited(dspan, ..) => with_metavar_spans(|mspans| { + let mspans = &mut mspans.write(); insert(mspans, dspan.open, metavar_span) && insert(mspans, dspan.close, metavar_span) && insert(mspans, dspan.entire(), metavar_span) @@ -438,13 +446,14 @@ fn maybe_use_metavar_location( match orig_tt { TokenTree::Token(Token { kind, span }, spacing) => { let span = metavar_span.with_ctxt(span.ctxt()); - with_metavar_spans(|mspans| insert(mspans, span, metavar_span)); + with_metavar_spans(|mspans| insert(&mut mspans.write(), span, metavar_span)); TokenTree::Token(Token { kind: kind.clone(), span }, *spacing) } TokenTree::Delimited(dspan, dspacing, delimiter, tts) => { let open = metavar_span.with_ctxt(dspan.open.ctxt()); let close = metavar_span.with_ctxt(dspan.close.ctxt()); with_metavar_spans(|mspans| { + let mspans = &mut mspans.write(); insert(mspans, open, metavar_span) && insert(mspans, close, metavar_span) }); let dspan = DelimSpan::from_pair(open, close); diff --git a/compiler/rustc_middle/src/hir/map/mod.rs b/compiler/rustc_middle/src/hir/map/mod.rs index 308078ddf87d9..2c3839e137bb7 100644 --- a/compiler/rustc_middle/src/hir/map/mod.rs +++ b/compiler/rustc_middle/src/hir/map/mod.rs @@ -4,6 +4,7 @@ use rustc_data_structures::fingerprint::Fingerprint; use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; use rustc_data_structures::svh::Svh; use rustc_data_structures::sync::{DynSend, DynSync, par_for_each_in, try_par_for_each_in}; +use rustc_data_structures::unord::UnordMap; use rustc_hir::def::{DefKind, Res}; use rustc_hir::def_id::{DefId, LOCAL_CRATE, LocalDefId, LocalModDefId}; use rustc_hir::definitions::{DefKey, DefPath, DefPathHash}; @@ -12,7 +13,9 @@ use rustc_hir::*; use rustc_hir_pretty as pprust_hir; use rustc_middle::hir::nested_filter; use rustc_span::def_id::StableCrateId; -use rustc_span::{ErrorGuaranteed, Ident, Span, Symbol, kw, sym}; +use rustc_span::{ + ErrorGuaranteed, Ident, Span, Symbol, freeze_metavar_spans, kw, sym, with_metavar_spans, +}; use crate::hir::ModuleItems; use crate::middle::debugger_visualizer::DebuggerVisualizerFile; @@ -1087,6 +1090,9 @@ pub(super) fn crate_hash(tcx: TyCtxt<'_>, _: LocalCrate) -> Svh { .map(DebuggerVisualizerFile::path_erased) .collect(); + // Freeze metavars since we do not expect any more expansion after this. + freeze_metavar_spans(); + let crate_hash: Fingerprint = tcx.with_stable_hashing_context(|mut hcx| { let mut stable_hasher = StableHasher::new(); hir_body_hash.hash_stable(&mut hcx, &mut stable_hasher); @@ -1116,6 +1122,16 @@ pub(super) fn crate_hash(tcx: TyCtxt<'_>, _: LocalCrate) -> Svh { // the fly in the resolver, storing only their accumulated hash in `ResolverGlobalCtxt`, // and combining it with other hashes here. resolutions.visibilities_for_hashing.hash_stable(&mut hcx, &mut stable_hasher); + with_metavar_spans(|mspans| { + // Only hash the spans we ended up using. + let filtered_spans: UnordMap<_, _> = mspans + .read() + .items() + .filter(|(_, (_, b))| *b) + .map(|(s1, (s2, _))| (*s1, *s2)) + .collect(); + filtered_spans.hash_stable(&mut hcx, &mut stable_hasher); + }); stable_hasher.finish() }); diff --git a/compiler/rustc_span/src/lib.rs b/compiler/rustc_span/src/lib.rs index d5c2a337b4c19..7fdc0fe7d7e0d 100644 --- a/compiler/rustc_span/src/lib.rs +++ b/compiler/rustc_span/src/lib.rs @@ -85,9 +85,9 @@ use std::str::FromStr; use std::{fmt, iter}; use md5::{Digest, Md5}; -use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::stable_hasher::{Hash64, Hash128, HashStable, StableHasher}; use rustc_data_structures::sync::{FreezeLock, FreezeWriteGuard, Lock, Lrc}; +use rustc_data_structures::unord::UnordMap; use sha1::Sha1; use sha2::Sha256; @@ -103,7 +103,7 @@ pub struct SessionGlobals { span_interner: Lock, /// Maps a macro argument token into use of the corresponding metavariable in the macro body. /// Collisions are possible and processed in `maybe_use_metavar_location` on best effort basis. - metavar_spans: Lock>, + metavar_spans: FreezeLock>, hygiene_data: Lock, /// The session's source map, if there is one. This field should only be @@ -178,8 +178,15 @@ pub fn create_default_session_globals_then(f: impl FnOnce() -> R) -> R { scoped_tls::scoped_thread_local!(static SESSION_GLOBALS: SessionGlobals); #[inline] -pub fn with_metavar_spans(f: impl FnOnce(&mut FxHashMap) -> R) -> R { - with_session_globals(|session_globals| f(&mut session_globals.metavar_spans.lock())) +pub fn with_metavar_spans(f: impl FnOnce(&FreezeLock>) -> R) -> R { + with_session_globals(|session_globals| f(&session_globals.metavar_spans)) +} + +#[inline] +pub fn freeze_metavar_spans() { + with_session_globals(|session_globals| { + session_globals.metavar_spans.freeze(); + }); } // FIXME: We should use this enum or something like it to get rid of the @@ -872,7 +879,18 @@ impl Span { /// Check if you can select metavar spans for the given spans to get matching contexts. fn try_metavars(a: SpanData, b: SpanData, a_orig: Span, b_orig: Span) -> (SpanData, SpanData) { - let get = |mspans: &FxHashMap<_, _>, s| mspans.get(&s).copied(); + let get = |mspans: &FreezeLock>, s| { + if let Some(mut mspans) = mspans.try_write() { + if let Some((span, read)) = mspans.get_mut(&s) { + *read = true; + Some(*span) + } else { + None + } + } else { + if let Some((span, true)) = mspans.read().get(&s) { Some(*span) } else { None } + } + }; match with_metavar_spans(|mspans| (get(mspans, a_orig), get(mspans, b_orig))) { (None, None) => {} (Some(meta_a), None) => {