hygiene: Rewrite apply_mark_internal to be more understandable

petrochenkov · petrochenkov · commit 7c74474d8d9f · 2025-03-28T22:02:00.000+03:00
diff --git a/compiler/rustc_span/src/hygiene.rs b/compiler/rustc_span/src/hygiene.rs
@@ -75,6 +75,21 @@ pub struct SyntaxContextData {
 }
 
 impl SyntaxContextData {
+    fn new(
+        (parent, outer_expn, outer_transparency): SyntaxContextKey,
+        opaque: SyntaxContext,
+        opaque_and_semitransparent: SyntaxContext,
+    ) -> SyntaxContextData {
+        SyntaxContextData {
+            outer_expn,
+            outer_transparency,
+            parent,
+            opaque,
+            opaque_and_semitransparent,
+            dollar_crate_name: kw::DollarCrate,
+        }
+    }
+
     fn root() -> SyntaxContextData {
         SyntaxContextData {
             outer_expn: ExpnId::root(),
@@ -543,7 +558,7 @@ impl HygieneData {
     ) -> SyntaxContext {
         assert_ne!(expn_id, ExpnId::root());
         if transparency == Transparency::Opaque {
-            return self.apply_mark_internal(ctxt, expn_id, transparency);
+            return self.alloc_ctxt(ctxt, expn_id, transparency);
         }
 
         let call_site_ctxt = self.expn_data(expn_id).call_site.ctxt();
@@ -554,7 +569,7 @@ impl HygieneData {
         };
 
         if call_site_ctxt.is_root() {
-            return self.apply_mark_internal(ctxt, expn_id, transparency);
+            return self.alloc_ctxt(ctxt, expn_id, transparency);
         }
 
         // Otherwise, `expn_id` is a macros 1.0 definition and the call site is in a
@@ -567,74 +582,60 @@ impl HygieneData {
         //
         // See the example at `test/ui/hygiene/legacy_interaction.rs`.
         for (expn_id, transparency) in self.marks(ctxt) {
-            call_site_ctxt = self.apply_mark_internal(call_site_ctxt, expn_id, transparency);
+            call_site_ctxt = self.alloc_ctxt(call_site_ctxt, expn_id, transparency);
         }
-        self.apply_mark_internal(call_site_ctxt, expn_id, transparency)
+        self.alloc_ctxt(call_site_ctxt, expn_id, transparency)
     }
 
-    fn apply_mark_internal(
+    /// Allocate a new context with the given key, or retrieve it from cache if the given key
+    /// already exists. The auxiliary fields are calculated from the key.
+    fn alloc_ctxt(
         &mut self,
-        ctxt: SyntaxContext,
+        parent: SyntaxContext,
         expn_id: ExpnId,
         transparency: Transparency,
     ) -> SyntaxContext {
-        let syntax_context_data = &mut self.syntax_context_data;
-        debug_assert!(!syntax_context_data[ctxt.0 as usize].is_decode_placeholder());
-        let mut opaque = syntax_context_data[ctxt.0 as usize].opaque;
-        let mut opaque_and_semitransparent =
-            syntax_context_data[ctxt.0 as usize].opaque_and_semitransparent;
-
-        if transparency >= Transparency::Opaque {
-            let parent = opaque;
-            opaque = *self
-                .syntax_context_map
-                .entry((parent, expn_id, transparency))
-                .or_insert_with(|| {
-                    let new_opaque = SyntaxContext::from_usize(syntax_context_data.len());
-                    syntax_context_data.push(SyntaxContextData {
-                        outer_expn: expn_id,
-                        outer_transparency: transparency,
-                        parent,
-                        opaque: new_opaque,
-                        opaque_and_semitransparent: new_opaque,
-                        dollar_crate_name: kw::DollarCrate,
-                    });
-                    new_opaque
-                });
-        }
+        debug_assert!(!self.syntax_context_data[parent.0 as usize].is_decode_placeholder());
 
-        if transparency >= Transparency::SemiTransparent {
-            let parent = opaque_and_semitransparent;
-            opaque_and_semitransparent = *self
-                .syntax_context_map
-                .entry((parent, expn_id, transparency))
-                .or_insert_with(|| {
-                    let new_opaque_and_semitransparent =
-                        SyntaxContext::from_usize(syntax_context_data.len());
-                    syntax_context_data.push(SyntaxContextData {
-                        outer_expn: expn_id,
-                        outer_transparency: transparency,
-                        parent,
-                        opaque,
-                        opaque_and_semitransparent: new_opaque_and_semitransparent,
-                        dollar_crate_name: kw::DollarCrate,
-                    });
-                    new_opaque_and_semitransparent
-                });
+        // Look into the cache first.
+        let key = (parent, expn_id, transparency);
+        if let Some(ctxt) = self.syntax_context_map.get(&key) {
+            return *ctxt;
         }
 
-        let parent = ctxt;
-        *self.syntax_context_map.entry((parent, expn_id, transparency)).or_insert_with(|| {
-            syntax_context_data.push(SyntaxContextData {
-                outer_expn: expn_id,
-                outer_transparency: transparency,
-                parent,
-                opaque,
-                opaque_and_semitransparent,
-                dollar_crate_name: kw::DollarCrate,
-            });
-            SyntaxContext::from_usize(syntax_context_data.len() - 1)
-        })
+        // Reserve a new syntax context.
+        let ctxt = SyntaxContext::from_usize(self.syntax_context_data.len());
+        self.syntax_context_data.push(SyntaxContextData::decode_placeholder());
+        self.syntax_context_map.insert(key, ctxt);
+
+        // Opaque and semi-transparent versions of the parent. Note that they may be equal to the
+        // parent itself. E.g. `parent_opaque` == `parent` if the expn chain contains only opaques,
+        // and `parent_opaque_and_semitransparent` == `parent` if the expn contains only opaques
+        // and semi-transparents.
+        let parent_opaque = self.syntax_context_data[parent.0 as usize].opaque;
+        let parent_opaque_and_semitransparent =
+            self.syntax_context_data[parent.0 as usize].opaque_and_semitransparent;
+
+        // Evaluate opaque and semi-transparent versions of the new syntax context.
+        let (opaque, opaque_and_semitransparent) = match transparency {
+            Transparency::Transparent => (parent_opaque, parent_opaque_and_semitransparent),
+            Transparency::SemiTransparent => (
+                parent_opaque,
+                // Will be the same as `ctxt` if the expn chain contains only opaques and semi-transparents.
+                self.alloc_ctxt(parent_opaque_and_semitransparent, expn_id, transparency),
+            ),
+            Transparency::Opaque => (
+                // Will be the same as `ctxt` if the expn chain contains only opaques.
+                self.alloc_ctxt(parent_opaque, expn_id, transparency),
+                // Will be the same as `ctxt` if the expn chain contains only opaques and semi-transparents.
+                self.alloc_ctxt(parent_opaque_and_semitransparent, expn_id, transparency),
+            ),
+        };
+
+        // Fill the full data, now that we have it.
+        self.syntax_context_data[ctxt.as_u32() as usize] =
+            SyntaxContextData::new(key, opaque, opaque_and_semitransparent);
+        ctxt
     }
 }
 
diff --git a/tests/ui/proc-macro/nonterminal-token-hygiene.stdout b/tests/ui/proc-macro/nonterminal-token-hygiene.stdout
@@ -5,19 +5,19 @@ PRINT-BANG INPUT (DEBUG): TokenStream [
         stream: TokenStream [
             Ident {
                 ident: "struct",
-                span: $DIR/nonterminal-token-hygiene.rs:32:5: 32:11 (#4),
+                span: $DIR/nonterminal-token-hygiene.rs:32:5: 32:11 (#5),
             },
             Ident {
                 ident: "S",
-                span: $DIR/nonterminal-token-hygiene.rs:32:12: 32:13 (#4),
+                span: $DIR/nonterminal-token-hygiene.rs:32:12: 32:13 (#5),
             },
             Punct {
                 ch: ';',
                 spacing: Alone,
-                span: $DIR/nonterminal-token-hygiene.rs:32:13: 32:14 (#4),
+                span: $DIR/nonterminal-token-hygiene.rs:32:13: 32:14 (#5),
             },
         ],
-        span: $DIR/nonterminal-token-hygiene.rs:22:27: 22:32 (#5),
+        span: $DIR/nonterminal-token-hygiene.rs:22:27: 22:32 (#4),
     },
 ]
 #![feature /* 0#0 */(prelude_import)]
@@ -59,7 +59,7 @@ macro_rules! outer
 struct S /* 0#0 */;
 macro inner /* 0#3 */ { () => { print_bang! { struct S; } } }
 
-struct S /* 0#4 */;
+struct S /* 0#5 */;
 // OK, not a duplicate definition of `S`
 
 fn main /* 0#0 */() {}
@@ -70,7 +70,7 @@ crate0::{{expn0}}: parent: crate0::{{expn0}}, call_site_ctxt: #0, def_site_ctxt:
 crate0::{{expn1}}: parent: crate0::{{expn0}}, call_site_ctxt: #0, def_site_ctxt: #0, kind: AstPass(StdImports)
 crate0::{{expn2}}: parent: crate0::{{expn0}}, call_site_ctxt: #0, def_site_ctxt: #0, kind: Macro(Bang, "outer")
 crate0::{{expn3}}: parent: crate0::{{expn2}}, call_site_ctxt: #3, def_site_ctxt: #3, kind: Macro(Bang, "inner")
-crate0::{{expn4}}: parent: crate0::{{expn3}}, call_site_ctxt: #5, def_site_ctxt: #0, kind: Macro(Bang, "print_bang")
+crate0::{{expn4}}: parent: crate0::{{expn3}}, call_site_ctxt: #4, def_site_ctxt: #0, kind: Macro(Bang, "print_bang")
 crate1::{{expnNNN}}: parent: crate0::{{expn0}}, call_site_ctxt: #0, def_site_ctxt: #0, kind: Macro(Attr, "diagnostic::on_unimplemented")
 crate1::{{expnNNN}}: parent: crate0::{{expn0}}, call_site_ctxt: #0, def_site_ctxt: #0, kind: Macro(Attr, "diagnostic::on_unimplemented")
 crate1::{{expnNNN}}: parent: crate0::{{expn0}}, call_site_ctxt: #0, def_site_ctxt: #0, kind: Macro(Attr, "diagnostic::on_unimplemented")
@@ -83,9 +83,9 @@ SyntaxContexts:
 #1: parent: #0, outer_mark: (crate0::{{expn1}}, Opaque)
 #2: parent: #0, outer_mark: (crate0::{{expn1}}, Transparent)
 #3: parent: #0, outer_mark: (crate0::{{expn2}}, SemiTransparent)
-#4: parent: #0, outer_mark: (crate0::{{expn3}}, Opaque)
-#5: parent: #3, outer_mark: (crate0::{{expn3}}, Opaque)
+#4: parent: #3, outer_mark: (crate0::{{expn3}}, Opaque)
+#5: parent: #0, outer_mark: (crate0::{{expn3}}, Opaque)
 #6: parent: #0, outer_mark: (crate0::{{expn4}}, Opaque)
-#7: parent: #5, outer_mark: (crate0::{{expn4}}, Transparent)
-#8: parent: #4, outer_mark: (crate0::{{expn4}}, SemiTransparent)
+#7: parent: #4, outer_mark: (crate0::{{expn4}}, Transparent)
+#8: parent: #5, outer_mark: (crate0::{{expn4}}, SemiTransparent)
 */
