Skip to content

Commit

Permalink
Auto merge of #35453 - jseyfried:hygienize_metavariables, r=nrc
Browse files Browse the repository at this point in the history
macros: Make metavariables hygienic

This PR makes metavariables hygienic. For example, consider:
```rust
macro_rules! foo {
    ($x:tt) => { // Suppose that this token tree argument is always a metavariable.
        macro_rules! bar { ($x:expr, $y:expr) => { ($x, $y) } }
    }
}

fn main() {
    foo!($z); // This currently compiles.
    foo!($y); // This is an error today but compiles after this PR.
}
```
Today, the `macro_rules! bar { ... }` definition is only valid when the metavariable passed to `foo` is not `$y` (since it unhygienically conflicts with the `$y` in the definition of `bar`) or `$x` (c.f. #35450).

After this PR, the definition of `bar` is always valid (and `bar!(a, b)` always expands to `(a, b)` as expected).

This can break code that was allowed in #34925 (landed two weeks ago). For example,
```rust
macro_rules! outer {
    ($t:tt) => {
        macro_rules! inner { ($i:item) => { $t } }
    }
}

outer!($i); // This `$i` should not interact with the `$i` in the definition of `inner!`.
inner!(fn main() {}); // After this PR, this is an error ("unknown macro variable `i`").
```

Due to the severe limitations on nested `macro_rules!` before #34925, this is not a breaking change for stable/beta.

Fixes #35450.

r? @nrc
  • Loading branch information
bors committed Aug 14, 2016
2 parents 2b7ea14 + 95b68aa commit eec30ea
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 14 deletions.
10 changes: 5 additions & 5 deletions src/libsyntax/ext/tt/macro_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ pub use self::ParseResult::*;
use self::TokenTreeOrTokenTreeVec::*;

use ast;
use ast::{Name, Ident};
use ast::Ident;
use syntax_pos::{self, BytePos, mk_sp, Span};
use codemap::Spanned;
use errors::FatalError;
Expand Down Expand Up @@ -202,9 +202,9 @@ pub enum NamedMatch {
}

pub fn nameize(p_s: &ParseSess, ms: &[TokenTree], res: &[Rc<NamedMatch>])
-> ParseResult<HashMap<Name, Rc<NamedMatch>>> {
-> ParseResult<HashMap<Ident, Rc<NamedMatch>>> {
fn n_rec(p_s: &ParseSess, m: &TokenTree, res: &[Rc<NamedMatch>],
ret_val: &mut HashMap<Name, Rc<NamedMatch>>, idx: &mut usize)
ret_val: &mut HashMap<Ident, Rc<NamedMatch>>, idx: &mut usize)
-> Result<(), (syntax_pos::Span, String)> {
match *m {
TokenTree::Sequence(_, ref seq) => {
Expand All @@ -218,7 +218,7 @@ pub fn nameize(p_s: &ParseSess, ms: &[TokenTree], res: &[Rc<NamedMatch>])
}
}
TokenTree::Token(sp, MatchNt(bind_name, _)) => {
match ret_val.entry(bind_name.name) {
match ret_val.entry(bind_name) {
Vacant(spot) => {
spot.insert(res[*idx].clone());
*idx += 1;
Expand Down Expand Up @@ -257,7 +257,7 @@ pub enum ParseResult<T> {
Error(syntax_pos::Span, String)
}

pub type NamedParseResult = ParseResult<HashMap<Name, Rc<NamedMatch>>>;
pub type NamedParseResult = ParseResult<HashMap<Ident, Rc<NamedMatch>>>;
pub type PositionalParseResult = ParseResult<Vec<Rc<NamedMatch>>>;

/// Perform a token equality check, ignoring syntax context (that is, an
Expand Down
4 changes: 2 additions & 2 deletions src/libsyntax/ext/tt/macro_rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ pub fn compile<'cx>(cx: &'cx mut ExtCtxt,
let mut valid = true;

// Extract the arguments:
let lhses = match **argument_map.get(&lhs_nm.name).unwrap() {
let lhses = match **argument_map.get(&lhs_nm).unwrap() {
MatchedSeq(ref s, _) => {
s.iter().map(|m| match **m {
MatchedNonterminal(NtTT(ref tt)) => {
Expand All @@ -315,7 +315,7 @@ pub fn compile<'cx>(cx: &'cx mut ExtCtxt,
_ => cx.span_bug(def.span, "wrong-structured lhs")
};

let rhses = match **argument_map.get(&rhs_nm.name).unwrap() {
let rhses = match **argument_map.get(&rhs_nm).unwrap() {
MatchedSeq(ref s, _) => {
s.iter().map(|m| match **m {
MatchedNonterminal(NtTT(ref tt)) => (**tt).clone(),
Expand Down
10 changes: 5 additions & 5 deletions src/libsyntax/ext/tt/transcribe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
// except according to those terms.
use self::LockstepIterSize::*;

use ast::{Ident, Name};
use ast::Ident;
use syntax_pos::{Span, DUMMY_SP};
use errors::{Handler, DiagnosticBuilder};
use ext::tt::macro_parser::{NamedMatch, MatchedSeq, MatchedNonterminal};
Expand Down Expand Up @@ -38,7 +38,7 @@ pub struct TtReader<'a> {
/// the unzipped tree:
stack: Vec<TtFrame>,
/* for MBE-style macro transcription */
interpolations: HashMap<Name, Rc<NamedMatch>>,
interpolations: HashMap<Ident, Rc<NamedMatch>>,
imported_from: Option<Ident>,

// Some => return imported_from as the next token
Expand All @@ -57,7 +57,7 @@ pub struct TtReader<'a> {
/// `src` contains no `TokenTree::Sequence`s, `MatchNt`s or `SubstNt`s, `interp` can
/// (and should) be None.
pub fn new_tt_reader(sp_diag: &Handler,
interp: Option<HashMap<Name, Rc<NamedMatch>>>,
interp: Option<HashMap<Ident, Rc<NamedMatch>>>,
imported_from: Option<Ident>,
src: Vec<tokenstream::TokenTree>)
-> TtReader {
Expand All @@ -71,7 +71,7 @@ pub fn new_tt_reader(sp_diag: &Handler,
/// `src` contains no `TokenTree::Sequence`s, `MatchNt`s or `SubstNt`s, `interp` can
/// (and should) be None.
pub fn new_tt_reader_with_doc_flag(sp_diag: &Handler,
interp: Option<HashMap<Name, Rc<NamedMatch>>>,
interp: Option<HashMap<Ident, Rc<NamedMatch>>>,
imported_from: Option<Ident>,
src: Vec<tokenstream::TokenTree>,
desugar_doc_comments: bool)
Expand Down Expand Up @@ -119,7 +119,7 @@ fn lookup_cur_matched_by_matched(r: &TtReader, start: Rc<NamedMatch>) -> Rc<Name
}

fn lookup_cur_matched(r: &TtReader, name: Ident) -> Option<Rc<NamedMatch>> {
let matched_opt = r.interpolations.get(&name.name).cloned();
let matched_opt = r.interpolations.get(&name).cloned();
matched_opt.map(|s| lookup_cur_matched_by_matched(r, s))
}

Expand Down
16 changes: 16 additions & 0 deletions src/test/compile-fail/issue-35450.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

macro_rules! m { ($t:tt) => { $t } }

fn main() {
m!($t); //~ ERROR unknown macro variable
//~| ERROR expected expression
}
13 changes: 12 additions & 1 deletion src/test/compile-fail/macro-tt-matchers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,16 @@ macro_rules! foo {

foo!(Box);

macro_rules! bar {
($x:tt) => {
macro_rules! baz {
($x:tt, $y:tt) => { ($x, $y) }
}
}
}

#[rustc_error]
fn main() {} //~ ERROR compilation successful
fn main() { //~ ERROR compilation successful
bar!($y);
let _: (i8, i16) = baz!(0i8, 0i16);
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ fn expand_mbe_matches(cx: &mut ExtCtxt, sp: Span, args: &[TokenTree])

let mac_expr = match TokenTree::parse(cx, &mbe_matcher[..], args) {
Success(map) => {
match (&*map[&str_to_ident("matched").name], &*map[&str_to_ident("pat").name]) {
match (&*map[&str_to_ident("matched")], &*map[&str_to_ident("pat")]) {
(&MatchedNonterminal(NtExpr(ref matched_expr)),
&MatchedSeq(ref pats, seq_sp)) => {
let pats: Vec<P<Pat>> = pats.iter().map(|pat_nt|
Expand Down

0 comments on commit eec30ea

Please sign in to comment.