Hash only the spans that we care ended up reading in Span::try_metavars

compiler-errors · compiler-errors · commit 2de21ad7d4a1 · 2024-12-21T20:37:27.000Z
diff --git a/compiler/rustc_expand/src/mbe/transcribe.rs b/compiler/rustc_expand/src/mbe/transcribe.rs
@@ -6,14 +6,13 @@ 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;
 use rustc_session::parse::{ParseSess, SymbolGallery};
 use rustc_span::hygiene::{LocalExpnId, Transparency};
 use rustc_span::{
-    Ident, MacroRulesNormalizedIdent, Span, Symbol, SyntaxContext, sym, with_metavar_spans_mut,
+    Ident, MacroRulesNormalizedIdent, Span, Symbol, SyntaxContext, sym, with_metavar_spans,
 };
 use smallvec::{SmallVec, smallvec};
 
@@ -283,13 +282,13 @@ pub(super) fn transcribe<'a>(
                         }
                         MatchedSingle(ParseNtResult::Ident(ident, is_raw)) => {
                             marker.visit_span(&mut sp);
-                            with_metavar_spans_mut(|mspans| mspans.insert(ident.span, 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_mut(|mspans| mspans.insert(ident.span, sp));
+                            with_metavar_spans(|mspans| mspans.insert(ident.span, sp));
                             let kind = token::NtLifetime(*ident, *is_raw);
                             TokenTree::token_alone(kind, sp)
                         }
@@ -299,7 +298,7 @@ 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_mut(|mspans| mspans.insert(use_span, sp));
+                            with_metavar_spans(|mspans| mspans.insert(use_span, sp));
                             TokenTree::token_alone(token::Interpolated(Lrc::clone(nt)), sp)
                         }
                         MatchedSeq(..) => {
@@ -415,19 +414,15 @@ fn maybe_use_metavar_location(
         return orig_tt.clone();
     }
 
-    let insert = |mspans: &mut UnordMap<_, _>, 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_mut(|mspans| insert(mspans, token.span, metavar_span))
+            with_metavar_spans(|mspans| mspans.insert(token.span, metavar_span))
         }
-        TokenTree::Delimited(dspan, ..) => with_metavar_spans_mut(|mspans| {
-            insert(mspans, dspan.open, metavar_span)
-                && insert(mspans, dspan.close, metavar_span)
-                && insert(mspans, dspan.entire(), metavar_span)
+        TokenTree::Delimited(dspan, ..) => with_metavar_spans(|mspans| {
+            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) {
@@ -439,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_mut(|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_mut(|mspans| {
-                insert(mspans, open, metavar_span) && insert(mspans, close, metavar_span)
+            with_metavar_spans(|mspans| {
+                mspans.insert(open, metavar_span) && mspans.insert(close, metavar_span)
             });
             let dspan = DelimSpan::from_pair(open, close);
             TokenTree::Delimited(dspan, *dspacing, *delimiter, tts.clone())
diff --git a/compiler/rustc_middle/src/hir/map/mod.rs b/compiler/rustc_middle/src/hir/map/mod.rs
@@ -12,9 +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, freeze_metavar_spans, kw, sym, with_metavar_spans,
-};
+use rustc_span::{ErrorGuaranteed, Ident, Span, Symbol, kw, sym, with_metavar_spans};
 
 use crate::hir::ModuleItems;
 use crate::middle::debugger_visualizer::DebuggerVisualizerFile;
@@ -1089,9 +1087,6 @@ 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);
@@ -1122,7 +1117,7 @@ pub(super) fn crate_hash(tcx: TyCtxt<'_>, _: LocalCrate) -> Svh {
         // and combining it with other hashes here.
         resolutions.visibilities_for_hashing.hash_stable(&mut hcx, &mut stable_hasher);
         with_metavar_spans(|mspans| {
-            mspans.hash_stable(&mut hcx, &mut stable_hasher);
+            mspans.freeze_and_get_read_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
@@ -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)]
@@ -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: FreezeLock<UnordMap<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,21 +178,42 @@ pub fn create_default_session_globals_then<R>(f: impl FnOnce() -> R) -> R {
 // deserialization.
 scoped_tls::scoped_thread_local!(static SESSION_GLOBALS: SessionGlobals);
 
-#[inline]
-pub fn with_metavar_spans_mut<R>(f: impl FnOnce(&mut UnordMap<Span, Span>) -> R) -> R {
-    with_session_globals(|session_globals| f(&mut session_globals.metavar_spans.write()))
-}
+#[derive(Default)]
+pub struct MetavarSpansMap(FreezeLock<UnordMap<Span, (Span, bool)>>);
 
-#[inline]
-pub fn with_metavar_spans<R>(f: impl FnOnce(&UnordMap<Span, Span>) -> R) -> R {
-    with_session_globals(|session_globals| f(&session_globals.metavar_spans.read()))
+impl MetavarSpansMap {
+    pub fn insert(&self, span: Span, var_span: Span) -> bool {
+        match self.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.
+    pub fn get(&self, span: Span) -> Option<Span> {
+        if let Some(mut mspans) = self.0.try_write() {
+            if let Some((var_span, read)) = mspans.get_mut(&span) {
+                *read = true;
+                Some(*var_span)
+            } else {
+                None
+            }
+        } else {
+            if let Some((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.
+    pub fn freeze_and_get_read_spans(&self) -> UnordMap<Span, Span> {
+        self.0.freeze().items().filter(|(_, (_, b))| *b).map(|(s1, (s2, _))| (*s1, *s2)).collect()
+    }
 }
 
 #[inline]
-pub fn freeze_metavar_spans() {
-    with_session_globals(|session_globals| {
-        session_globals.metavar_spans.freeze();
-    });
+pub fn with_metavar_spans<R>(f: impl FnOnce(&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
@@ -884,8 +906,7 @@ 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: &UnordMap<_, _>, s| mspans.get(&s).copied();
-        match with_metavar_spans(|mspans| (get(mspans, a_orig), get(mspans, b_orig))) {
+        match with_metavar_spans(|mspans| (mspans.get(a_orig), mspans.get(b_orig))) {
             (None, None) => {}
             (Some(meta_a), None) => {
                 let meta_a = meta_a.data();
diff --git a/tests/ui/expr/if/if-let.stderr b/tests/ui/expr/if/if-let.stderr
@@ -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
@@ -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");

Original file line number	Diff line number	Diff line change
@@ -18,7 +18,7 @@ warning: irrefutable `if let` pattern
`18`	`18`	`--> $DIR/if-let.rs:6:16`
`19`	`19`	`\|`
`20`	`20`	`LL \| if let $p = $e $b`
`21`		`- \| ^^^^^^^^^^^`
	`21`	`+ \| ^^^`
`22`	`22`	`...`
`23`	`23`	`LL \| / bar!(a, 1, {`
`24`	`24`	`LL \| \| println!("irrefutable pattern");`