Skip to content

Commit

Permalink
Rollup merge of rust-lang#68746 - matthewjasper:metahygiene, r=petroc…
Browse files Browse the repository at this point in the history
…henkov

Make macro metavars respect (non-)hygiene

This makes them more consistent with other name resolution while not breaking any code on crater.
  • Loading branch information
Centril authored Mar 17, 2020
2 parents 5e9ebf4 + ec86270 commit 34afc76
Show file tree
Hide file tree
Showing 7 changed files with 111 additions and 24 deletions.
13 changes: 8 additions & 5 deletions src/librustc_expand/mbe/macro_check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -179,7 +179,7 @@ struct BinderInfo {
}

/// An environment of meta-variables to their binder information.
type Binders = FxHashMap<Ident, BinderInfo>;
type Binders = FxHashMap<MacroRulesNormalizedIdent, BinderInfo>;

/// The state at which we entered a macro definition in the RHS of another macro definition.
struct MacroState<'a> {
Expand Down Expand Up @@ -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.
Expand All @@ -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.
Expand Down Expand Up @@ -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)))
}
Expand Down Expand Up @@ -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) => {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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],
) {
Expand Down
17 changes: 10 additions & 7 deletions src/librustc_expand/mbe/macro_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
/// `MacroRulesNormalizedIdent`s to `NamedMatch`es. This represents the mapping
/// of metavars to the token trees they bind to.
crate type NamedParseResult = ParseResult<FxHashMap<MacroRulesNormalizedIdent, NamedMatch>>;

/// Count how many metavars are named in the given matcher `ms`.
pub(super) fn count_names(ms: &[TokenTree]) -> usize {
Expand Down Expand Up @@ -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<MacroRulesNormalizedIdent, NamedMatch>,
) -> Result<(), (rustc_span::Span, String)> {
match *m {
TokenTree::Sequence(_, ref seq) => {
Expand All @@ -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(MacroRulesNormalizedIdent::new(bind_name))
{
Vacant(spot) => {
spot.insert(res.next().unwrap());
}
Expand Down
6 changes: 3 additions & 3 deletions src/librustc_expand/mbe/macro_rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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| {
Expand All @@ -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| {
Expand Down
21 changes: 12 additions & 9 deletions src/librustc_expand/mbe/transcribe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,15 @@ 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};
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};
Expand Down Expand Up @@ -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<MacroRulesNormalizedIdent, NamedMatch>,
src: Vec<mbe::TokenTree>,
transparency: Transparency,
) -> TokenStream {
Expand Down Expand Up @@ -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
Expand All @@ -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());
}
}

Expand Down Expand Up @@ -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: MacroRulesNormalizedIdent,
interpolations: &'a FxHashMap<MacroRulesNormalizedIdent, NamedMatch>,
repeats: &[(usize, usize)],
) -> Option<&'a NamedMatch> {
interpolations.get(&ident).map(|matched| {
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -360,7 +362,7 @@ impl LockstepIterSize {
/// multiple nested matcher sequences.
fn lockstep_iter_size(
tree: &mbe::TokenTree,
interpolations: &FxHashMap<Ident, NamedMatch>,
interpolations: &FxHashMap<MacroRulesNormalizedIdent, NamedMatch>,
repeats: &[(usize, usize)],
) -> LockstepIterSize {
use mbe::TokenTree;
Expand All @@ -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,
Expand Down
25 changes: 25 additions & 0 deletions src/librustc_span/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -983,6 +983,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
Expand Down
29 changes: 29 additions & 0 deletions src/test/ui/hygiene/macro-metavars-legacy.rs
Original file line number Diff line number Diff line change
@@ -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));
}
24 changes: 24 additions & 0 deletions src/test/ui/hygiene/macro-metavars-transparent.rs
Original file line number Diff line number Diff line change
@@ -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));
}

0 comments on commit 34afc76

Please sign in to comment.