Skip to content

Commit

Permalink
Auto merge of #60932 - Centril:macro-at-most-once-2015, r=<try>
Browse files Browse the repository at this point in the history
Support ? Kleene macro operator in 2015

Closes #56668.

See that issue for rationale and discussion.

Crater will be needed and then, if all goes well, FCP.
  • Loading branch information
bors committed May 18, 2019
2 parents 548add7 + 695b601 commit 097e262
Show file tree
Hide file tree
Showing 13 changed files with 189 additions and 254 deletions.
171 changes: 4 additions & 167 deletions src/libsyntax/ext/tt/quoted.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use crate::ast::NodeId;
use crate::early_buffered_lints::BufferedEarlyLintId;
use crate::ext::tt::macro_parser;
use crate::feature_gate::Features;
use crate::parse::{token, ParseSess};
Expand Down Expand Up @@ -291,16 +290,7 @@ where
macro_node_id,
);
// Get the Kleene operator and optional separator
let (separator, op) =
parse_sep_and_kleene_op(
trees,
span.entire(),
sess,
features,
attrs,
edition,
macro_node_id,
);
let (separator, op) = parse_sep_and_kleene_op(trees, span.entire(), sess);
// Count the number of captured "names" (i.e., named metavars)
let name_captures = macro_parser::count_names(&sequence);
TokenTree::Sequence(
Expand Down Expand Up @@ -411,164 +401,11 @@ where
/// session `sess`. If the next one (or possibly two) tokens in `input` correspond to a Kleene
/// operator and separator, then a tuple with `(separator, KleeneOp)` is returned. Otherwise, an
/// error with the appropriate span is emitted to `sess` and a dummy value is returned.
///
/// N.B., in the 2015 edition, `*` and `+` are the only Kleene operators, and `?` is a separator.
/// In the 2018 edition however, `?` is a Kleene operator, and not a separator.
fn parse_sep_and_kleene_op<I>(
input: &mut Peekable<I>,
span: Span,
sess: &ParseSess,
features: &Features,
attrs: &[ast::Attribute],
edition: Edition,
macro_node_id: NodeId,
) -> (Option<token::Token>, KleeneOp)
where
I: Iterator<Item = tokenstream::TokenTree>,
{
match edition {
Edition::Edition2015 => parse_sep_and_kleene_op_2015(
input,
span,
sess,
features,
attrs,
macro_node_id,
),
Edition::Edition2018 => parse_sep_and_kleene_op_2018(input, span, sess, features, attrs),
}
}

// `?` is a separator (with a migration warning) and never a KleeneOp.
fn parse_sep_and_kleene_op_2015<I>(
input: &mut Peekable<I>,
span: Span,
sess: &ParseSess,
_features: &Features,
_attrs: &[ast::Attribute],
macro_node_id: NodeId,
) -> (Option<token::Token>, KleeneOp)
where
I: Iterator<Item = tokenstream::TokenTree>,
{
// 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 (warning) or a Kleene::ZeroOrOne (error), 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 (error in 2015) 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, op1_span))) => {
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 `?`, which is not allowed as a Kleene op in 2015 edition,
// but is allowed in the 2018 edition.
Ok(Ok((op, op2_span))) if op == KleeneOp::ZeroOrOne => {
sess.span_diagnostic
.struct_span_err(op2_span, "expected `*` or `+`")
.note("`?` is not a macro repetition operator in the 2015 edition, \
but is accepted in the 2018 edition")
.emit();

// Return a dummy
return (None, KleeneOp::ZeroOrMore);
}

// #2 is a Kleene op, which is the only valid option
Ok(Ok((op, _))) => {
// Warn that `?` as a separator will be deprecated
sess.buffer_lint(
BufferedEarlyLintId::QuestionMarkMacroSep,
op1_span,
macro_node_id,
"using `?` as a separator is deprecated and will be \
a hard error in an upcoming edition",
);

return (Some(token::Question), op);
}

// #2 is a random token (this is an error) :(
Ok(Err((_, _))) => op1_span,

// #2 is not even a token at all :(
Err(_) => op1_span,
}
} else {
// `?` is not allowed as a Kleene op in 2015,
// but is allowed in the 2018 edition
sess.span_diagnostic
.struct_span_err(op1_span, "expected `*` or `+`")
.note("`?` is not a macro repetition operator in the 2015 edition, \
but is accepted in the 2018 edition")
.emit();

// Return a dummy
return (None, KleeneOp::ZeroOrMore);
}
}

