Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update ? repetition disambiguation. #49719

Merged
merged 2 commits into from
Apr 16, 2018
Merged

Update ? repetition disambiguation. #49719

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b2ed17b
No separator for `?`. No `?` as a separator.
mark-i-m Apr 6, 2018
54bba4c
fix test
mark-i-m Apr 6, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 22 additions & 67 deletions src/libsyntax/ext/tt/quoted.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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),

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

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

assert_eq!(x, $num);
} }
Expand All @@ -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);
}
34 changes: 15 additions & 19 deletions src/test/ui/macros/macro-at-most-once-rep-ambig.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)?*) => {}
}
Expand All @@ -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
}
76 changes: 29 additions & 47 deletions src/test/ui/macros/macro-at-most-once-rep-ambig.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,80 +1,62 @@
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:40: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:41: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:42:11
--> $DIR/macro-at-most-once-rep-ambig.rs:38:11
|
LL | foo!(a?); //~ ERROR no rules expected the token `?`
| ^

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 `?`
| ^

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 `?`
| ^

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 `?`
| ^

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:39: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:40: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:41: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:42: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:43: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:44: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