Possible updates/corrections to P4_16 spec grammar #928
Conversation
|
@mbudiu-vmw So you believe the other occurrences of THIS added by this PR are not related to abstract methods, and should be included in spec grammar? |
|
this is only used with abstract methods. |
|
@mbudiu-vmw The annotationToken had one production in grammar.mdk |
|
@mbudiu-vmw to check the "voids". |
p4-16/spec/P4-16-spec.mdk
Outdated
| @@ -2642,6 +2642,7 @@ P4 supports extern object declarations and extern function declarations using th | |||
| externDeclaration | |||
| : optAnnotations EXTERN nonTypeName optTypeParameters '{' methodPrototypes '}' | |||
| | optAnnotations EXTERN functionPrototype ';' | |||
| | optAnnotations EXTERN name ';' | |||
There was a problem hiding this comment.
@jafingerhut Action item for me to propose adding a sentence or two to the spec for this. The basic idea is that it enables giving a kind of forward declaration for an extern name, before it is fully defined. Chris Dodd believes the motivation for this being in the grammar was for Register and RegisterAction definition in TNA.
There was a problem hiding this comment.
I have added some proposed text for this, which should be reviewed.
There was a problem hiding this comment.
During 2021-May-03 LDWG meeting it was decided to leave forward extern declarations out of the spec. It will remain in the p4c implementation, which already includes several features marked experimental that are not in the spec.
AI Andy: Remove this part of this PR and update it today.
| @@ -3057,6 +3058,7 @@ typeArg | |||
| : DONTCARE | |||
| | typeRef | |||
| | nonTypeName | |||
| | VOID | |||
There was a problem hiding this comment.
@mbudiu-vmw Mihai took the action item in LDWG meeting to think about why VOID is here, and also the occurrence in the production of the realTypeArg nonterminal of the grammar.
There was a problem hiding this comment.
Perhaps this may make sense for some extern objects that could be instantiated with "void" for some type parameters.
p4-16/spec/P4-16-spec.mdk
Outdated
| ; | ||
| ~ End P4Grammar | ||
|
|
||
| The last case is a forward extern declaration, intended for declaring |
There was a problem hiding this comment.
I guess this allows mutually-recursive declarations (instantiations). Otherwise this would be impossible.
There was a problem hiding this comment.
I do not know of an example, other than Chris Dodd's mention during the LDWG meeting that perhaps this was added because of the way that the TNA architecture defines the Register and RegisterAction externs. He did not sound confident that this was the motivating reason, so it might be some other reason. I do not see any occurrences of forward declarations of externs in this public file, for example: https://github.com/barefootnetworks/Open-Tofino/blob/master/share/p4c/p4include/tofino.p4
There was a problem hiding this comment.
Following discussion at the 2021-May-03 LDWG meeting, I have removed all changes related to forward extern declarations from this PR.
It shall remain an experimental feature implemented in p4c, but not made part of the language spec without further consideration.
|
@mbudiu-vmw I do not recall whether it was decided to merge this PR into the spec today, but if it was, note that I have removed all of the changes related to the forward declarations of externs from this PR, as suggested during today's LDWG meeting. |
I found a few differences between the latest p4lang/p4c P4_16 grammar, as compared to the grammar.mdk file in the P4_16 spec repository, when reviewing another issue on the spec recently.
I suspect that some of these differences might be due to experimental features in p4c, but some of them look like things we might want to update in the spec's grammar.mdk file. Happy to get comments about which differences are in which category, and update this PR.