Skip to content

Commit

Permalink
Revert "Auto merge of #49719 - mark-i-m:no_sep, r=petrochenkov"
Browse files Browse the repository at this point in the history
This reverts commit d6ba1b9, reversing
changes made to 8de5353.
  • Loading branch information
pietroalbini committed Jun 8, 2018
1 parent eb01d0f commit efce22c
Show file tree
Hide file tree
Showing 4 changed files with 156 additions and 72 deletions.
89 changes: 67 additions & 22 deletions src/libsyntax/ext/tt/quoted.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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),

Expand All @@ -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 {
Expand Down
29 changes: 23 additions & 6 deletions src/test/run-pass/macro-at-most-once-rep.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
} }
Expand All @@ -50,7 +62,7 @@ macro_rules! barstar {

$(
x += $a;
)?
)*

assert_eq!(x, $num);
} }
Expand All @@ -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);
}
34 changes: 19 additions & 15 deletions src/test/ui/macros/macro-at-most-once-rep-ambig.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?*) => {}
}
Expand All @@ -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
}
76 changes: 47 additions & 29 deletions src/test/ui/macros/macro-at-most-once-rep-ambig.stderr
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit efce22c

Please sign in to comment.