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.

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

Allow precedence annotations in macro defs #667

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

djrenren
Copy link

@djrenren djrenren commented May 31, 2022

This pull requests allows for use of precedence annotations inside macro definitions:

    grammar;
    Nothing: u32 = {}
    Expr<B>: u32 = {
       #[precedence(level="0")]
        B,
       #[precedence(level="1")]
       #[assoc(side="left")]
       <left:Expr<B>> "*" <right:Expr<B>> => 0,
    }

    FullExpr = Expr<Nothing>;

This change has two essential parts:

  • Extending precedence substitution to include macro names so that <right:Expr<B>> becomes <right:Expr0<B>>
  • Forwarding macro parameters when generating the preceding alternative so that we get:
    Expr<B>: u32 = {
      <left: Expr<B>> "*" <right: Expr0<B>> => 0,
      Expr0<B> // <-- Note that B is forwarded to the next level of precedence
    }
    

It seems to work well, my only concern is how to handle substitution of macro arguments in the presence of associativity.
That is, what should we get when have:

Expr<B>: u32 {
   #[precedence(level="0")]
   B,
   #[precedence(level="1")]
   #[assoc(side="left")]
    <left:Expr<B>> "*" <right:Expr<Expr<B>>> => 0,
}

Should it be <right: Expr0<Expr<B>> or <right: Expr0<Expr0<B>>. In this PR we get the former.

My suspicion is that substitution within macro args was already a bit wonky because associative replacement was done in a depth-first manner, skipping the first substitution. This meant that even prior to this change we'd have weird behavior like:

Foo: u32 {
   #[precedence(level="0")]
   "x" => 0,
   #[precedence(level="1")]
   #[assoc(side="left")]
   Bar<Foo, Foo> Foo => 0

becomes:

Foo: u32 {
   Bar<Foo, Foo0> Foo0 => 0
   Foo0
}

So the situation was already messy, but now it's a little worse that the macro name is also a possible substitution. Curious what people's thoughts are.

@nikomatsakis nikomatsakis added this to the 1.0 milestone Mar 21, 2023
@yannham yannham self-assigned this Apr 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants