From ec862703fde44d23f7da8bc5fdd94a63354d5daf Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Wed, 11 Mar 2020 20:05:19 +0000 Subject: [PATCH] Make macro metavars respect (non-)hygiene --- src/librustc_expand/mbe/macro_check.rs | 13 +++++---- src/librustc_expand/mbe/macro_parser.rs | 17 ++++++----- src/librustc_expand/mbe/macro_rules.rs | 6 ++-- src/librustc_expand/mbe/transcribe.rs | 21 ++++++++------ src/librustc_span/symbol.rs | 25 ++++++++++++++++ src/test/ui/hygiene/macro-metavars-legacy.rs | 29 +++++++++++++++++++ .../ui/hygiene/macro-metavars-transparent.rs | 24 +++++++++++++++ 7 files changed, 111 insertions(+), 24 deletions(-) create mode 100644 src/test/ui/hygiene/macro-metavars-legacy.rs create mode 100644 src/test/ui/hygiene/macro-metavars-transparent.rs diff --git a/src/librustc_expand/mbe/macro_check.rs b/src/librustc_expand/mbe/macro_check.rs index 8c6bfbec902..582c26162ed 100644 --- a/src/librustc_expand/mbe/macro_check.rs +++ b/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::MacroRulesNormalizedIdent, MultiSpan, Span}; use smallvec::SmallVec; @@ -179,7 +179,7 @@ struct BinderInfo { } /// An environment of meta-variables to their binder information. -type Binders = FxHashMap; +type Binders = FxHashMap; /// 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 = MacroRulesNormalizedIdent::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 = MacroRulesNormalizedIdent::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: MacroRulesNormalizedIdent, ) -> 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 = MacroRulesNormalizedIdent::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: MacroRulesNormalizedIdent, ) { 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: MacroRulesNormalizedIdent, binder_ops: &[KleeneToken], occurrence_ops: &[KleeneToken], ) { diff --git a/src/librustc_expand/mbe/macro_parser.rs b/src/librustc_expand/mbe/macro_parser.rs index 6d4d7f5b4f3..3b9158f4445 100644 --- a/src/librustc_expand/mbe/macro_parser.rs +++ b/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, MacroRulesNormalizedIdent, Symbol}; use rustc_errors::{FatalError, PResult}; use rustc_span::Span; @@ -273,9 +273,10 @@ crate enum ParseResult { 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>; +/// A `ParseResult` where the `Success` variant contains a mapping of +/// `MacroRulesNormalizedIdent`s to `NamedMatch`es. This represents the mapping +/// of metavars to the token trees they bind to. +crate type NamedParseResult = ParseResult>; /// 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>( sess: &ParseSess, m: &TokenTree, res: &mut I, - ret_val: &mut FxHashMap, + ret_val: &mut FxHashMap, ) -> Result<(), (rustc_span::Span, String)> { match *m { TokenTree::Sequence(_, ref seq) => { @@ -386,7 +387,9 @@ fn nameize>( 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(MacroRulesNormalizedIdent::new(bind_name)) + { Vacant(spot) => { spot.insert(res.next().unwrap()); } diff --git a/src/librustc_expand/mbe/macro_rules.rs b/src/librustc_expand/mbe/macro_rules.rs index 3cad3ff55d9..2268c9b3854 100644 --- a/src/librustc_expand/mbe/macro_rules.rs +++ b/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, MacroRulesNormalizedIdent, 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[&MacroRulesNormalizedIdent::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[&MacroRulesNormalizedIdent::new(rhs_nm)] { MatchedSeq(ref s) => s .iter() .map(|m| { diff --git a/src/librustc_expand/mbe/transcribe.rs b/src/librustc_expand/mbe/transcribe.rs index 7a64d40785e..1b1093c9529 100644 --- a/src/librustc_expand/mbe/transcribe.rs +++ b/src/librustc_expand/mbe/transcribe.rs @@ -2,7 +2,7 @@ use crate::base::ExtCtxt; use crate::mbe; use crate::mbe::macro_parser::{MatchedNonterminal, MatchedSeq, NamedMatch}; -use rustc_ast::ast::{Ident, MacCall}; +use rustc_ast::ast::MacCall; use rustc_ast::mut_visit::{self, MutVisitor}; use rustc_ast::token::{self, NtTT, Token}; use rustc_ast::tokenstream::{DelimSpan, TokenStream, TokenTree, TreeAndJoint}; @@ -10,6 +10,7 @@ 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::MacroRulesNormalizedIdent; 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, + interp: &FxHashMap, src: Vec, 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 = MacroRulesNormalizedIdent::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: MacroRulesNormalizedIdent, + interpolations: &'a FxHashMap, 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, MacroRulesNormalizedIdent), /// 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, + interpolations: &FxHashMap, repeats: &[(usize, usize)], ) -> LockstepIterSize { use mbe::TokenTree; @@ -376,6 +378,7 @@ fn lockstep_iter_size( }) } TokenTree::MetaVar(_, name) | TokenTree::MetaVarDecl(_, name, _) => { + let name = MacroRulesNormalizedIdent::new(name); match lookup_cur_matched(name, interpolations, repeats) { Some(matched) => match matched { MatchedNonterminal(_) => LockstepIterSize::Unconstrained, diff --git a/src/librustc_span/symbol.rs b/src/librustc_span/symbol.rs index 5760e1d004e..7dfd6c58046 100644 --- a/src/librustc_span/symbol.rs +++ b/src/librustc_span/symbol.rs @@ -979,6 +979,31 @@ impl fmt::Display for IdentPrinter { } } +/// An newtype around `Ident` that calls [Ident::normalize_to_macro_rules] 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 MacroRulesNormalizedIdent(Ident); + +impl MacroRulesNormalizedIdent { + pub fn new(ident: Ident) -> Self { + Self(ident.normalize_to_macro_rules()) + } +} + +impl fmt::Debug for MacroRulesNormalizedIdent { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Debug::fmt(&self.0, f) + } +} + +impl fmt::Display for MacroRulesNormalizedIdent { + 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 diff --git a/src/test/ui/hygiene/macro-metavars-legacy.rs b/src/test/ui/hygiene/macro-metavars-legacy.rs new file mode 100644 index 00000000000..09070f0f561 --- /dev/null +++ b/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)); +} diff --git a/src/test/ui/hygiene/macro-metavars-transparent.rs b/src/test/ui/hygiene/macro-metavars-transparent.rs new file mode 100644 index 00000000000..e475b5728a0 --- /dev/null +++ b/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)); +}