Skip to content

Macro rules: using lifetime specifier inside recursion causes local ambiguity #50903

Closed

Description

@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

Activity

  1. added
    A-macrosArea: All kinds of macros (custom derive, macro_rules!, proc macros, ..)
    C-bugCategory: This is a bug.
    on May 19, 2018
  2. kennytm commented on May 19, 2018

    @kennytm
    Member

    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.

    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.

  3. added
    E-mentorCall for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion.
    T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.
    on May 19, 2018
  4. yaahc commented on May 21, 2018

    @yaahc
    Member

    I'd like to take this issue

  5. yaahc commented on May 25, 2018

    @yaahc
    Member

    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.

  6. kennytm commented on May 25, 2018

    @kennytm
    Member

    @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.)

  7. yaahc commented on May 25, 2018

    @yaahc
    Member
    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

  8. added a commit that references this issue on Jun 11, 2018
    13f8d07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-macrosArea: All kinds of macros (custom derive, macro_rules!, proc macros, ..)C-bugCategory: This is a bug.E-mentorCall for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions