Semicolons in the parser are seen as invalid

[fruffy]

There is another expression that is seen as invalid by the p4-parser. This again happens in the parser function and is generated by some passes.

example.p4(40):syntax error, unexpected ;, expecting { or CONST                
        ;                                                                        
        ^                                                                        
error: 1 errors encountered, aborting compilation   

This error can be reproduced by simply adding ; after state_start.

An example where this occurs is fabric.p4 after the fabric-FrontEnd_31_SimplifyDefUse pass:

./p4c/build/p4c --top4 FrontEnd,MidEnd --dump dmp p4c/testdata/p4_16_samples/fabr
ic_20190420/fabric.p4

then run
./p4c/build/p4c dmp/fabric-FrontEnd_31_SimplifyDefUse.p4i
on the generated file.

This may or may not be related to #2126. Are semicolons valid in the parser? They seem to be in control functions.

2127.txt

[hesingh]

If you see the p4-16 grammar for the parser and its parser statement, an emptyStatement or ; is not supported by the parser. Thus this issue is not a bug.

https://github.com/p4lang/p4-spec/blob/master/p4-16/spec/grammar.mdk#L138

If you patch these code changes in your p4c workspace, you will see the error of this issue go away.

diff --git a/frontends/parsers/p4/p4parser.ypp b/frontends/parsers/p4/p4parser.ypp
index 856654ca..f45ac28c 100644
--- a/frontends/parsers/p4/p4parser.ypp
+++ b/frontends/parsers/p4/p4parser.ypp
@@ -716,6 +716,7 @@ parserStatement
     | variableDeclaration             { $$ = $1; }
     | constantDeclaration             { $$ = $1; }
     | parserBlockStatement            { $$ = $1; }
+    | emptyStatement                  { $$ = $1; }
     ;
 
 parserBlockStatement

[jafingerhut]

Whether this is a bug, seems to depend upon whether it is considered a bug by the p4c developers that the --top4 option can produce a P4 program at some intermediate compiler step that is not syntactically correct according to the spec. It does seem to be creating such a syntactically incorrect program in this case.

[ChrisDodd]

Adding this to the language (allowing emtpy ; statements/declarations anywhere they don't introduce ambiguities) would be a reasonable thing to do, and could be proposed to the language committee.

[hesingh]

Ah, sorry, I just used fruffy's P4 program and thought the program has the issue. Now, I see some pass is adding the emptyStatement to the p4i file. Sure, then it's an issue with some pass because if the grammar does not support the statement a pass should not add such a statement to any parser code.

Before we go to the LDWG, it would be good to find out why a pass is generating incorrect grammar. However, if the emptyStatement is removed by the time FrontEndLast pass is invoked, we could discuss more what to do with this issue.

[hesingh]

@fruffy I wonder if fabric.p4 is a p4-14 program which was used? I googled for the program and found one which is p4-14 code.

[jafingerhut]

@hesingh fabric.p4 is a P4_16 program, with a version from a particular date copied into the p4c source code. These comments in the p4c copy tell where the original version was copied from, and what few small changes were made in the p4c copy: https://github.com/p4lang/p4c/blob/master/testdata/p4_16_samples/fabric_20190420/fabric.p4#L17-L30

[hesingh]

@jafingerhut thanks. I do see the same issue with fabric.p4 that fruffy reported. However, when I just dumped FrontEndLast, the issue does not exist. As I said before, if FrontEndLast does not show any incorrect code, I wonder if one should chase any intermediate pass's incorrect code?

[jafingerhut]

"I wonder if one should chase any intermediate pass's incorrect code?" That is really up to the people who spend the most time maintaining p4c, which is my point of this earlier comment: #2127 (comment) I do not presume to answer that question for them.

[mihaibudiu]

The situation is a bit tricky. There are places where the compiler temporarily generates internally code that is not serializable as pure P4-16, but then eliminates such constructs. For example, there's an optimization pass which generates if statements in a parser, and then a subsequent pass which converts such if statements to new states. I don't remember if this is all packaged within a single "pass". We have never promised that the compiler can generate P4-16 anyplace, but I have tried to maintain this invariant as much as possible in the front-end and mid-end to simplify debugging.

That being said, we could allow empty statements in the parser. Maybe even ifs?

[hesingh]

I don't see any ambiguity if an empty statement is added to the parser.

Regarding if-statements in parser, I have my reservations. The parser's transition select is already something like an if-statement. If value == x, transition to state x, else if value == y, transition to state y.

[fruffy]

I also do not think this is a critical issue but it may be worth documenting. For context, we are currently working on a tool that analyzes correctness of individual passes in the P4 compiler and inconsistencies like these occasionally pop up.
We have not yet encountered invalid if-statements in the parser so this might be packaged.

[jafingerhut]

@fruffy The latest p4c should now accept empty statements in a parser. Can you try testing again, and if all goes well, close this issue as fixed?

[fruffy]

Yes, the issue got fixed so we can close this now.