Make macro metavars respect (non-)hygiene

Author: matthewjasper

src/librustc_expand/mbe/macro_check.rs

@@ -112,7 +112,7 @@ use rustc_data_structures::fx::FxHashMap;
 use rustc_session::lint::builtin::META_VARIABLE_MISUSE;
 use rustc_session::parse::ParseSess;
 use rustc_span::symbol::kw;
-use rustc_span::{symbol::Ident, MultiSpan, Span};
+use rustc_span::{symbol::LegacyIdent, MultiSpan, Span};
 
 use smallvec::SmallVec;
 
@@ -179,7 +179,7 @@ struct BinderInfo {
 }
 
 /// An environment of meta-variables to their binder information.
-type Binders = FxHashMap<Ident, BinderInfo>;
+type Binders = FxHashMap<LegacyIdent, BinderInfo>;
 
 /// The state at which we entered a macro definition in the RHS of another macro definition.
 struct MacroState<'a> {
@@ -245,6 +245,7 @@ fn check_binders(
             if macros.is_empty() {
                 sess.span_diagnostic.span_bug(span, "unexpected MetaVar in lhs");
             }
+            let name = LegacyIdent::new(name);
             // There are 3 possibilities:
             if let Some(prev_info) = binders.get(&name) {
                 // 1. The meta-variable is already bound in the current LHS: This is an error.
@@ -264,6 +265,7 @@ fn check_binders(
             if !macros.is_empty() {
                 sess.span_diagnostic.span_bug(span, "unexpected MetaVarDecl in nested lhs");
             }
+            let name = LegacyIdent::new(name);
             if let Some(prev_info) = get_binder_info(macros, binders, name) {
                 // Duplicate binders at the top-level macro definition are errors. The lint is only
                 // for nested macro definitions.
@@ -300,7 +302,7 @@ fn check_binders(
 fn get_binder_info<'a>(
     mut macros: &'a Stack<'a, MacroState<'a>>,
     binders: &'a Binders,
-    name: Ident,
+    name: LegacyIdent,
 ) -> Option<&'a BinderInfo> {
     binders.get(&name).or_else(|| macros.find_map(|state| state.binders.get(&name)))
 }
@@ -331,6 +333,7 @@ fn check_occurrences(
             sess.span_diagnostic.span_bug(span, "unexpected MetaVarDecl in rhs")
         }
         TokenTree::MetaVar(span, name) => {
+            let name = LegacyIdent::new(name);
             check_ops_is_prefix(sess, node_id, macros, binders, ops, span, name);
         }
         TokenTree::Delimited(_, ref del) => {
@@ -552,7 +555,7 @@ fn check_ops_is_prefix(
     binders: &Binders,
     ops: &Stack<'_, KleeneToken>,
     span: Span,
-    name: Ident,
+    name: LegacyIdent,
 ) {
     let macros = macros.push(MacroState { binders, ops: ops.into() });
     // Accumulates the stacks the operators of each state until (and including when) the
@@ -598,7 +601,7 @@ fn ops_is_prefix(
     sess: &ParseSess,
     node_id: NodeId,
     span: Span,
-    name: Ident,
+    name: LegacyIdent,
     binder_ops: &[KleeneToken],
     occurrence_ops: &[KleeneToken],
 ) {

src/librustc_expand/mbe/macro_parser.rs

@@ -76,13 +76,13 @@ use TokenTreeOrTokenTreeSlice::*;
 
 use crate::mbe::{self, TokenTree};
 
-use rustc_ast::ast::{Ident, Name};
+use rustc_ast::ast::Name;
 use rustc_ast::ptr::P;
 use rustc_ast::token::{self, DocComment, Nonterminal, Token};
 use rustc_ast_pretty::pprust;
 use rustc_parse::parser::{FollowedByType, Parser, PathStyle};
 use rustc_session::parse::ParseSess;
-use rustc_span::symbol::{kw, sym, Symbol};
+use rustc_span::symbol::{kw, sym, Ident, LegacyIdent, Symbol};
 
 use rustc_errors::{FatalError, PResult};
 use rustc_span::Span;
@@ -273,9 +273,10 @@ crate enum ParseResult<T> {
     Error(rustc_span::Span, String),
 }
 
-/// A `ParseResult` where the `Success` variant contains a mapping of `Ident`s to `NamedMatch`es.
-/// This represents the mapping of metavars to the token trees they bind to.
-crate type NamedParseResult = ParseResult<FxHashMap<Ident, NamedMatch>>;
+/// A `ParseResult` where the `Success` variant contains a mapping of
+/// `LegacyIdent`s to `NamedMatch`es. This represents the mapping of metavars
+/// to the token trees they bind to.
+crate type NamedParseResult = ParseResult<FxHashMap<LegacyIdent, NamedMatch>>;
 
 /// Count how many metavars are named in the given matcher `ms`.
 pub(super) fn count_names(ms: &[TokenTree]) -> usize {
@@ -368,7 +369,7 @@ fn nameize<I: Iterator<Item = NamedMatch>>(
         sess: &ParseSess,
         m: &TokenTree,
         res: &mut I,
-        ret_val: &mut FxHashMap<Ident, NamedMatch>,
+        ret_val: &mut FxHashMap<LegacyIdent, NamedMatch>,
     ) -> Result<(), (rustc_span::Span, String)> {
         match *m {
             TokenTree::Sequence(_, ref seq) => {
@@ -386,7 +387,9 @@ fn nameize<I: Iterator<Item = NamedMatch>>(
                     return Err((span, "missing fragment specifier".to_string()));
                 }
             }
-            TokenTree::MetaVarDecl(sp, bind_name, _) => match ret_val.entry(bind_name) {
+            TokenTree::MetaVarDecl(sp, bind_name, _) => match ret_val
+                .entry(LegacyIdent::new(bind_name))
+            {
                 Vacant(spot) => {
                     spot.insert(res.next().unwrap());
                 }

src/librustc_expand/mbe/macro_rules.rs

@@ -22,7 +22,7 @@ use rustc_parse::Directory;
 use rustc_session::parse::ParseSess;
 use rustc_span::edition::Edition;
 use rustc_span::hygiene::Transparency;
-use rustc_span::symbol::{kw, sym, Symbol};
+use rustc_span::symbol::{kw, sym, LegacyIdent, Symbol};
 use rustc_span::Span;
 
 use log::debug;
@@ -411,7 +411,7 @@ pub fn compile_declarative_macro(
     let mut valid = true;
 
     // Extract the arguments:
-    let lhses = match argument_map[&lhs_nm] {
+    let lhses = match argument_map[&LegacyIdent::new(lhs_nm)] {
         MatchedSeq(ref s) => s
             .iter()
             .map(|m| {
@@ -428,7 +428,7 @@ pub fn compile_declarative_macro(
         _ => sess.span_diagnostic.span_bug(def.span, "wrong-structured lhs"),
     };
 
-    let rhses = match argument_map[&rhs_nm] {
+    let rhses = match argument_map[&LegacyIdent::new(rhs_nm)] {
         MatchedSeq(ref s) => s
             .iter()
             .map(|m| {

src/librustc_expand/mbe/transcribe.rs

@@ -2,14 +2,15 @@ use crate::base::ExtCtxt;
 use crate::mbe;
 use crate::mbe::macro_parser::{MatchedNonterminal, MatchedSeq, NamedMatch};
 
-use rustc_ast::ast::{Ident, Mac};
+use rustc_ast::ast::Mac;
 use rustc_ast::mut_visit::{self, MutVisitor};
 use rustc_ast::token::{self, NtTT, Token};
 use rustc_ast::tokenstream::{DelimSpan, TokenStream, TokenTree, TreeAndJoint};
 use rustc_data_structures::fx::FxHashMap;
 use rustc_data_structures::sync::Lrc;
 use rustc_errors::pluralize;
 use rustc_span::hygiene::{ExpnId, Transparency};
+use rustc_span::symbol::LegacyIdent;
 use rustc_span::Span;
 
 use smallvec::{smallvec, SmallVec};
@@ -81,7 +82,7 @@ impl Iterator for Frame {
 /// Along the way, we do some additional error checking.
 pub(super) fn transcribe(
     cx: &ExtCtxt<'_>,
-    interp: &FxHashMap<Ident, NamedMatch>,
+    interp: &FxHashMap<LegacyIdent, NamedMatch>,
     src: Vec<mbe::TokenTree>,
     transparency: Transparency,
 ) -> TokenStream {
@@ -223,9 +224,10 @@ pub(super) fn transcribe(
             }
 
             // Replace the meta-var with the matched token tree from the invocation.
-            mbe::TokenTree::MetaVar(mut sp, mut ident) => {
+            mbe::TokenTree::MetaVar(mut sp, mut orignal_ident) => {
                 // Find the matched nonterminal from the macro invocation, and use it to replace
                 // the meta-var.
+                let ident = LegacyIdent::new(orignal_ident);
                 if let Some(cur_matched) = lookup_cur_matched(ident, interp, &repeats) {
                     if let MatchedNonterminal(ref nt) = cur_matched {
                         // FIXME #2887: why do we apply a mark when matching a token tree meta-var
@@ -249,9 +251,9 @@ pub(super) fn transcribe(
                     // 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).
                     marker.visit_span(&mut sp);
-                    marker.visit_ident(&mut ident);
+                    marker.visit_ident(&mut orignal_ident);
                     result.push(TokenTree::token(token::Dollar, sp).into());
-                    result.push(TokenTree::Token(Token::from_ast_ident(ident)).into());
+                    result.push(TokenTree::Token(Token::from_ast_ident(orignal_ident)).into());
                 }
             }
 
@@ -287,8 +289,8 @@ pub(super) fn transcribe(
 /// into the right place in nested matchers. If we attempt to descend too far, the macro writer has
 /// made a mistake, and we return `None`.
 fn lookup_cur_matched<'a>(
-    ident: Ident,
-    interpolations: &'a FxHashMap<Ident, NamedMatch>,
+    ident: LegacyIdent,
+    interpolations: &'a FxHashMap<LegacyIdent, NamedMatch>,
     repeats: &[(usize, usize)],
 ) -> Option<&'a NamedMatch> {
     interpolations.get(&ident).map(|matched| {
@@ -316,7 +318,7 @@ enum LockstepIterSize {
 
     /// A `MetaVar` with an actual `MatchedSeq`. The length of the match and the name of the
     /// meta-var are returned.
-    Constraint(usize, Ident),
+    Constraint(usize, LegacyIdent),
 
     /// Two `Constraint`s on the same sequence had different lengths. This is an error.
     Contradiction(String),
@@ -360,7 +362,7 @@ impl LockstepIterSize {
 /// multiple nested matcher sequences.
 fn lockstep_iter_size(
     tree: &mbe::TokenTree,
-    interpolations: &FxHashMap<Ident, NamedMatch>,
+    interpolations: &FxHashMap<LegacyIdent, NamedMatch>,
     repeats: &[(usize, usize)],
 ) -> LockstepIterSize {
     use mbe::TokenTree;
@@ -376,6 +378,7 @@ fn lockstep_iter_size(
             })
         }
         TokenTree::MetaVar(_, name) | TokenTree::MetaVarDecl(_, name, _) => {
+            let name = LegacyIdent::new(name);
             match lookup_cur_matched(name, interpolations, repeats) {
                 Some(matched) => match matched {
                     MatchedNonterminal(_) => LockstepIterSize::Unconstrained,

src/librustc_span/symbol.rs

@@ -980,6 +980,31 @@ impl fmt::Display for IdentPrinter {
     }
 }
 
+/// An newtype around `Ident` that calls [Ident::modern_and_legacy] on
+/// construction.
+// FIXME(matthewj, petrochenkov) Use this more often, add a similar
+// `ModernIdent` struct and use that as well.
+#[derive(Copy, Clone, Eq, PartialEq, Hash)]
+pub struct LegacyIdent(Ident);
+
+impl LegacyIdent {
+    pub fn new(ident: Ident) -> Self {
+        Self(ident.modern_and_legacy())
+    }
+}
+
+impl fmt::Debug for LegacyIdent {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        fmt::Debug::fmt(&self.0, f)
+    }
+}
+
+impl fmt::Display for LegacyIdent {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        fmt::Display::fmt(&self.0, f)
+    }
+}
+
 /// An interned string.
 ///
 /// Internally, a `Symbol` is implemented as an index, and all operations

src/test/ui/hygiene/macro-metavars-legacy.rs

@@ -0,0 +1,29 @@
+// Ensure macro metavariables are compared with legacy hygiene
+
+#![feature(rustc_attrs)]
+
+// run-pass
+
+macro_rules! make_mac {
+    ( $($dollar:tt $arg:ident),+ ) => {
+        macro_rules! mac {
+            ( $($dollar $arg : ident),+ ) => {
+                $( $dollar $arg )-+
+            }
+        }
+    }
+}
+
+macro_rules! show_hygiene {
+    ( $dollar:tt $arg:ident ) => {
+        make_mac!($dollar $arg, $dollar arg);
+    }
+}
+
+show_hygiene!( $arg );
+
+fn main() {
+    let x = 5;
+    let y = 3;
+    assert_eq!(2, mac!(x, y));
+}

src/test/ui/hygiene/macro-metavars-transparent.rs

@@ -0,0 +1,24 @@
+// Ensure macro metavariables are not compared without removing transparent
+// marks.
+
+#![feature(rustc_attrs)]
+
+// run-pass
+
+#[rustc_macro_transparency = "transparent"]
+macro_rules! k {
+    ($($s:tt)*) => {
+        macro_rules! m {
+            ($y:tt) => {
+                $($s)*
+            }
+        }
+    }
+}
+
+k!(1 + $y);
+
+fn main() {
+    let x = 2;
+    assert_eq!(3, m!(x));
+}