Refine parserStatement grammar in P4 parser#5638
Conversation
Signed-off-by: jaehyun1ee <99jaehyunlee@kaist.ac.kr>
jaehyun1ee
force-pushed
the
parser-statement-grammar
branch
from
f1022f6 to
d9b1818
Compare
|
It is perfectly acceptable to use Github CI checks for such PRs. I see all of them passing, so that is good. Setting up a local development environment so that it passes tests for all P4 targets can be painstaking and error-prone. |
jafingerhut
left a comment
jafingerhut
left a comment
There was a problem hiding this comment.
These grammar changes look consistent with the related p4-spec issue. LGTM
|
IMO it is better to make the parser more permissive (and simpler) allowing all constructs orthogonally in all contexts, and then in the ValidateParsedProgram pass have checks for statements in a context where they are not allowed. That both makes the parser simpler and easier to maintain, AND give better error messages (something like "switch statements not allowed in parser states" rather than a generic "syntax error" with only an indication of which token triggered the error). |
3 participants
This is a follow-up from a
p4-specissue in p4lang/p4-spec#1375.Previously,
parserStatmentgrammar in the specification was defined as:where the
conditionalStatementactually led to ordinarystatements (which allow more production thanparserStatements) being syntactically included withinparserStatement.A
p4-specPR (p4lang/p4-spec#1415) amends theparserStatementgrammar such that it is closed under itself, without a path tostatement. This PR is a mirror of it in p4c. In short, the P4 parser is refined as follows:parserState : optAnnotations STATE name { driver.structure->pushContainerType($3, false); } - "{" parserStatements transitionStatement "}" { + "{" parserStatOrDeclList transitionStatement "}" { driver.structure->pop(); $$ = new IR::ParserState(@3, std::move($3), std::move($1), std::move($6), $7); } ; -parserStatements - : %empty { $$ = {}; } - | parserStatements parserStatement { ($$ = std::move($1)).push_back($2); $$.srcInfo = @1 + @2; } - ; - parserStatement : assignmentOrMethodCallStatement { $$ = $1; } | emptyStatement { $$ = $1; } - | variableDeclaration { $$ = $1; } - | constantDeclaration { $$ = $1; } | parserBlockStatement { $$ = $1; } - | conditionalStatement { $$ = $1; } + | parserConditionalStatement { $$ = $1; } + ; + +parserStatementOrDeclaration + : variableDeclaration { $$ = $1; } + | constantDeclaration { $$ = $1; } + | parserStatement { $$ = $1; } + ; + +parserStatOrDeclList + : %empty { $$ = {}; } + | parserStatOrDeclList parserStatementOrDeclaration { ($$ = std::move($1)).push_back($2); + $$.srcInfo = @1 + @2; } ; parserBlockStatement - : optAnnotations "{" { driver.structure->pushNamespace(@2, false); } - parserStatements "}" { driver.structure->pop(); - $$ = new IR::BlockStatement(@1+@5, std::move($1), std::move($4)); } + : optAnnotations "{" { driver.structure->pushNamespace(@2, false); } + parserStatOrDeclList "}" { driver.structure->pop(); + $$ = new IR::BlockStatement(@1+@5, std::move($1), std::move($4)); } + ; + +parserConditionalStatement + : IF "(" expression ")" parserStatement %prec THEN + { $$ = new IR::IfStatement(@1, $3, $5, nullptr); } + | IF "(" expression ")" parserStatement ELSE parserStatement %prec THEN + { $$ = new IR::IfStatement(@1, $3, $5, $7); } ;Now,
parserStatementis similar to howstatementis defined:parserStatementOrDeclarationandparserStatOrDeclListcorrespond tostatementOrDeclarationandstatOrDeclListparserStateandparserBlockStatementnow hasparserStatOrDeclListas its field instead ofparserStatements, andparserStatementsproduction is now droppedparserStatementhas a newparserConditionalStatementproduction that is closed underparserStatementparserStatementhas a newdirectApplicationproduction, which was already in the P4 spec grammar, but was omitted in the p4c parserI've tested it with
cmake --build build --target check, where it seems to fail on:However, these appear to be environmental issues unrelated to this change: the
bmv2failures seem to stem from local linking issues/PTF version mismatches, and theebpf-kernelfailures are likely due tobpftoolexecution issues inside my local Docker container.As I am still familiarizing myself with the
p4cdevelopment environment, please let me know if there are specific testing workflows, target-specific checks, or CI behaviors I should look out for. Any guidance or feedback would be highly appreciated!