Auto merge of #134478 - compiler-errors:attr-span, r=oli-obk

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#132908
Alternative to #133270
cc `@ehuss`
cc `@petrochenkov`

Author: bors

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(..) => {
@@ -410,19 +414,15 @@ fn maybe_use_metavar_location(
 return orig_tt.clone();
 }
 
-let insert = |mspans:&mutFxHashMap<_,_>, s, ms| match mspans.try_insert(s, ms){
-Ok(_) => true,
-Err(err) => *err.entry.get() == 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| mspans.insert(token.span, metavar_span))
 }
 TokenTree::Delimited(dspan, ..) => with_metavar_spans(|mspans| {
-insert(mspans,dspan.open, metavar_span)
- && insert(mspans,dspan.close, metavar_span)
- && insert(mspans,dspan.entire(), metavar_span)
+mspans.insert(dspan.open, metavar_span)
+ && mspans.insert(dspan.close, metavar_span)
+ && mspans.insert(dspan.entire(), metavar_span)
 }),
 };
 if no_collision || psess.source_map().is_imported(metavar_span){
@@ -434,14 +434,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| mspans.insert(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| {
-insert(mspans,open, metavar_span) && insert(mspans,close, metavar_span)
+mspans.insert(open, metavar_span) && mspans.insert(close, metavar_span)
 });
 let dspan = DelimSpan::from_pair(open, close);
 TokenTree::Delimited(dspan,*dspacing,*delimiter, tts.clone())

compiler/rustc_middle/src/hir/map/mod.rs

@@ -12,7 +12,7 @@ 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, kw, sym, with_metavar_spans};
 
 usecrate::hir::ModuleItems;
 usecrate::middle::debugger_visualizer::DebuggerVisualizerFile;
@@ -1117,6 +1117,9 @@ 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| {
+ mspans.freeze_and_get_read_spans().hash_stable(&mut hcx,&mut stable_hasher);
+});
  stable_hasher.finish()
 });
 

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};
 
 usecrate::{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{

compiler/rustc_span/src/lib.rs

@@ -25,6 +25,7 @@
 #![feature(hash_set_entry)]
 #![feature(if_let_guard)]
 #![feature(let_chains)]
+#![feature(map_try_insert)]
 #![feature(negative_impls)]
 #![feature(read_buf)]
 #![feature(round_char_boundary)]
@@ -85,9 +86,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 +104,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:MetavarSpansMap,
 hygiene_data:Lock<hygiene::HygieneData>,
 
 /// The session's source map, if there is one. This field should only be
@@ -177,9 +178,42 @@ pub fn create_default_session_globals_then<R>(f: impl FnOnce() -> R) -> R {
 // deserialization.
 scoped_tls::scoped_thread_local!(staticSESSION_GLOBALS:SessionGlobals);
 
+#[derive(Default)]
+pubstructMetavarSpansMap(FreezeLock<UnordMap<Span,(Span,bool)>>);
+
+implMetavarSpansMap{
+pubfninsert(&self,span:Span,var_span:Span) -> bool{
+matchself.0.write().try_insert(span,(var_span,false)){
+Ok(_) => true,
+Err(entry) => entry.entry.get().0 == var_span,
+}
+}
+
+/// Read a span and record that it was read.
+pubfnget(&self,span:Span) -> Option<Span>{
+ifletSome(mut mspans) = self.0.try_write(){
+ifletSome((var_span, read)) = mspans.get_mut(&span){
+*read = true;
+Some(*var_span)
+}else{
+None
+}
+}else{
+ifletSome((span,true)) = self.0.read().get(&span){Some(*span)}else{None}
+}
+}
+
+/// Freeze the set, and return the spans which have been read.
+///
+/// After this is frozen, no spans that have not been read can be read.
+pubfnfreeze_and_get_read_spans(&self) -> UnordMap<Span,Span>{
+self.0.freeze().items().filter(|(_,(_, b))| *b).map(|(s1,(s2, _))| (*s1,*s2)).collect()
+}
+}
+
 #[inline]
-pubfnwith_metavar_spans<R>(f:implFnOnce(&mutFxHashMap<Span,Span>) -> R) -> R{
-with_session_globals(|session_globals| f(&mutsession_globals.metavar_spans.lock()))
+pubfnwith_metavar_spans<R>(f:implFnOnce(&MetavarSpansMap) -> R) -> R{
+with_session_globals(|session_globals| f(&session_globals.metavar_spans))
 }
 
 // FIXME: We should use this enum or something like it to get rid of the
@@ -872,8 +906,7 @@ impl Span {
 
 /// Check if you can select metavar spans for the given spans to get matching contexts.
 fntry_metavars(a:SpanData,b:SpanData,a_orig:Span,b_orig:Span) -> (SpanData,SpanData){
-let get = |mspans:&FxHashMap<_,_>, s| mspans.get(&s).copied();
-matchwith_metavar_spans(|mspans| (get(mspans, a_orig),get(mspans, b_orig))){
+matchwith_metavar_spans(|mspans| (mspans.get(a_orig), mspans.get(b_orig))){
 (None,None) => {}
 (Some(meta_a),None) => {
 let meta_a = meta_a.data();

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

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

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

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

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

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