From 1df781712297270c74d175552e420ae055a7f193 Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Thu, 7 Jun 2018 17:07:05 +0200 Subject: [PATCH] Revert "Auto merge of #49719 - mark-i-m:no_sep, r=petrochenkov" This reverts commit d6ba1b9b021c408fcad60ee52acf8af5e1b2eb00, reversing changes made to 8de5353f75dcde04abe947e0560dc5edd861cf3a. --- 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 | 76 ++++++++++------ 4 files changed, 156 insertions(+), 72 deletions(-) diff --git a/src/libsyntax/ext/tt/quoted.rs b/src/libsyntax/ext/tt/quoted.rs index 77c6afa1c64a6..01b971976a763 100644 --- a/src/libsyntax/ext/tt/quoted.rs +++ b/src/libsyntax/ext/tt/quoted.rs @@ -386,26 +386,72 @@ 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 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, - ); + // #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); } - 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 @@ -421,11 +467,8 @@ where GateIssue::Language, explain, ); - } else { - sess.span_diagnostic - .span_err(span, "`?` macro repetition does not allow a separator"); } - return (None, op); + return (Some(tok), op); } Ok(Ok(op)) => return (Some(tok), op), @@ -440,7 +483,9 @@ 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 c08effe549328..b7e942f938321 100644 --- a/src/test/run-pass/macro-at-most-once-rep.rs +++ b/src/test/run-pass/macro-at-most-once-rep.rs @@ -32,13 +32,25 @@ 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); } } @@ -50,7 +62,7 @@ macro_rules! barstar { $( x += $a; - )? + )* assert_eq!(x, $num); } } @@ -62,10 +74,15 @@ 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!(+ ; 0); - barplus!(a + ; 1); - barstar!(* ; 0); - barstar!(a * ; 1); + 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); } 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 e25c3ccfcd980..a5660f8b41f8d 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,26 +8,30 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// 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. +// 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. #![feature(macro_at_most_once_rep)] -// should match `` and `a` macro_rules! foo { ($(a)?) => {} } macro_rules! baz { - ($(a),?) => {} //~ ERROR `?` macro repetition does not allow a separator + ($(a),?) => {} // comma separator is meaningless for `?` } -// should match `+` and `a+` macro_rules! barplus { ($(a)?+) => {} } -// should match `*` and `a*` macro_rules! barstar { ($(a)?*) => {} } @@ -36,14 +40,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 - 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 + barplus!(a?); //~ ERROR unexpected end of macro invocation + barstar!(a?); //~ ERROR unexpected end of macro invocation } 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 cb1e360471cc8..d382082a57585 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,62 +1,80 @@ -error: `?` macro repetition does not allow a separator - --> $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:36:11 + --> $DIR/macro-at-most-once-rep-ambig.rs:40: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:37:11 + --> $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:38:11 + --> $DIR/macro-at-most-once-rep-ambig.rs:42:11 | LL | foo!(a?); //~ ERROR no rules expected the token `?` | ^ -error: unexpected end of macro invocation - --> $DIR/macro-at-most-once-rep-ambig.rs:39:5 +error: no rules expected the token `?` + --> $DIR/macro-at-most-once-rep-ambig.rs:43:11 | -LL | barplus!(); //~ ERROR unexpected end of macro invocation - | ^^^^^^^^^^^ +LL | baz!(a?a?a); //~ ERROR no rules expected the token `?` + | ^ -error: unexpected end of macro invocation - --> $DIR/macro-at-most-once-rep-ambig.rs:40:5 +error: no rules expected the token `?` + --> $DIR/macro-at-most-once-rep-ambig.rs:44:11 | -LL | barstar!(); //~ ERROR unexpected end of macro invocation - | ^^^^^^^^^^^ +LL | baz!(a?a); //~ ERROR no rules expected the token `?` + | ^ error: no rules expected the token `?` - --> $DIR/macro-at-most-once-rep-ambig.rs:41:15 + --> $DIR/macro-at-most-once-rep-ambig.rs:45:11 | -LL | barplus!(a?); //~ ERROR no rules expected the token `?` - | ^ +LL | baz!(a?); //~ ERROR no rules expected the token `?` + | ^ error: unexpected end of macro invocation - --> $DIR/macro-at-most-once-rep-ambig.rs:42:14 + --> $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 + | +LL | baz!(a?a?a,); //~ ERROR no rules expected the token `?` + | ^ + +error: no rules expected the token `?` + --> $DIR/macro-at-most-once-rep-ambig.rs:48:11 | -LL | barplus!(a); //~ ERROR unexpected end of macro invocation - | ^ +LL | baz!(a?a,); //~ ERROR no rules expected the token `?` + | ^ error: no rules expected the token `?` - --> $DIR/macro-at-most-once-rep-ambig.rs:43:15 + --> $DIR/macro-at-most-once-rep-ambig.rs:49:11 + | +LL | baz!(a?,); //~ ERROR no rules expected the token `?` + | ^ + +error: unexpected end of macro invocation + --> $DIR/macro-at-most-once-rep-ambig.rs:50:5 + | +LL | barplus!(); //~ ERROR unexpected end of macro invocation + | ^^^^^^^^^^^ + +error: unexpected end of macro invocation + --> $DIR/macro-at-most-once-rep-ambig.rs:51:15 | -LL | barstar!(a?); //~ ERROR no rules expected the token `?` +LL | barplus!(a?); //~ ERROR unexpected end of macro invocation | ^ error: unexpected end of macro invocation - --> $DIR/macro-at-most-once-rep-ambig.rs:44:14 + --> $DIR/macro-at-most-once-rep-ambig.rs:52:15 | -LL | barstar!(a); //~ ERROR unexpected end of macro invocation - | ^ +LL | barstar!(a?); //~ ERROR unexpected end of macro invocation + | ^ -error: aborting due to 10 previous errors +error: aborting due to 13 previous errors