Macro rules: using lifetime specifier inside recursion causes local ambiguity

[jjedelsky]

# is not valid lifetime, so using :litetime specifier should not cause local ambiguity.

Example:

macro_rules! bug {
    ($($lif2:lifetime ,)* #) => {};
}

bug!('a, #);

error: local ambiguity: multiple parsing options: built-in NTs lifetime ('lif2') or 1 other option.
 --> src/main.rs:7:10
  |
7 | bug!('a, #);
  |          ^

Playground link

cc tracking issue for $:lifetime matcher: #34303

[kennytm]

The problem is that currently a lifetime "may start with anything" because we haven't spelled out the rules.

Mentoring instruction

Edit macro_parser.rs, and insert the case "lifetime" into the may_begin_with() function.

rust/src/libsyntax/ext/tt/macro_parser.rs

Lines 725 to 739 in 5c3960c

	fn may_begin_with(name: &str, token: &Token) -> bool {
	/// Checks whether the non-terminal may contain a single (non-keyword) identifier.
	fn may_be_ident(nt: &token::Nonterminal) -> bool {
	match *nt {
	token::NtItem(_) | token::NtBlock(_) | token::NtVis(_) => false,
	_ => true,
	}
	}
	match name {
	"expr" => token.can_begin_expr(),
	"ty" => token.can_begin_type(),
	"ident" => get_macro_ident(token).is_some(),
	"literal" => token.can_begin_literal_or_bool(),
	"vis" => match *token {

You may also want to review this function to see if any case did not consider NtLifetime and recent syntax additions.

[yaahc]

I'd like to take this issue

[yaahc]

I'm on a train right now with limited connectivity, so I cant push my PR, but I have a change that I've compiled and tested, and I'm not sure if it's working correctly, I could use some more background.

I added this to may_begin_with, compiled and set as a custom toolchain all good so far

        "lifetime" => match *token {
            Token::Pound => false,
            _ => true,
        },

I created a new cargo bin project (I didn't see any tests for macro_parser or any test dirs in the libsyntax directory, not sure if there's a better way to do this) and made the main function define that macro and use it, as in the original example. With nightly it showed the expected ambiguity error. Then I used rustup overrides with my custom toolchain and rebuilt, and it compiled with no errors. Is this the expected behavior when parsing the macro? I'm unclear on how the macro parser works in rust. Is it that, before adding this branch, when parsing 'a, # they both matched the :lifetime specifier. But after adding the bit that says "no that ain't a lifetime," it no longer gets caught by that arm of the macro, and when parsing the #, it just skips it and doesn't complain? I guess I was expecting it to say something like "lifetimes cant start with #" instead of the local ambiguity, not for it to just stop being an error.

Also, you mentioned reviewing the rest of the function to see if other things didn't consider lifetime or other new syntax additions, but I'm not sure what other syntax additions there are to consider.

[kennytm]

@jrlusby

Then I used rustup overrides with my custom toolchain and rebuilt, and it compiled with no errors. Is this the expected behavior when parsing the macro?

Yes this is the expected behavior. When may_begin_with returns false, the parser knows that # is definitely not a :lifetime and thus stop matching against $($lif2:lifetime,)*. The original template expects a # next, which it matched, and thus everything goes fine.

This is similar to the behavior of

macro_rules! no_bug {
    ($($lif2:ident ,)* #) => {};
}
fn main() {
    no_bug!(a, #);
}

Also, you mentioned reviewing the rest of the function to see if other things didn't consider lifetime or other new syntax additions, but I'm not sure what other syntax additions there are to consider.

Basically to check the interaction with all other fragment specifiers, e.g.

macro_rules! check {
    (1. $($a:lifetime,)* $b:block) => {};
    (2. $($b:block,)* $a:lifetime) => {};
}
fn main() {
    check!(1. 'a, 'b, {});
    check!(2. {}, {}, 'a);
}

Interpolated lifetimes are encoded as NtLifetime which I don't see listed in the may_begin_with function. But maybe adding a "lifetime" case is sufficient to disambiguate the above examples.

(For new syntax I'd like to reserve DotDotEq alongside DotDotDot in :pat, though that would be off-topic and could also be a breaking change.)

[yaahc]

error: local ambiguity: multiple parsing options: built-in NTs lifetime ('a') or block ('b').
  --> src/main.rs:12:23
   |
12 |     check!(1. 'a, 'b, {});
   |                       ^

apparently not