p4lang / p4-spec

Apache License 2.0
175 stars 80 forks source link

Anomalous non-use of optAnnotations in grammar #295

Closed tomrodeheffer closed 7 years ago

tomrodeheffer commented 7 years ago

In P4_16 Language Specification 2017-05-22, Section E contains the production:

optAnnotations
    : /* empty */
    | annotations
    ;

which makes it convenient to emplace optional annotations into all the different places in the grammar in which they can appear. However, there are a few places in the grammar that anomalously decline to use this convenient non-terminal, namely:

instantiation
    : typeRef '(' argumentList ')' name ';'
    : annotations typeRef '(' argumentList ')' name ';'
    ;
typedefDeclaration
    : annotations TYPEDEF typeRef name ';'
    | TYPEDEF typeRef name ';'
    | annotations TYPEDEF derivedTypeDeclaration name ';'
    | TYPEDEF derivedTypeDeclarationame ';'
    ;
variableDeclaration
    : annotations typeRef name optInitializer ';'
    | typeRef name optInitializer ';'
    ;

Now, the fourth alternative for typedefDeclaration is clearly a typo, since obviously it should read

    | TYPEDEF derivedTypeDeclaration name ';'

Assuming that this typo is corrected, then the reader is left to wonder what in the world the grammar is trying to imply in these productions by using extra verbiage rather than the convenient non-terminal optAnnotations.

I understand that the grammar has been transcribed from the parser definition and these anomalies are probably the result of historical accident. Nonetheless, they should be cleaned up.

mihaibudiu commented 7 years ago

We have fixed the typos in the master document, but we haven't yet released the new pdf/html. I think that we should rebuild the pdf/html for such small bug-fixes.

Unfortunately simplifying some of these productions with optAnnotations makes the grammar ambiguous. The grammar could be much nicer without bison limitations. It looks like we can simplify the typedef production, though.

jnfoster commented 7 years ago

Good point. We can post an official live "master" version on p4lang.github.io. I'll take care of this.

-N

On Fri, Jun 23, 2017 at 3:01 PM, Mihai Budiu notifications@github.com wrote:

We have fixed the typos in the master document, but we haven't yet released the new pdf/html. I think that we should rebuild the pdf/html for such small bug-fixes.

Unfortunately simplifying some of these productions with optAnnotations makes the grammar ambiguous. The grammar could be much nicer without bison limitations. It looks like we can simplify the typedef production, though.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/p4lang/p4-spec/issues/295#issuecomment-310782601, or mute the thread https://github.com/notifications/unsubscribe-auth/ABwi0m7L1ppzTRZ55hdxscyE9zltbyM9ks5sHDWogaJpZM4OEEyL .

mihaibudiu commented 7 years ago

I will submit a PR to change the grammar for typedef. I have tried the other two changes on the bison file (frontends/parsers/p4/p4parser.ypp) and they both produce lots of shift/reduce conflicts. We have chosen to use the real bison grammar in the spec (minus some very important missing pieces, like priorities and the lexer) so that we are sure that it is implementable using bison.