Auto merge of #25444 - nikomatsakis:macro-tt-fix, r=pnkfelix

Permit token trees, identifiers, and blocks to be following by sequences.

Fixes #25436.

r? @pnkfelix

Author: bors

src/libsyntax/ext/tt/macro_rules.rs

@@ -325,42 +325,55 @@ fn check_matcher<'a, I>(cx: &mut ExtCtxt, matcher: I, follow: &Token)
         last = match *token {
             TtToken(sp, MatchNt(ref name, ref frag_spec, _, _)) => {
                 // ii. If T is a simple NT, look ahead to the next token T' in
-                // M.
-                let next_token = match tokens.peek() {
-                    // If T' closes a complex NT, replace T' with F
-                    Some(&&TtToken(_, CloseDelim(_))) => follow.clone(),
-                    Some(&&TtToken(_, ref tok)) => tok.clone(),
-                    Some(&&TtSequence(sp, _)) => {
-                        cx.span_err(sp,
-                                    &format!("`${0}:{1}` is followed by a \
-                                              sequence repetition, which is not \
-                                              allowed for `{1}` fragments",
-                                             name.as_str(), frag_spec.as_str())
+                // M. If T' is in the set FOLLOW(NT), continue. Else; reject.
+                if can_be_followed_by_any(frag_spec.as_str()) {
+                    continue
+                } else {
+                    let next_token = match tokens.peek() {
+                        // If T' closes a complex NT, replace T' with F
+                        Some(&&TtToken(_, CloseDelim(_))) => follow.clone(),
+                        Some(&&TtToken(_, ref tok)) => tok.clone(),
+                        Some(&&TtSequence(sp, _)) => {
+                            // Be conservative around sequences: to be
+                            // more specific, we would need to
+                            // consider FIRST sets, but also the
+                            // possibility that the sequence occurred
+                            // zero times (in which case we need to
+                            // look at the token that follows the
+                            // sequence, which may itself a sequence,
+                            // and so on).
+                            cx.span_err(sp,
+                                        &format!("`${0}:{1}` is followed by a \
+                                                  sequence repetition, which is not \
+                                                  allowed for `{1}` fragments",
+                                                 name.as_str(), frag_spec.as_str())
                                         );
-                        Eof
-                    },
-                    // die next iteration
-                    Some(&&TtDelimited(_, ref delim)) => delim.close_token(),
-                    // else, we're at the end of the macro or sequence
-                    None => follow.clone()
-                };
-
-                let tok = if let TtToken(_, ref tok) = *token { tok } else { unreachable!() };
-                // If T' is in the set FOLLOW(NT), continue. Else, reject.
-                match (&next_token, is_in_follow(cx, &next_token, frag_spec.as_str())) {
-                    (_, Err(msg)) => {
-                        cx.span_err(sp, &msg);
-                        continue
+                            Eof
+                        },
+                        // die next iteration
+                        Some(&&TtDelimited(_, ref delim)) => delim.close_token(),
+                        // else, we're at the end of the macro or sequence
+                        None => follow.clone()
+                    };
+
+                    let tok = if let TtToken(_, ref tok) = *token { tok } else { unreachable!() };
+
+                    // If T' is in the set FOLLOW(NT), continue. Else, reject.
+                    match (&next_token, is_in_follow(cx, &next_token, frag_spec.as_str())) {
+                        (_, Err(msg)) => {
+                            cx.span_err(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 \
+                                                      is not allowed for `{1}` fragments",
+                                                     name.as_str(), frag_spec.as_str(),
+                                                     token_to_string(next)));
+                            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 \
-                                                  is not allowed for `{1}` fragments",
-                                                 name.as_str(), frag_spec.as_str(),
-                                                 token_to_string(next)));
-                        continue
-                    },
                 }
             },
             TtSequence(sp, ref seq) => {
@@ -427,8 +440,39 @@ fn check_matcher<'a, I>(cx: &mut ExtCtxt, matcher: I, follow: &Token)
     last
 }
 
+/// 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 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 `frag` can legally be followed by the token `tok`. For
+/// fragments that can consume an unbounded numbe 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
+/// break macros that were relying on that binary operator as a
+/// separator.
 fn is_in_follow(_: &ExtCtxt, tok: &Token, frag: &str) -> Result<bool, String> {
     if let &CloseDelim(_) = tok {
+        // closing a token tree can never be matched by any fragment;
+        // iow, we always require that `(` and `)` match, etc.
         Ok(true)
     } else {
         match frag {

src/test/compile-fail/macro-followed-by-seq-bad.rs

@@ -0,0 +1,19 @@
+// 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 <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.
+
+// Regression test for issue #25436: check that things which can be
+// followed by any token also permit X* to come afterwards.
+
+macro_rules! foo {
+  ( $a:expr $($b:tt)* ) => { }; //~ ERROR not allowed for `expr` fragments
+  ( $a:ty $($b:tt)* ) => { };   //~ ERROR not allowed for `ty` fragments
+}
+
+fn main() { }

src/test/compile-fail/macro-seq-followed-by-seq.rs

@@ -0,0 +1,18 @@
+// 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 <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.
+
+// 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() { }

src/test/run-pass/macro-followed-by-seq.rs

@@ -0,0 +1,22 @@
+// 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 <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.
+
+// Regression test for issue #25436: check that things which can be
+// followed by any token also permit X* to come afterwards.
+
+macro_rules! foo {
+  ( $a:tt $($b:tt)* ) => { };
+  ( $a:ident $($b:tt)* ) => { };
+  ( $a:item $($b:tt)* ) => { };
+  ( $a:block $($b:tt)* ) => { };
+  ( $a:meta $($b:tt)* ) => { }
+}
+
+fn main() { }

src/test/run-pass/macro-tt-followed-by-seq.rs

@@ -0,0 +1,36 @@
+// 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 <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.
+
+// Regression test for issue #25436: permit token-trees to be followed
+// by sequences, enabling more general parsing.
+
+use self::Join::*;
+
+#[derive(Debug)]
+enum Join<A,B> {
+  Keep(A,B),
+  Skip(A,B),
+}
+
+macro_rules! parse_list {
+  ( < $a:expr; > $($b:tt)* ) => { Keep(parse_item!($a),parse_list!($($b)*)) };
+  ( $a:tt $($b:tt)* ) => { Skip(parse_item!($a), parse_list!($($b)*)) };
+  ( ) => { () };
+}
+
+macro_rules! parse_item {
+  ( $x:expr ) => { $x }
+}
+
+fn main() {
+    let list = parse_list!(<1;> 2 <3;> 4);
+    assert_eq!("Keep(1, Skip(2, Keep(3, Skip(4, ()))))",
+               format!("{:?}", list));
+}