Auto merge of rust-lang#134478 - compiler-errors:attr-span, r=<try>

Properly record metavar spans for other expansions other than TT This properly records metavar spans for nonterminals other than tokentree. This means that we operations like `span.to(other_span)` work correctly for macros. As you can see, other diagnostics involving metavars have improved as a result. Fixes rust-lang#132908 Alternative to rust-lang#133270 cc `@ehuss` cc `@petrochenkov`
rust-lang-ci · Dec 18, 2024 · 020aed3 · 020aed3
2 parents 4ba4ac6 + 4d766b5
commit 020aed3
Show file tree

Hide file tree

Showing 12 changed files with 111 additions and 29 deletions.
diff --git 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,11 +283,17 @@ pub(super) fn transcribe<'a>(
                         }
                         MatchedSingle(ParseNtResult::Ident(ident, is_raw)) => {
                             marker.visit_span(&mut 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.write().insert(ident.span, (sp, false))
+                            });
                             let kind = token::NtLifetime(*ident, *is_raw);
                             TokenTree::token_alone(kind, sp)
                         }
@@ -295,6 +302,10 @@ 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.write().insert(use_span, (sp, false))
+                            });
                             TokenTree::token_alone(token::Interpolated(Lrc::clone(nt)), sp)
                         }
                         MatchedSeq(..) => {
@@ -410,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)
@@ -434,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
@@ -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_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/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<span_encoding::SpanInterner>,
     /// 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<FxHashMap<Span, Span>>,
+    metavar_spans: FreezeLock<UnordMap<Span, (Span, bool)>>,
     hygiene_data: Lock<hygiene::HygieneData>,
 
     /// 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<R>(f: impl FnOnce() -> R) -> R {
 scoped_tls::scoped_thread_local!(static SESSION_GLOBALS: SessionGlobals);
 
 #[inline]
-pub fn with_metavar_spans<R>(f: impl FnOnce(&mut FxHashMap<Span, Span>) -> R) -> R {
-    with_session_globals(|session_globals| f(&mut session_globals.metavar_spans.lock()))
+pub fn with_metavar_spans<R>(f: impl FnOnce(&FreezeLock<UnordMap<Span, (Span, bool)>>) -> 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<UnordMap<_, _>>, 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) => {

diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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 <https://doc.rust-lang.org/nightly/edition-guide/rust-2024/unsafe-attributes.html>
+   = 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