// #1 is a separator followed by #2, a KleeneOp
Ok(Err((tok, span))) => match parse_kleene_op(input, span) {
// #2 is a `?`, which is not allowed as a Kleene op in 2015 edition,
// but is allowed in the 2018 edition
Ok(Ok((op, op2_span))) if op == KleeneOp::ZeroOrOne => {
sess.span_diagnostic
.struct_span_err(op2_span, "expected `*` or `+`")
.note("`?` is not a macro repetition operator in the 2015 edition, \
but is accepted in the 2018 edition")
.emit();

// Return a dummy
return (None, KleeneOp::ZeroOrMore);
}

// #2 is a KleeneOp :D
Ok(Ok((op, _))) => return (Some(tok), op),

// #2 is a random token :(
Ok(Err((_, span))) => span,

// #2 is not a token at all :(
Err(span) => span,
},

// #1 is not a token
Err(span) => span,
};

sess.span_diagnostic.span_err(span, "expected `*` or `+`");

// Return a dummy
(None, KleeneOp::ZeroOrMore)
}

// `?` is a Kleene op, not a separator
fn parse_sep_and_kleene_op_2018<I>(
input: &mut Peekable<I>,
fn parse_sep_and_kleene_op(
input: &mut Peekable<impl Iterator<Item = tokenstream::TokenTree>>,
span: Span,
sess: &ParseSess,
_features: &Features,
_attrs: &[ast::Attribute],
) -> (Option<token::Token>, KleeneOp)
where
I: Iterator<Item = tokenstream::TokenTree>,
{
) -> (Option<token::Token>, KleeneOp) {
// 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 `?` (needs feature gate)
Expand Down
33 changes: 33 additions & 0 deletions src/test/run-pass/macros/macro-at-most-once-rep-2015.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// run-pass
#![allow(unused_mut)]
// 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 non-error cases and making sure the correct number of repetitions happen.

// edition:2015

macro_rules! foo {
($($a:ident)? ; $num:expr) => { {
let mut x = 0;

$(
x += $a;
)?

assert_eq!(x, $num);
} }
}

pub fn main() {
let a = 1;

// accept 0 or 1 repetitions
foo!( ; 0);
foo!(a ; 1);
}
2 changes: 1 addition & 1 deletion src/test/ui/issues/issue-39388.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#![allow(unused_macros)]

macro_rules! assign {
(($($a:tt)*) = ($($b:tt))*) => { //~ ERROR expected `*` or `+`
(($($a:tt)*) = ($($b:tt))*) => { //~ ERROR expected one of: `*`, `+`, or `?`
$($a)* = $($b)*
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/issues/issue-39388.stderr
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error: expected `*` or `+`
error: expected one of: `*`, `+`, or `?`
--> $DIR/issue-39388.rs:4:22
|
LL | (($($a:tt)*) = ($($b:tt))*) => {
Expand Down
13 changes: 0 additions & 13 deletions src/test/ui/macros/macro-at-most-once-rep-2015-ques-rep.rs

This file was deleted.

18 changes: 0 additions & 18 deletions src/test/ui/macros/macro-at-most-once-rep-2015-ques-rep.stderr

This file was deleted.

28 changes: 0 additions & 28 deletions src/test/ui/macros/macro-at-most-once-rep-2015-ques-sep.rs

This file was deleted.

24 changes: 0 additions & 24 deletions src/test/ui/macros/macro-at-most-once-rep-2015-ques-sep.stderr

This file was deleted.

41 changes: 41 additions & 0 deletions src/test/ui/macros/macro-at-most-once-rep-2015.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// Tests that `?` is a Kleene op and not a macro separator in the 2015 edition.

// edition:2015

macro_rules! foo {
($(a)?) => {};
}

macro_rules! baz {
($(a),?) => {}; //~ERROR the `?` macro repetition operator
}

macro_rules! barplus {
($(a)?+) => {}; // ok. matches "a+" and "+"
}

macro_rules! barstar {
($(a)?*) => {}; // ok. matches "a*" and "*"
}

pub fn main() {
foo!();
foo!(a);
foo!(a?); //~ ERROR no rules expected the token `?`
foo!(a?a); //~ ERROR no rules expected the token `?`
foo!(a?a?a); //~ ERROR no rules expected the token `?`

barplus!(); //~ERROR unexpected end of macro invocation
barplus!(a); //~ERROR unexpected end of macro invocation
barplus!(a?); //~ ERROR no rules expected the token `?`
barplus!(a?a); //~ ERROR no rules expected the token `?`
barplus!(a+);
barplus!(+);

barstar!(); //~ERROR unexpected end of macro invocation
barstar!(a); //~ERROR unexpected end of macro invocation
barstar!(a?); //~ ERROR no rules expected the token `?`
barstar!(a?a); //~ ERROR no rules expected the token `?`
barstar!(a*);
barstar!(*);
}
Loading

0 comments on commit 097e262

Please sign in to comment.