From b2ed17b5ea8d300838138844e3b2701cf000eae2 Mon Sep 17 00:00:00 2001 From: Mark Mansi Date: Thu, 5 Apr 2018 19:50:08 -0500 Subject: [PATCH 1/2] No separator for `?`. No `?` as a separator. --- src/libsyntax/ext/tt/quoted.rs | 89 +++++-------------- src/test/run-pass/macro-at-most-once-rep.rs | 29 ++---- .../ui/macros/macro-at-most-once-rep-ambig.rs | 34 ++++--- .../macro-at-most-once-rep-ambig.stderr | 72 ++++++--------- 4 files changed, 70 insertions(+), 154 deletions(-) diff --git a/src/libsyntax/ext/tt/quoted.rs b/src/libsyntax/ext/tt/quoted.rs index f324edeb1178a..30b993dda3de5 100644 --- a/src/libsyntax/ext/tt/quoted.rs +++ b/src/libsyntax/ext/tt/quoted.rs @@ -389,72 +389,26 @@ where { // We basically look at two token trees here, denoted as #1 and #2 below let span = match parse_kleene_op(input, span) { - // #1 is a `+` or `*` KleeneOp - // - // `?` is ambiguous: it could be a separator or a Kleene::ZeroOrOne, so we need to look - // ahead one more token to be sure. - Ok(Ok(op)) if op != KleeneOp::ZeroOrOne => return (None, op), - - // #1 is `?` token, but it could be a Kleene::ZeroOrOne without a separator or it could - // be a `?` separator followed by any Kleene operator. We need to look ahead 1 token to - // find out which. - Ok(Ok(op)) => { - assert_eq!(op, KleeneOp::ZeroOrOne); - - // Lookahead at #2. If it is a KleenOp, then #1 is a separator. - let is_1_sep = if let Some(&tokenstream::TokenTree::Token(_, ref tok2)) = input.peek() { - kleene_op(tok2).is_some() - } else { - false - }; - - if is_1_sep { - // #1 is a separator and #2 should be a KleepeOp::* - // (N.B. We need to advance the input iterator.) - match parse_kleene_op(input, span) { - // #2 is a KleeneOp (this is the only valid option) :) - Ok(Ok(op)) if op == KleeneOp::ZeroOrOne => { - if !features.macro_at_most_once_rep - && !attr::contains_name(attrs, "allow_internal_unstable") - { - let explain = feature_gate::EXPLAIN_MACRO_AT_MOST_ONCE_REP; - emit_feature_err( - sess, - "macro_at_most_once_rep", - span, - GateIssue::Language, - explain, - ); - } - return (Some(token::Question), op); - } - Ok(Ok(op)) => return (Some(token::Question), op), - - // #2 is a random token (this is an error) :( - Ok(Err((_, span))) => span, - - // #2 is not even a token at all :( - Err(span) => span, - } - } else { - if !features.macro_at_most_once_rep - && !attr::contains_name(attrs, "allow_internal_unstable") - { - let explain = feature_gate::EXPLAIN_MACRO_AT_MOST_ONCE_REP; - emit_feature_err( - sess, - "macro_at_most_once_rep", - span, - GateIssue::Language, - explain, - ); - } - - // #2 is a random tree and #1 is KleeneOp::ZeroOrOne - return (None, op); + // #1 is any KleeneOp (`?`) + Ok(Ok(op)) if op == KleeneOp::ZeroOrOne => { + if !features.macro_at_most_once_rep + && !attr::contains_name(attrs, "allow_internal_unstable") + { + let explain = feature_gate::EXPLAIN_MACRO_AT_MOST_ONCE_REP; + emit_feature_err( + sess, + "macro_at_most_once_rep", + span, + GateIssue::Language, + explain, + ); } + return (None, op); } + // #1 is any KleeneOp (`+`, `*`) + Ok(Ok(op)) => return (None, op), + // #1 is a separator followed by #2, a KleeneOp Ok(Err((tok, span))) => match parse_kleene_op(input, span) { // #2 is a KleeneOp :D @@ -470,8 +424,11 @@ where GateIssue::Language, explain, ); + } else { + sess.span_diagnostic + .span_err(span, "`?` macro repetition does not allow a separator"); } - return (Some(tok), op); + return (None, op); } Ok(Ok(op)) => return (Some(tok), op), @@ -486,9 +443,7 @@ where Err(span) => span, }; - if !features.macro_at_most_once_rep - && !attr::contains_name(attrs, "allow_internal_unstable") - { + if !features.macro_at_most_once_rep && !attr::contains_name(attrs, "allow_internal_unstable") { sess.span_diagnostic .span_err(span, "expected one of: `*`, `+`, or `?`"); } else { diff --git a/src/test/run-pass/macro-at-most-once-rep.rs b/src/test/run-pass/macro-at-most-once-rep.rs index b7e942f938321..c08effe549328 100644 --- a/src/test/run-pass/macro-at-most-once-rep.rs +++ b/src/test/run-pass/macro-at-most-once-rep.rs @@ -32,25 +32,13 @@ macro_rules! foo { } } } -macro_rules! baz { - ($($a:ident),? ; $num:expr) => { { // comma separator is meaningless for `?` - let mut x = 0; - - $( - x += $a; - )? - - assert_eq!(x, $num); - } } -} - macro_rules! barplus { ($($a:ident)?+ ; $num:expr) => { { let mut x = 0; $( x += $a; - )+ + )? assert_eq!(x, $num); } } @@ -62,7 +50,7 @@ macro_rules! barstar { $( x += $a; - )* + )? assert_eq!(x, $num); } } @@ -74,15 +62,10 @@ pub fn main() { // accept 0 or 1 repetitions foo!( ; 0); foo!(a ; 1); - baz!( ; 0); - baz!(a ; 1); // Make sure using ? as a separator works as before - barplus!(a ; 1); - barplus!(a?a ; 2); - barplus!(a?a?a ; 3); - barstar!( ; 0); - barstar!(a ; 1); - barstar!(a?a ; 2); - barstar!(a?a?a ; 3); + barplus!(+ ; 0); + barplus!(a + ; 1); + barstar!(* ; 0); + barstar!(a * ; 1); } diff --git a/src/test/ui/macros/macro-at-most-once-rep-ambig.rs b/src/test/ui/macros/macro-at-most-once-rep-ambig.rs index a5660f8b41f8d..63827ffeed045 100644 --- a/src/test/ui/macros/macro-at-most-once-rep-ambig.rs +++ b/src/test/ui/macros/macro-at-most-once-rep-ambig.rs @@ -8,30 +8,26 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// The logic for parsing Kleene operators in macros has a special case to disambiguate `?`. -// Specifically, `$(pat)?` is the ZeroOrOne operator whereas `$(pat)?+` or `$(pat)?*` are the -// ZeroOrMore and OneOrMore operators using `?` as a separator. These tests are intended to -// exercise that logic in the macro parser. -// -// Moreover, we also throw in some tests for using a separator with `?`, which is meaningless but -// included for consistency with `+` and `*`. -// -// This test focuses on error cases. +// Tests the behavior of various Kleene operators in macros with respect to `?` terminals. In +// particular, `?` in the position of a separator and of a Kleene operator is tested. #![feature(macro_at_most_once_rep)] +// should match `` and `a` macro_rules! foo { ($(a)?) => {} } macro_rules! baz { - ($(a),?) => {} // comma separator is meaningless for `?` + ($(a),?) => {} //~ ERROR `?` macro repetition does not allow a separator } +// should match `+` and `a+` macro_rules! barplus { ($(a)?+) => {} } +// should match `*` and `a*` macro_rules! barstar { ($(a)?*) => {} } @@ -40,14 +36,14 @@ pub fn main() { foo!(a?a?a); //~ ERROR no rules expected the token `?` foo!(a?a); //~ ERROR no rules expected the token `?` foo!(a?); //~ ERROR no rules expected the token `?` - baz!(a?a?a); //~ ERROR no rules expected the token `?` - baz!(a?a); //~ ERROR no rules expected the token `?` - baz!(a?); //~ ERROR no rules expected the token `?` - baz!(a,); //~ ERROR unexpected end of macro invocation - baz!(a?a?a,); //~ ERROR no rules expected the token `?` - baz!(a?a,); //~ ERROR no rules expected the token `?` - baz!(a?,); //~ ERROR no rules expected the token `?` barplus!(); //~ ERROR unexpected end of macro invocation - barplus!(a?); //~ ERROR unexpected end of macro invocation - barstar!(a?); //~ ERROR unexpected end of macro invocation + barstar!(); //~ ERROR unexpected end of macro invocation + barplus!(a?); //~ ERROR no rules expected the token `?` + barplus!(a); //~ ERROR unexpected end of macro invocation + barstar!(a?); //~ ERROR no rules expected the token `?` + barstar!(a); //~ ERROR unexpected end of macro invocation + barplus!(+); // ok + barstar!(*); // ok + barplus!(a); // ok + barstar!(a*); // ok } diff --git a/src/test/ui/macros/macro-at-most-once-rep-ambig.stderr b/src/test/ui/macros/macro-at-most-once-rep-ambig.stderr index d382082a57585..eb355c70b5593 100644 --- a/src/test/ui/macros/macro-at-most-once-rep-ambig.stderr +++ b/src/test/ui/macros/macro-at-most-once-rep-ambig.stderr @@ -1,80 +1,62 @@ -error: no rules expected the token `?` - --> $DIR/macro-at-most-once-rep-ambig.rs:40:11 +error: `?` macro repetition does not allow a separator + --> $DIR/macro-at-most-once-rep-ambig.rs:29:10 | -LL | foo!(a?a?a); //~ ERROR no rules expected the token `?` - | ^ - -error: no rules expected the token `?` - --> $DIR/macro-at-most-once-rep-ambig.rs:41:11 - | -LL | foo!(a?a); //~ ERROR no rules expected the token `?` - | ^ - -error: no rules expected the token `?` - --> $DIR/macro-at-most-once-rep-ambig.rs:42:11 - | -LL | foo!(a?); //~ ERROR no rules expected the token `?` - | ^ +LL | ($(a),?) => {} //~ ERROR `?` macro repetition does not allow a separator + | ^ error: no rules expected the token `?` --> $DIR/macro-at-most-once-rep-ambig.rs:43:11 | -LL | baz!(a?a?a); //~ ERROR no rules expected the token `?` +LL | foo!(a?a?a); //~ ERROR no rules expected the token `?` | ^ error: no rules expected the token `?` --> $DIR/macro-at-most-once-rep-ambig.rs:44:11 | -LL | baz!(a?a); //~ ERROR no rules expected the token `?` +LL | foo!(a?a); //~ ERROR no rules expected the token `?` | ^ error: no rules expected the token `?` --> $DIR/macro-at-most-once-rep-ambig.rs:45:11 | -LL | baz!(a?); //~ ERROR no rules expected the token `?` +LL | foo!(a?); //~ ERROR no rules expected the token `?` | ^ error: unexpected end of macro invocation - --> $DIR/macro-at-most-once-rep-ambig.rs:46:11 - | -LL | baz!(a,); //~ ERROR unexpected end of macro invocation - | ^ - -error: no rules expected the token `?` - --> $DIR/macro-at-most-once-rep-ambig.rs:47:11 + --> $DIR/macro-at-most-once-rep-ambig.rs:46:5 | -LL | baz!(a?a?a,); //~ ERROR no rules expected the token `?` - | ^ +LL | barplus!(); //~ ERROR unexpected end of macro invocation + | ^^^^^^^^^^^ -error: no rules expected the token `?` - --> $DIR/macro-at-most-once-rep-ambig.rs:48:11 +error: unexpected end of macro invocation + --> $DIR/macro-at-most-once-rep-ambig.rs:47:5 | -LL | baz!(a?a,); //~ ERROR no rules expected the token `?` - | ^ +LL | barstar!(); //~ ERROR unexpected end of macro invocation + | ^^^^^^^^^^^ error: no rules expected the token `?` - --> $DIR/macro-at-most-once-rep-ambig.rs:49:11 + --> $DIR/macro-at-most-once-rep-ambig.rs:48:15 | -LL | baz!(a?,); //~ ERROR no rules expected the token `?` - | ^ +LL | barplus!(a?); //~ ERROR no rules expected the token `?` + | ^ error: unexpected end of macro invocation - --> $DIR/macro-at-most-once-rep-ambig.rs:50:5 + --> $DIR/macro-at-most-once-rep-ambig.rs:49:14 | -LL | barplus!(); //~ ERROR unexpected end of macro invocation - | ^^^^^^^^^^^ +LL | barplus!(a); //~ ERROR unexpected end of macro invocation + | ^ -error: unexpected end of macro invocation - --> $DIR/macro-at-most-once-rep-ambig.rs:51:15 +error: no rules expected the token `?` + --> $DIR/macro-at-most-once-rep-ambig.rs:50:15 | -LL | barplus!(a?); //~ ERROR unexpected end of macro invocation +LL | barstar!(a?); //~ ERROR no rules expected the token `?` | ^ error: unexpected end of macro invocation - --> $DIR/macro-at-most-once-rep-ambig.rs:52:15 + --> $DIR/macro-at-most-once-rep-ambig.rs:51:14 | -LL | barstar!(a?); //~ ERROR unexpected end of macro invocation - | ^ +LL | barstar!(a); //~ ERROR unexpected end of macro invocation + | ^ -error: aborting due to 13 previous errors +error: aborting due to 10 previous errors From 54bba4c45648b02b92dcec74f4230bfa02846d5e Mon Sep 17 00:00:00 2001 From: Mark Mansi Date: Fri, 6 Apr 2018 17:26:51 -0500 Subject: [PATCH 2/2] fix test --- .../ui/macros/macro-at-most-once-rep-ambig.rs | 2 +- .../macro-at-most-once-rep-ambig.stderr | 20 +++++++++---------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/test/ui/macros/macro-at-most-once-rep-ambig.rs b/src/test/ui/macros/macro-at-most-once-rep-ambig.rs index 63827ffeed045..e25c3ccfcd980 100644 --- a/src/test/ui/macros/macro-at-most-once-rep-ambig.rs +++ b/src/test/ui/macros/macro-at-most-once-rep-ambig.rs @@ -44,6 +44,6 @@ pub fn main() { barstar!(a); //~ ERROR unexpected end of macro invocation barplus!(+); // ok barstar!(*); // ok - barplus!(a); // ok + barplus!(a+); // ok barstar!(a*); // ok } diff --git a/src/test/ui/macros/macro-at-most-once-rep-ambig.stderr b/src/test/ui/macros/macro-at-most-once-rep-ambig.stderr index eb355c70b5593..cb1e360471cc8 100644 --- a/src/test/ui/macros/macro-at-most-once-rep-ambig.stderr +++ b/src/test/ui/macros/macro-at-most-once-rep-ambig.stderr @@ -1,59 +1,59 @@ error: `?` macro repetition does not allow a separator - --> $DIR/macro-at-most-once-rep-ambig.rs:29:10 + --> $DIR/macro-at-most-once-rep-ambig.rs:22:10 | LL | ($(a),?) => {} //~ ERROR `?` macro repetition does not allow a separator | ^ error: no rules expected the token `?` - --> $DIR/macro-at-most-once-rep-ambig.rs:43:11 + --> $DIR/macro-at-most-once-rep-ambig.rs:36:11 | LL | foo!(a?a?a); //~ ERROR no rules expected the token `?` | ^ error: no rules expected the token `?` - --> $DIR/macro-at-most-once-rep-ambig.rs:44:11 + --> $DIR/macro-at-most-once-rep-ambig.rs:37:11 | LL | foo!(a?a); //~ ERROR no rules expected the token `?` | ^ error: no rules expected the token `?` - --> $DIR/macro-at-most-once-rep-ambig.rs:45:11 + --> $DIR/macro-at-most-once-rep-ambig.rs:38:11 | LL | foo!(a?); //~ ERROR no rules expected the token `?` | ^ error: unexpected end of macro invocation - --> $DIR/macro-at-most-once-rep-ambig.rs:46:5 + --> $DIR/macro-at-most-once-rep-ambig.rs:39:5 | LL | barplus!(); //~ ERROR unexpected end of macro invocation | ^^^^^^^^^^^ error: unexpected end of macro invocation - --> $DIR/macro-at-most-once-rep-ambig.rs:47:5 + --> $DIR/macro-at-most-once-rep-ambig.rs:40:5 | LL | barstar!(); //~ ERROR unexpected end of macro invocation | ^^^^^^^^^^^ error: no rules expected the token `?` - --> $DIR/macro-at-most-once-rep-ambig.rs:48:15 + --> $DIR/macro-at-most-once-rep-ambig.rs:41:15 | LL | barplus!(a?); //~ ERROR no rules expected the token `?` | ^ error: unexpected end of macro invocation - --> $DIR/macro-at-most-once-rep-ambig.rs:49:14 + --> $DIR/macro-at-most-once-rep-ambig.rs:42:14 | LL | barplus!(a); //~ ERROR unexpected end of macro invocation | ^ error: no rules expected the token `?` - --> $DIR/macro-at-most-once-rep-ambig.rs:50:15 + --> $DIR/macro-at-most-once-rep-ambig.rs:43:15 | LL | barstar!(a?); //~ ERROR no rules expected the token `?` | ^ error: unexpected end of macro invocation - --> $DIR/macro-at-most-once-rep-ambig.rs:51:14 + --> $DIR/macro-at-most-once-rep-ambig.rs:44:14 | LL | barstar!(a); //~ ERROR unexpected end of macro invocation | ^