From 076e64475a54bacf7b0f0040e48800bca9b728dd Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Thu, 12 Nov 2015 20:55:28 +0100 Subject: [PATCH 1/5] macro_rules: proper FIRST/FOLLOW computations for checking macro_rules validity. See RFC amendment 1384 and tracking issue 30450: https://github.com/rust-lang/rfcs/pull/1384 https://github.com/rust-lang/rust/issues/30450 Moved old check_matcher code into check_matcher_old combined the two checks to enable a warning cycle (where we will continue to error if the two checks agree to reject, accept if the new check says accept, and warn if the old check accepts but the new check rejects). --- src/libsyntax/ext/tt/macro_rules.rs | 516 +++++++++++++++++++++++++++- 1 file changed, 500 insertions(+), 16 deletions(-) diff --git a/src/libsyntax/ext/tt/macro_rules.rs b/src/libsyntax/ext/tt/macro_rules.rs index c61b91df09249..add10612cd8cf 100644 --- a/src/libsyntax/ext/tt/macro_rules.rs +++ b/src/libsyntax/ext/tt/macro_rules.rs @@ -25,8 +25,9 @@ use ptr::P; use util::small_vector::SmallVector; use std::cell::RefCell; +use std::collections::{HashMap}; +use std::collections::hash_map::{Entry}; use std::rc::Rc; -use std::iter::once; struct ParserAnyMacro<'a> { parser: RefCell>, @@ -320,15 +321,18 @@ pub fn compile<'cx>(cx: &'cx mut ExtCtxt, NormalTT(exp, Some(def.span), def.allow_internal_unstable) } +// why is this here? because of https://github.com/rust-lang/rust/issues/27774 +fn ref_slice(s: &A) -> &[A] { use std::slice::from_raw_parts; unsafe { from_raw_parts(s, 1) } } + fn check_lhs_nt_follows(cx: &mut ExtCtxt, lhs: &TokenTree, sp: Span) { // lhs is going to be like TokenTree::Delimited(...), where the // entire lhs is those tts. Or, it can be a "bare sequence", not wrapped in parens. match lhs { &TokenTree::Delimited(_, ref tts) => { - check_matcher(cx, tts.tts.iter(), &Eof); + check_matcher(cx, &tts.tts); }, tt @ &TokenTree::Sequence(..) => { - check_matcher(cx, Some(tt).into_iter(), &Eof); + check_matcher(cx, ref_slice(tt)); }, _ => cx.span_err(sp, "invalid macro matcher; matchers must be contained \ in balanced delimiters or a repetition indicator") @@ -345,10 +349,59 @@ fn check_rhs(cx: &mut ExtCtxt, rhs: &TokenTree) -> bool { false } -// returns the last token that was checked, for TokenTree::Sequence. this gets used later on. -fn check_matcher<'a, I>(cx: &mut ExtCtxt, matcher: I, follow: &Token) +// Issue 30450: when we are through a warning cycle, we can just error +// on all failure conditions and remove this struct and enum. + +#[derive(Debug)] +struct OnFail { + saw_failure: bool, + action: OnFailAction, +} + +#[derive(Copy, Clone, Debug)] +enum OnFailAction { Warn, Error, DoNothing } + +impl OnFail { + fn warn() -> OnFail { OnFail { saw_failure: false, action: OnFailAction::Warn } } + fn error() -> OnFail { OnFail { saw_failure: false, action: OnFailAction::Error } } + fn do_nothing() -> OnFail { OnFail { saw_failure: false, action: OnFailAction::DoNothing } } + fn react(&mut self, cx: &mut ExtCtxt, sp: Span, msg: &str) { + match self.action { + OnFailAction::DoNothing => {} + OnFailAction::Error => cx.span_err(sp, msg), + OnFailAction::Warn => { + cx.struct_span_warn(sp, msg) + .span_note(sp, "The above warning will be a hard error in the next release.") + .emit(); + } + }; + self.saw_failure = true; + } +} + +fn check_matcher(cx: &mut ExtCtxt, matcher: &[TokenTree]) { + // Issue 30450: when we are through a warning cycle, we can just + // error on all failure conditions (and remove check_matcher_old). + + // First run the old-pass, but *only* to find out if it would have failed. + let mut on_fail = OnFail::do_nothing(); + check_matcher_old(cx, matcher.iter(), &Eof, &mut on_fail); + // Then run the new pass, but merely warn if the old pass accepts and new pass rejects. + // (Note this silently accepts code if new pass accepts.) + let mut on_fail = if on_fail.saw_failure { + OnFail::error() + } else { + OnFail::warn() + }; + check_matcher_new(cx, matcher, &mut on_fail); +} + +// returns the last token that was checked, for TokenTree::Sequence. +// return value is used by recursive calls. +fn check_matcher_old<'a, I>(cx: &mut ExtCtxt, matcher: I, follow: &Token, on_fail: &mut OnFail) -> Option<(Span, Token)> where I: Iterator { use print::pprust::token_to_string; + use std::iter::once; let mut last = None; @@ -375,7 +428,7 @@ fn check_matcher<'a, I>(cx: &mut ExtCtxt, matcher: I, follow: &Token) // look at the token that follows the // sequence, which may itself be a sequence, // and so on). - cx.span_err(sp, + on_fail.react(cx, sp, &format!("`${0}:{1}` is followed by a \ sequence repetition, which is not \ allowed for `{1}` fragments", @@ -398,13 +451,13 @@ fn check_matcher<'a, I>(cx: &mut ExtCtxt, matcher: I, follow: &Token) // If T' is in the set FOLLOW(NT), continue. Else, reject. match (&next_token, is_in_follow(cx, &next_token, &frag_spec.name.as_str())) { (_, Err(msg)) => { - cx.span_err(sp, &msg); + on_fail.react(cx, sp, &msg); continue } (&Eof, _) => return Some((sp, tok.clone())), (_, Ok(true)) => continue, (next, Ok(false)) => { - cx.span_err(sp, &format!("`${0}:{1}` is followed by `{2}`, which \ + on_fail.react(cx, sp, &format!("`${0}:{1}` is followed by `{2}`, which \ is not allowed for `{1}` fragments", name, frag_spec, token_to_string(next))); @@ -420,7 +473,7 @@ fn check_matcher<'a, I>(cx: &mut ExtCtxt, matcher: I, follow: &Token) // run the algorithm on the contents with F set to U. If it // accepts, continue, else, reject. Some(ref u) => { - let last = check_matcher(cx, seq.tts.iter(), u); + let last = check_matcher_old(cx, seq.tts.iter(), u, on_fail); match last { // Since the delimiter isn't required after the last // repetition, make sure that the *next* token is @@ -434,14 +487,14 @@ fn check_matcher<'a, I>(cx: &mut ExtCtxt, matcher: I, follow: &Token) Some(&&TokenTree::Delimited(_, ref delim)) => delim.close_token(), Some(_) => { - cx.span_err(sp, "sequence repetition followed by \ + on_fail.react(cx, sp, "sequence repetition followed by \ another sequence repetition, which is not allowed"); Eof }, None => Eof }; - check_matcher(cx, once(&TokenTree::Token(span, tok.clone())), - &fol) + check_matcher_old(cx, once(&TokenTree::Token(span, tok.clone())), + &fol, on_fail) }, None => last, } @@ -454,13 +507,13 @@ fn check_matcher<'a, I>(cx: &mut ExtCtxt, matcher: I, follow: &Token) Some(&&TokenTree::Token(_, ref tok)) => tok.clone(), Some(&&TokenTree::Delimited(_, ref delim)) => delim.close_token(), Some(_) => { - cx.span_err(sp, "sequence repetition followed by another \ + on_fail.react(cx, sp, "sequence repetition followed by another \ sequence repetition, which is not allowed"); Eof }, None => Eof }; - check_matcher(cx, seq.tts.iter(), &fol) + check_matcher_old(cx, seq.tts.iter(), &fol, on_fail) } } }, @@ -471,13 +524,425 @@ fn check_matcher<'a, I>(cx: &mut ExtCtxt, matcher: I, follow: &Token) TokenTree::Delimited(_, ref tts) => { // if we don't pass in that close delimiter, we'll incorrectly consider the matcher // `{ $foo:ty }` as having a follow that isn't `RBrace` - check_matcher(cx, tts.tts.iter(), &tts.close_token()) + check_matcher_old(cx, tts.tts.iter(), &tts.close_token(), on_fail) } } } last } +fn check_matcher_new(cx: &mut ExtCtxt, matcher: &[TokenTree], on_fail: &mut OnFail) { + let first_sets = FirstSets::new(matcher); + let empty_suffix = TokenSet::empty(); + check_matcher_core(cx, &first_sets, matcher, &empty_suffix, on_fail); +} + +// The FirstSets for a matcher is a mapping from subsequences in the +// matcher to the FIRST set for that subsequence. +// +// This mapping is partially precomputed via a backwards scan over the +// token trees of the matcher, which provides a mapping from each +// repetition sequence to its FIRST set. +// +// (Hypothetically sequences should be uniquely identifiable via their +// spans, though perhaps that is false e.g. for macro-generated macros +// that do not try to inject artificial span information. My plan is +// to try to catch such cases ahead of time and not include them in +// the precomputed mapping.) +struct FirstSets { + // this maps each TokenTree::Sequence `$(tt ...) SEP OP` that is uniquely identified by its + // span in the original matcher to the First set for the inner sequence `tt ...`. + // + // If two sequences have the same span in a matcher, then map that + // span to None (invalidating the mapping here and forcing the code to + // use a slow path). + first: HashMap>, +} + +impl FirstSets { + fn new(tts: &[TokenTree]) -> FirstSets { + let mut sets = FirstSets { first: HashMap::new() }; + build_recur(&mut sets, tts); + return sets; + + // walks backward over `tts`, returning the FIRST for `tts` + // and updating `sets` at the same time for all sequence + // substructure we find within `tts`. + fn build_recur(sets: &mut FirstSets, tts: &[TokenTree]) -> TokenSet { + let mut first = TokenSet::empty(); + for tt in tts.iter().rev() { + match *tt { + TokenTree::Token(sp, ref tok) => { + first.replace_with((sp, tok.clone())); + } + TokenTree::Delimited(_, ref delimited) => { + build_recur(sets, &delimited.tts[..]); + first.replace_with((delimited.open_span, + Token::OpenDelim(delimited.delim))); + } + TokenTree::Sequence(sp, ref seq_rep) => { + let subfirst = build_recur(sets, &seq_rep.tts[..]); + + match sets.first.entry(sp) { + Entry::Vacant(vac) => { + vac.insert(Some(subfirst.clone())); + } + Entry::Occupied(mut occ) => { + // if there is already an entry, then a span must have collided. + // This should not happen with typical macro_rules macros, + // but syntax extensions need not maintain distinct spans, + // so distinct syntax trees can be assigned the same span. + // In such a case, the map cannot be trusted; so mark this + // entry as unusable. + occ.insert(None); + } + } + + // If the sequence contents can be empty, then the first + // token could be the separator token itself. + + if let (Some(ref sep), true) = (seq_rep.separator.clone(), + subfirst.maybe_empty) { + first.add_one_maybe((sp, sep.clone())); + } + + // Reverse scan: Sequence comes before `first`. + if subfirst.maybe_empty || seq_rep.op == ast::KleeneOp::ZeroOrMore { + // If sequence is potentially empty, then + // union them (preserving first emptiness). + first.add_all(&TokenSet { maybe_empty: true, ..subfirst }); + } else { + // Otherwise, sequence guaranteed + // non-empty; replace first. + first = subfirst; + } + } + } + } + + return first; + } + } + + // walks forward over `tts` until all potential FIRST tokens are + // identified. + fn first(&self, tts: &[TokenTree]) -> TokenSet { + let mut first = TokenSet::empty(); + for tt in tts.iter() { + assert!(first.maybe_empty); + match *tt { + TokenTree::Token(sp, ref tok) => { + first.add_one((sp, tok.clone())); + return first; + } + TokenTree::Delimited(_, ref delimited) => { + first.add_one((delimited.open_span, + Token::OpenDelim(delimited.delim))); + return first; + } + TokenTree::Sequence(sp, ref seq_rep) => { + match self.first.get(&sp) { + Some(&Some(ref subfirst)) => { + + // If the sequence contents can be empty, then the first + // token could be the separator token itself. + + if let (Some(ref sep), true) = (seq_rep.separator.clone(), + subfirst.maybe_empty) { + first.add_one_maybe((sp, sep.clone())); + } + + assert!(first.maybe_empty); + first.add_all(subfirst); + if subfirst.maybe_empty || seq_rep.op == ast::KleeneOp::ZeroOrMore { + // continue scanning for more first + // tokens, but also make sure we + // restore empty-tracking state + first.maybe_empty = true; + continue; + } else { + return first; + } + } + + Some(&None) => { + panic!("assume all sequences have (unique) spans for now"); + } + + None => { + panic!("We missed a sequence during FirstSets construction"); + } + } + } + } + } + + // we only exit the loop if `tts` was empty or if every + // element of `tts` matches the empty sequence. + assert!(first.maybe_empty); + return first; + } +} + +// A set of Tokens, which may include MatchNt tokens (for +// macro-by-example syntactic variables). It also carries the +// `maybe_empty` flag; that is true if and only if the matcher can +// match an empty token sequence. +// +// The First set is computed on submatchers like `$($a:expr b),* $(c)* d`, +// which has corresponding FIRST = {$a:expr, c, d}. +// Likewise, `$($a:expr b),* $(c)+ d` has FIRST = {$a:expr, c}. +// +// (Notably, we must allow for *-op to occur zero times.) +#[derive(Clone, Debug)] +struct TokenSet { + tokens: Vec<(Span, Token)>, + maybe_empty: bool, +} + +impl TokenSet { + // Returns a set for the empty sequence. + fn empty() -> Self { TokenSet { tokens: Vec::new(), maybe_empty: true } } + + // Returns the set `{ tok }` for the single-token (and thus + // non-empty) sequence [tok]. + fn singleton(tok: (Span, Token)) -> Self { + TokenSet { tokens: vec![tok], maybe_empty: false } + } + + // Changes self to be the set `{ tok }`. + // Since `tok` is always present, marks self as non-empty. + fn replace_with(&mut self, tok: (Span, Token)) { + self.tokens.clear(); + self.tokens.push(tok); + self.maybe_empty = false; + } + + // Changes self to be the empty set `{}`; meant for use when + // the particular token does not matter, but we want to + // record that it occurs. + fn replace_with_irrelevant(&mut self) { + self.tokens.clear(); + self.maybe_empty = false; + } + + // Adds `tok` to the set for `self`, marking sequence as non-empy. + fn add_one(&mut self, tok: (Span, Token)) { + if !self.tokens.contains(&tok) { + self.tokens.push(tok); + } + self.maybe_empty = false; + } + + // Adds `tok` to the set for `self`. (Leaves `maybe_empty` flag alone.) + fn add_one_maybe(&mut self, tok: (Span, Token)) { + if !self.tokens.contains(&tok) { + self.tokens.push(tok); + } + } + + // Adds all elements of `other` to this. + // + // (Since this is a set, we filter out duplicates.) + // + // If `other` is potentially empty, then preserves the previous + // setting of the empty flag of `self`. If `other` is guaranteed + // non-empty, then `self` is marked non-empty. + fn add_all(&mut self, other: &Self) { + for tok in &other.tokens { + if !self.tokens.contains(tok) { + self.tokens.push(tok.clone()); + } + } + if !other.maybe_empty { + self.maybe_empty = false; + } + } +} + +// Checks that `matcher` is internally consistent and that it +// can legally by followed by a token N, for all N in `follow`. +// (If `follow` is empty, then it imposes no constraint on +// the `matcher`.) +// +// Returns the set of NT tokens that could possibly come last in +// `matcher`. (If `matcher` matches the empty sequence, then +// `maybe_empty` will be set to true.) +// +// Requires that `first_sets` is pre-computed for `matcher`; +// see `FirstSets::new`. +fn check_matcher_core(cx: &mut ExtCtxt, + first_sets: &FirstSets, + matcher: &[TokenTree], + follow: &TokenSet, + on_fail: &mut OnFail) -> TokenSet { + use print::pprust::token_to_string; + + let mut last = TokenSet::empty(); + + // 2. For each token and suffix [T, SUFFIX] in M: + // ensure that T can be followed by SUFFIX, and if SUFFIX may be empty, + // then ensure T can also be followed by any element of FOLLOW. + 'each_token: for i in 0..matcher.len() { + let token = &matcher[i]; + let suffix = &matcher[i+1..]; + + let build_suffix_first = || { + let mut s = first_sets.first(suffix); + if s.maybe_empty { s.add_all(follow); } + return s; + }; + + // (we build `suffix_first` on demand below; you can tell + // which cases are supposed to fall through by looking for the + // initialization of this variable.) + let suffix_first; + + // First, update `last` so that it corresponds to the set + // of NT tokens that might end the sequence `... token`. + match *token { + TokenTree::Token(sp, ref tok) => { + let can_be_followed_by_any; + if let Err(bad_frag) = has_legal_fragment_specifier(tok) { + on_fail.react(cx, sp, &format!("invalid fragment specifier `{}`", bad_frag)); + // (This eliminates false positives and duplicates + // from error messages.) + can_be_followed_by_any = true; + } else { + can_be_followed_by_any = token_can_be_followed_by_any(tok); + } + + if can_be_followed_by_any { + // don't need to track tokens that work with any, + last.replace_with_irrelevant(); + // ... and don't need to check tokens that can be + // followed by anything against SUFFIX. + continue 'each_token; + } else { + last.replace_with((sp, tok.clone())); + suffix_first = build_suffix_first(); + } + } + TokenTree::Delimited(_, ref d) => { + let my_suffix = TokenSet::singleton((d.close_span, Token::CloseDelim(d.delim))); + check_matcher_core(cx, first_sets, &d.tts, &my_suffix, on_fail); + // don't track non NT tokens + last.replace_with_irrelevant(); + + // also, we don't need to check delimited sequences + // against SUFFIX + continue 'each_token; + } + TokenTree::Sequence(sp, ref seq_rep) => { + suffix_first = build_suffix_first(); + // The trick here: when we check the interior, we want + // to include the separator (if any) as a potential + // (but not guaranteed) element of FOLLOW. So in that + // case, we make a temp copy of suffix and stuff + // delimiter in there. + // + // FIXME: Should I first scan suffix_first to see if + // delimiter is already in it before I go through the + // work of cloning it? But then again, this way I may + // get a "tighter" span? + let mut new; + let my_suffix = if let Some(ref u) = seq_rep.separator { + new = suffix_first.clone(); + new.add_one_maybe((sp, u.clone())); + &new + } else { + &suffix_first + }; + + // At this point, `suffix_first` is built, and + // `my_suffix` is some TokenSet that we can use + // for checking the interior of `seq_rep`. + let next = check_matcher_core(cx, first_sets, &seq_rep.tts, my_suffix, on_fail); + if next.maybe_empty { + last.add_all(&next); + } else { + last = next; + } + + // the recursive call to check_matcher_core already ran the 'each_last + // check below, so we can just keep going forward here. + continue 'each_token; + } + } + + // (`suffix_first` guaranteed initialized once reaching here.) + + // Now `last` holds the complete set of NT tokens that could + // end the sequence before SUFFIX. Check that every one works with `suffix`. + 'each_last: for &(_sp, ref t) in &last.tokens { + if let MatchNt(ref name, ref frag_spec, _, _) = *t { + for &(sp, ref next_token) in &suffix_first.tokens { + match is_in_follow(cx, next_token, &frag_spec.name.as_str()) { + Err(msg) => { + on_fail.react(cx, sp, &msg); + // don't bother reporting every source of + // conflict for a particular element of `last`. + continue 'each_last; + } + Ok(true) => {} + Ok(false) => { + let may_be = if last.tokens.len() == 1 && + suffix_first.tokens.len() == 1 + { + "is" + } else { + "may be" + }; + + on_fail.react( + cx, sp, + &format!("`${name}:{frag}` {may_be} followed by `{next}`, which \ + is not allowed for `{frag}` fragments", + name=name, + frag=frag_spec, + next=token_to_string(next_token), + may_be=may_be)); + } + } + } + } + } + } + last +} + + +fn token_can_be_followed_by_any(tok: &Token) -> bool { + if let &MatchNt(_, ref frag_spec, _, _) = tok { + frag_can_be_followed_by_any(&frag_spec.name.as_str()) + } else { + // (Non NT's can always be followed by anthing in matchers.) + true + } +} + +/// True if a fragment of type `frag` can be followed by any sort of +/// token. We use this (among other things) as a useful approximation +/// for when `frag` can be followed by a repetition like `$(...)*` or +/// `$(...)+`. In general, these can be a bit tricky to reason about, +/// so we adopt a conservative position that says that any fragment +/// specifier which consumes at most one token tree can be followed by +/// a fragment specifier (indeed, these fragments can be followed by +/// ANYTHING without fear of future compatibility hazards). +fn frag_can_be_followed_by_any(frag: &str) -> bool { + match frag { + "item" | // always terminated by `}` or `;` + "block" | // exactly one token tree + "ident" | // exactly one token tree + "meta" | // exactly one token tree + "tt" => // exactly one token tree + true, + + _ => + false, + } +} + /// True if a fragment of type `frag` can be followed by any sort of /// token. We use this (among other things) as a useful approximation /// for when `frag` can be followed by a repetition like `$(...)*` or @@ -501,7 +966,7 @@ fn can_be_followed_by_any(frag: &str) -> bool { } /// True if `frag` can legally be followed by the token `tok`. For -/// fragments that can consume an unbounded numbe of tokens, `tok` +/// fragments that can consume an unbounded number of tokens, `tok` /// must be within a well-defined follow set. This is intended to /// guarantee future compatibility: for example, without this rule, if /// we expanded `expr` to include a new binary operator, we might @@ -557,3 +1022,22 @@ fn is_in_follow(_: &ExtCtxt, tok: &Token, frag: &str) -> Result { } } } + +fn has_legal_fragment_specifier(tok: &Token) -> Result<(), String> { + debug!("has_legal_fragment_specifier({:?})", tok); + if let &MatchNt(_, ref frag_spec, _, _) = tok { + let s = &frag_spec.name.as_str(); + if !is_legal_fragment_specifier(s) { + return Err(s.to_string()); + } + } + Ok(()) +} + +fn is_legal_fragment_specifier(frag: &str) -> bool { + match frag { + "item" | "block" | "stmt" | "expr" | "pat" | + "path" | "ty" | "ident" | "meta" | "tt" => true, + _ => false, + } +} From 3703ef582087b187f3989548c3a059f7d45503fa Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Wed, 18 Nov 2015 00:17:48 +0100 Subject: [PATCH 2/5] extending FOLLOW(NT) as specified in amendment. See RFC amendment 1384: https://github.com/rust-lang/rfcs/pull/1384 --- src/libsyntax/ext/tt/macro_rules.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/libsyntax/ext/tt/macro_rules.rs b/src/libsyntax/ext/tt/macro_rules.rs index add10612cd8cf..9f069cb17ed90 100644 --- a/src/libsyntax/ext/tt/macro_rules.rs +++ b/src/libsyntax/ext/tt/macro_rules.rs @@ -997,15 +997,18 @@ fn is_in_follow(_: &ExtCtxt, tok: &Token, frag: &str) -> Result { }, "pat" => { match *tok { - FatArrow | Comma | Eq => Ok(true), - Ident(i, _) if i.name.as_str() == "if" || i.name.as_str() == "in" => Ok(true), + FatArrow | Comma | Eq | BinOp(token::Or) => Ok(true), + Ident(i, _) if (i.name.as_str() == "if" || + i.name.as_str() == "in") => Ok(true), _ => Ok(false) } }, "path" | "ty" => { match *tok { - Comma | FatArrow | Colon | Eq | Gt | Semi => Ok(true), - Ident(i, _) if i.name.as_str() == "as" => Ok(true), + OpenDelim(token::DelimToken::Brace) | + Comma | FatArrow | Colon | Eq | Gt | Semi | BinOp(token::Or) => Ok(true), + Ident(i, _) if (i.name.as_str() == "as" || + i.name.as_str() == "where") => Ok(true), _ => Ok(false) } }, From 3e4b7012d0e0ec60b9575ef4735b110cbf6d5ce1 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Tue, 24 Nov 2015 19:46:21 +0100 Subject: [PATCH 3/5] Updated future-proofing test, removed outdated test, and added run-pass test for some new functionality. --- .../macro-input-future-proofing.rs | 2 ++ .../compile-fail/macro-seq-followed-by-seq.rs | 18 ------------- .../run-pass/macro-seq-followed-by-seq.rs | 26 +++++++++++++++++++ 3 files changed, 28 insertions(+), 18 deletions(-) delete mode 100644 src/test/compile-fail/macro-seq-followed-by-seq.rs create mode 100644 src/test/run-pass/macro-seq-followed-by-seq.rs diff --git a/src/test/compile-fail/macro-input-future-proofing.rs b/src/test/compile-fail/macro-input-future-proofing.rs index 15f6d88fd8998..522f1499e4d7b 100644 --- a/src/test/compile-fail/macro-input-future-proofing.rs +++ b/src/test/compile-fail/macro-input-future-proofing.rs @@ -25,6 +25,8 @@ macro_rules! errors_everywhere { ($($ty:ty)* -) => (); //~ ERROR `$ty:ty` is followed by `-` ($($a:ty, $b:ty)* -) => (); //~ ERROR `$b:ty` is followed by `-` ($($ty:ty)-+) => (); //~ ERROR `$ty:ty` is followed by `-`, which is not allowed for `ty` + ( $($a:expr)* $($b:tt)* ) => { }; + //~^ ERROR `$a:expr` is followed by `$b:tt`, which is not allowed for `expr` fragments } fn main() { } diff --git a/src/test/compile-fail/macro-seq-followed-by-seq.rs b/src/test/compile-fail/macro-seq-followed-by-seq.rs deleted file mode 100644 index b4f71343d546a..0000000000000 --- a/src/test/compile-fail/macro-seq-followed-by-seq.rs +++ /dev/null @@ -1,18 +0,0 @@ -// Copyright 2015 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 or the MIT license -// , at your -// option. This file may not be copied, modified, or distributed -// except according to those terms. - -// Check that we cannot have two sequence repetitions in a row. - -macro_rules! foo { - ( $($a:expr)* $($b:tt)* ) => { }; //~ ERROR sequence repetition followed by another sequence - ( $($a:tt)* $($b:tt)* ) => { }; //~ ERROR sequence repetition followed by another sequence -} - -fn main() { } diff --git a/src/test/run-pass/macro-seq-followed-by-seq.rs b/src/test/run-pass/macro-seq-followed-by-seq.rs new file mode 100644 index 0000000000000..23c7d2516a27e --- /dev/null +++ b/src/test/run-pass/macro-seq-followed-by-seq.rs @@ -0,0 +1,26 @@ +// 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Test of allowing two sequences repetitions in a row, +// functionality added as byproduct of RFC amendment #1384 +// https://github.com/rust-lang/rfcs/pull/1384 + +// Old version of Rust would reject this macro definition, even though +// there are no local ambiguities (the initial `banana` and `orange` +// tokens are enough for the expander to distinguish which case is +// intended). +macro_rules! foo { + ( $(banana $a:ident)* $(orange $b:tt)* ) => { }; +} + +fn main() { + foo!( banana id1 banana id2 + orange hi orange (hello world) ); +} From c032e0c7a700144e035731ee153c13dde8db8575 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Thu, 7 Jan 2016 00:22:47 +0100 Subject: [PATCH 4/5] After RFC amendment 1384, FOLLOW(pat) includes `|`, so update tests accordingly. --- src/test/compile-fail/macro-input-future-proofing.rs | 1 - src/test/run-pass/macro-pat-follow.rs | 10 ++++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/test/compile-fail/macro-input-future-proofing.rs b/src/test/compile-fail/macro-input-future-proofing.rs index 522f1499e4d7b..fe758a4a6310f 100644 --- a/src/test/compile-fail/macro-input-future-proofing.rs +++ b/src/test/compile-fail/macro-input-future-proofing.rs @@ -18,7 +18,6 @@ macro_rules! errors_everywhere { ($bl:block < ) => (); ($pa:pat >) => (); //~ ERROR `$pa:pat` is followed by `>`, which is not allowed for `pat` ($pa:pat , ) => (); - ($pa:pat | ) => (); //~ ERROR `$pa:pat` is followed by `|` ($pa:pat $pb:pat $ty:ty ,) => (); //~^ ERROR `$pa:pat` is followed by `$pb:pat`, which is not allowed //~^^ ERROR `$pb:pat` is followed by `$ty:ty`, which is not allowed diff --git a/src/test/run-pass/macro-pat-follow.rs b/src/test/run-pass/macro-pat-follow.rs index 77c6ed4447f11..c1abebd5f9040 100644 --- a/src/test/run-pass/macro-pat-follow.rs +++ b/src/test/run-pass/macro-pat-follow.rs @@ -24,7 +24,17 @@ macro_rules! pat_if { }} } +macro_rules! pat_bar { + ($p:pat | $p2:pat) => {{ + match Some(1u8) { + $p | $p2 => {}, + _ => {} + } + }} +} + fn main() { pat_in!(Some(_) in 0..10); pat_if!(Some(x) if x > 0); + pat_bar!(Some(1u8) | None); } From a2960bc7c6108f92a6c8c27418b20af1337208cf Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 8 Jan 2016 00:36:09 +0100 Subject: [PATCH 5/5] update test to reflect other sources of brokenness in it under new macro future proofing rules. (We may want to think about what this test was actually testing and figure out a way to test it without running afoul of macro future proofing. I spent some time trying to do this, e.g. by inserting parenthesis in the macro input pattern, but I could not quickly get it working, so I took this tack instead.) --- src/test/compile-fail/issue-30715.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/test/compile-fail/issue-30715.rs b/src/test/compile-fail/issue-30715.rs index 16761905cb983..7ad43954010b9 100644 --- a/src/test/compile-fail/issue-30715.rs +++ b/src/test/compile-fail/issue-30715.rs @@ -10,7 +10,12 @@ macro_rules! parallel { ( - for $id:ident in $iter:expr { + // If future has `pred`/`moelarry` fragments (where "pred" is + // "like expr, but with `{` in its FOLLOW set"), then could + // use `pred` instead of future-proof erroring here. See also: + // + // https://github.com/rust-lang/rfcs/pull/1384#issuecomment-160165525 + for $id:ident in $iter:expr { //~ WARN `$iter:expr` is followed by `{` $( $inner:expr; )* } ) => {};