-
Notifications
You must be signed in to change notification settings - Fork 449
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
Create a new state for initializations #2319
Conversation
} | ||
state L3_i { | ||
b.extract<I>(hdr.i); | ||
transition L3_start; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I might be missing something, but this file issue2314-frontend.p4 seems to be showing the same bug that exists without this change: state L3_i transitions to L3_start, which makes an assignment to l3_etherType that should only be done when the sub parser is first called. Should this transition be to L3_start_0 instead, perhaps?
frontends/p4/moveDeclarations.h
Outdated
@@ -67,17 +67,20 @@ class MoveDeclarations : public Transform { | |||
|
|||
/** After MoveDeclarations, some variable declarations in the "local" | |||
* section of a parser and control may still have initializers; these are moved | |||
* into the start state, and to the beginning of the apply body repectively. | |||
* into the a new start state, and to the beginning of the apply body repectively. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor typo: s/into the a new start state/into a new start state/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, I don't know enough about the compiler internals to review those, but the new test program issue2314.p4 and its expected outputs look correct to me, with the 2nd commit.
Fixes #2314