hygiene: Require passing transparency explicitly to apply_mark

Author: petrochenkov

src/libsyntax/ext/expand.rs

@@ -1388,17 +1388,3 @@ impl<'feat> ExpansionConfig<'feat> {
         self.features.map_or(false, |features| features.custom_inner_attributes)
     }
 }
-
-// A Marker adds the given mark to the syntax context.
-#[derive(Debug)]
-pub struct Marker(pub ExpnId);
-
-impl MutVisitor for Marker {
-    fn visit_span(&mut self, span: &mut Span) {
-        *span = span.apply_mark(self.0)
-    }
-
-    fn visit_mac(&mut self, mac: &mut ast::Mac) {
-        noop_visit_mac(mac, self)
-    }
-}

src/libsyntax/ext/tt/macro_rules.rs

@@ -19,6 +19,7 @@ use crate::{ast, attr, attr::TransparencyError};
 
 use errors::{DiagnosticBuilder, FatalError};
 use log::debug;
+use syntax_pos::hygiene::Transparency;
 use syntax_pos::Span;
 
 use rustc_data_structures::fx::FxHashMap;
@@ -128,6 +129,7 @@ impl<'a> ParserAnyMacro<'a> {
 struct MacroRulesMacroExpander {
     name: ast::Ident,
     span: Span,
+    transparency: Transparency,
     lhses: Vec<quoted::TokenTree>,
     rhses: Vec<quoted::TokenTree>,
     valid: bool,
@@ -143,7 +145,9 @@ impl TTMacroExpander for MacroRulesMacroExpander {
         if !self.valid {
             return DummyResult::any(sp);
         }
-        generic_extension(cx, sp, self.span, self.name, input, &self.lhses, &self.rhses)
+        generic_extension(
+            cx, sp, self.span, self.name, self.transparency, input, &self.lhses, &self.rhses
+        )
     }
 }
 
@@ -158,6 +162,7 @@ fn generic_extension<'cx>(
     sp: Span,
     def_span: Span,
     name: ast::Ident,
+    transparency: Transparency,
     arg: TokenStream,
     lhses: &[quoted::TokenTree],
     rhses: &[quoted::TokenTree],
@@ -187,7 +192,7 @@ fn generic_extension<'cx>(
 
                 let rhs_spans = rhs.iter().map(|t| t.span()).collect::<Vec<_>>();
                 // rhs has holes ( `$id` and `$(...)` that need filled)
-                let mut tts = transcribe(cx, &named_matches, rhs);
+                let mut tts = transcribe(cx, &named_matches, rhs, transparency);
 
                 // Replace all the tokens for the corresponding positions in the macro, to maintain
                 // proper positions in error reporting, while maintaining the macro_backtrace.
@@ -415,11 +420,7 @@ pub fn compile(
     // that is not lint-checked and trigger the "failed to process buffered lint here" bug.
     valid &= macro_check::check_meta_variables(sess, ast::CRATE_NODE_ID, def.span, &lhses, &rhses);
 
-    let expander: Box<_> =
-        Box::new(MacroRulesMacroExpander { name: def.ident, span: def.span, lhses, rhses, valid });
-
-    let (default_transparency, transparency_error) =
-        attr::find_transparency(&def.attrs, body.legacy);
+    let (transparency, transparency_error) = attr::find_transparency(&def.attrs, body.legacy);
     match transparency_error {
         Some(TransparencyError::UnknownTransparency(value, span)) =>
             sess.span_diagnostic.span_err(
@@ -432,6 +433,10 @@ pub fn compile(
         None => {}
     }
 
+    let expander: Box<_> = Box::new(MacroRulesMacroExpander {
+        name: def.ident, span: def.span, transparency, lhses, rhses, valid
+    });
+
     let allow_internal_unstable =
         attr::find_by_name(&def.attrs, sym::allow_internal_unstable).map(|attr| {
             attr.meta_item_list()
@@ -473,7 +478,7 @@ pub fn compile(
     SyntaxExtension {
         kind: SyntaxExtensionKind::LegacyBang(expander),
         span: def.span,
-        default_transparency,
+        default_transparency: transparency,
         allow_internal_unstable,
         allow_internal_unsafe: attr::contains_name(&def.attrs, sym::allow_internal_unsafe),
         local_inner_macros,

src/libsyntax/ext/tt/transcribe.rs

@@ -1,18 +1,40 @@
-use crate::ast::Ident;
+use crate::ast::{Ident, Mac};
 use crate::ext::base::ExtCtxt;
-use crate::ext::expand::Marker;
 use crate::ext::tt::macro_parser::{MatchedNonterminal, MatchedSeq, NamedMatch};
 use crate::ext::tt::quoted;
-use crate::mut_visit::noop_visit_tt;
+use crate::mut_visit::{self, MutVisitor};
 use crate::parse::token::{self, NtTT, Token};
 use crate::tokenstream::{DelimSpan, TokenStream, TokenTree, TreeAndJoint};
 
 use smallvec::{smallvec, SmallVec};
 
 use rustc_data_structures::fx::FxHashMap;
 use rustc_data_structures::sync::Lrc;
+use syntax_pos::hygiene::{ExpnId, Transparency};
+use syntax_pos::Span;
+
 use std::mem;
 
+// A Marker adds the given mark to the syntax context.
+struct Marker(ExpnId, Transparency);
+
+impl MutVisitor for Marker {
+    fn visit_span(&mut self, span: &mut Span) {
+        *span = span.apply_mark(self.0, self.1)
+    }
+
+    fn visit_mac(&mut self, mac: &mut Mac) {
+        mut_visit::noop_visit_mac(mac, self)
+    }
+}
+
+impl Marker {
+    fn visit_delim_span(&mut self, dspan: &mut DelimSpan) {
+        self.visit_span(&mut dspan.open);
+        self.visit_span(&mut dspan.close);
+    }
+}
+
 /// An iterator over the token trees in a delimited token tree (`{ ... }`) or a sequence (`$(...)`).
 enum Frame {
     Delimited { forest: Lrc<quoted::Delimited>, idx: usize, span: DelimSpan },
@@ -68,6 +90,7 @@ pub(super) fn transcribe(
     cx: &ExtCtxt<'_>,
     interp: &FxHashMap<Ident, NamedMatch>,
     src: Vec<quoted::TokenTree>,
+    transparency: Transparency,
 ) -> TokenStream {
     // Nothing for us to transcribe...
     if src.is_empty() {
@@ -96,6 +119,7 @@ pub(super) fn transcribe(
     // again, and we are done transcribing.
     let mut result: Vec<TreeAndJoint> = Vec::new();
     let mut result_stack = Vec::new();
+    let mut marker = Marker(cx.current_expansion.id, transparency);
 
     loop {
         // Look at the last frame on the stack.
@@ -207,7 +231,7 @@ pub(super) fn transcribe(
             }
 
             // Replace the meta-var with the matched token tree from the invocation.
-            quoted::TokenTree::MetaVar(mut sp, ident) => {
+            quoted::TokenTree::MetaVar(mut sp, mut ident) => {
                 // Find the matched nonterminal from the macro invocation, and use it to replace
                 // the meta-var.
                 if let Some(cur_matched) = lookup_cur_matched(ident, interp, &repeats) {
@@ -218,7 +242,7 @@ pub(super) fn transcribe(
                         if let NtTT(ref tt) = **nt {
                             result.push(tt.clone().into());
                         } else {
-                            sp = sp.apply_mark(cx.current_expansion.id);
+                            marker.visit_span(&mut sp);
                             let token = TokenTree::token(token::Interpolated(nt.clone()), sp);
                             result.push(token.into());
                         }
@@ -232,9 +256,8 @@ pub(super) fn transcribe(
                 } else {
                     // If we aren't able to match the meta-var, we push it back into the result but
                     // with modified syntax context. (I believe this supports nested macros).
-                    let ident =
-                        Ident::new(ident.name, ident.span.apply_mark(cx.current_expansion.id));
-                    sp = sp.apply_mark(cx.current_expansion.id);
+                    marker.visit_span(&mut sp);
+                    marker.visit_ident(&mut ident);
                     result.push(TokenTree::token(token::Dollar, sp).into());
                     result.push(TokenTree::Token(Token::from_ast_ident(ident)).into());
                 }
@@ -246,17 +269,16 @@ pub(super) fn transcribe(
             // jump back out of the Delimited, pop the result_stack and add the new results back to
             // the previous results (from outside the Delimited).
             quoted::TokenTree::Delimited(mut span, delimited) => {
-                span = span.apply_mark(cx.current_expansion.id);
+                marker.visit_delim_span(&mut span);
                 stack.push(Frame::Delimited { forest: delimited, idx: 0, span });
                 result_stack.push(mem::take(&mut result));
             }
 
             // Nothing much to do here. Just push the token to the result, being careful to
             // preserve syntax context.
             quoted::TokenTree::Token(token) => {
-                let mut marker = Marker(cx.current_expansion.id);
                 let mut tt = TokenTree::Token(token);
-                noop_visit_tt(&mut tt, &mut marker);
+                marker.visit_tt(&mut tt);
                 result.push(tt.into());
             }
 

src/libsyntax/tokenstream.rs

@@ -19,7 +19,7 @@ use crate::parse::Directory;
 use crate::parse::token::{self, DelimToken, Token, TokenKind};
 use crate::print::pprust;
 
-use syntax_pos::{BytePos, ExpnId, Span, DUMMY_SP};
+use syntax_pos::{BytePos, Span, DUMMY_SP};
 #[cfg(target_arch = "x86_64")]
 use rustc_data_structures::static_assert_size;
 use rustc_data_structures::sync::Lrc;
@@ -547,11 +547,4 @@ impl DelimSpan {
     pub fn entire(self) -> Span {
         self.open.with_hi(self.close.hi())
     }
-
-    pub fn apply_mark(self, expn_id: ExpnId) -> Self {
-        DelimSpan {
-            open: self.open.apply_mark(expn_id),
-            close: self.close.apply_mark(expn_id),
-        }
-    }
 }

src/libsyntax_pos/hygiene.rs

@@ -192,10 +192,10 @@ impl HygieneData {
         self.syntax_context_data[ctxt.0 as usize].parent
     }
 
-    fn remove_mark(&self, ctxt: &mut SyntaxContext) -> ExpnId {
-        let outer_expn = self.outer_expn(*ctxt);
+    fn remove_mark(&self, ctxt: &mut SyntaxContext) -> (ExpnId, Transparency) {
+        let outer_mark = self.outer_mark(*ctxt);
         *ctxt = self.parent_ctxt(*ctxt);
-        outer_expn
+        outer_mark
     }
 
     fn marks(&self, mut ctxt: SyntaxContext) -> Vec<(ExpnId, Transparency)> {
@@ -218,20 +218,14 @@ impl HygieneData {
     fn adjust(&self, ctxt: &mut SyntaxContext, expn_id: ExpnId) -> Option<ExpnId> {
         let mut scope = None;
         while !self.is_descendant_of(expn_id, self.outer_expn(*ctxt)) {
-            scope = Some(self.remove_mark(ctxt));
+            scope = Some(self.remove_mark(ctxt).0);
         }
         scope
     }
 
-    fn apply_mark(&mut self, ctxt: SyntaxContext, expn_id: ExpnId) -> SyntaxContext {
-        assert_ne!(expn_id, ExpnId::root());
-        self.apply_mark_with_transparency(
-            ctxt, expn_id, self.expn_data(expn_id).default_transparency
-        )
-    }
-
-    fn apply_mark_with_transparency(&mut self, ctxt: SyntaxContext, expn_id: ExpnId,
-                                    transparency: Transparency) -> SyntaxContext {
+    fn apply_mark(
+        &mut self, ctxt: SyntaxContext, expn_id: ExpnId, transparency: Transparency
+    ) -> SyntaxContext {
         assert_ne!(expn_id, ExpnId::root());
         if transparency == Transparency::Opaque {
             return self.apply_mark_internal(ctxt, expn_id, transparency);
@@ -365,15 +359,9 @@ impl SyntaxContext {
         SyntaxContext(raw)
     }
 
-    /// Extend a syntax context with a given expansion and default transparency for that expansion.
-    pub fn apply_mark(self, expn_id: ExpnId) -> SyntaxContext {
-        HygieneData::with(|data| data.apply_mark(self, expn_id))
-    }
-
     /// Extend a syntax context with a given expansion and transparency.
-    pub fn apply_mark_with_transparency(self, expn_id: ExpnId, transparency: Transparency)
-                                        -> SyntaxContext {
-        HygieneData::with(|data| data.apply_mark_with_transparency(self, expn_id, transparency))
+    pub fn apply_mark(self, expn_id: ExpnId, transparency: Transparency) -> SyntaxContext {
+        HygieneData::with(|data| data.apply_mark(self, expn_id, transparency))
     }
 
     /// Pulls a single mark off of the syntax context. This effectively moves the
@@ -393,7 +381,7 @@ impl SyntaxContext {
     /// invocation of f that created g1.
     /// Returns the mark that was removed.
     pub fn remove_mark(&mut self) -> ExpnId {
-        HygieneData::with(|data| data.remove_mark(self))
+        HygieneData::with(|data| data.remove_mark(self).0)
     }
 
     pub fn marks(self) -> Vec<(ExpnId, Transparency)> {
@@ -466,8 +454,8 @@ impl SyntaxContext {
             let mut scope = None;
             let mut glob_ctxt = data.modern(glob_span.ctxt());
             while !data.is_descendant_of(expn_id, data.outer_expn(glob_ctxt)) {
-                scope = Some(data.remove_mark(&mut glob_ctxt));
-                if data.remove_mark(self) != scope.unwrap() {
+                scope = Some(data.remove_mark(&mut glob_ctxt).0);
+                if data.remove_mark(self).0 != scope.unwrap() {
                     return None;
                 }
             }
@@ -498,9 +486,9 @@ impl SyntaxContext {
                 marks.push(data.remove_mark(&mut glob_ctxt));
             }
 
-            let scope = marks.last().cloned();
-            while let Some(mark) = marks.pop() {
-                *self = data.apply_mark(*self, mark);
+            let scope = marks.last().map(|mark| mark.0);
+            while let Some((expn_id, transparency)) = marks.pop() {
+                *self = data.apply_mark(*self, expn_id, transparency);
             }
             Some(scope)
         })
@@ -571,9 +559,7 @@ impl Span {
     ) -> Span {
         HygieneData::with(|data| {
             let expn_id = data.fresh_expn(Some(expn_data));
-            self.with_ctxt(data.apply_mark_with_transparency(
-                SyntaxContext::root(), expn_id, transparency
-            ))
+            self.with_ctxt(data.apply_mark(SyntaxContext::root(), expn_id, transparency))
         })
     }
 }

src/libsyntax_pos/lib.rs

@@ -517,13 +517,13 @@ impl Span {
     /// given ID and transparency, assuming that macro was defined directly and not produced by
     /// some other macro (which is the case for built-in and procedural macros).
     pub fn with_ctxt_from_mark(self, expn_id: ExpnId, transparency: Transparency) -> Span {
-        self.with_ctxt(SyntaxContext::root().apply_mark_with_transparency(expn_id, transparency))
+        self.with_ctxt(SyntaxContext::root().apply_mark(expn_id, transparency))
     }
 
     #[inline]
-    pub fn apply_mark(self, mark: ExpnId) -> Span {
+    pub fn apply_mark(self, expn_id: ExpnId, transparency: Transparency) -> Span {
         let span = self.data();
-        span.with_ctxt(span.ctxt.apply_mark(mark))
+        span.with_ctxt(span.ctxt.apply_mark(expn_id, transparency))
     }
 
     #[inline]