-
Notifications
You must be signed in to change notification settings - Fork 446
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
Allow to use simple types inside out struct as a parser's parameter #3134
base: main
Are you sure you want to change the base?
Allow to use simple types inside out struct as a parser's parameter #3134
Conversation
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.
This is missing an STF test that actually runs the behavioural model.
I will check the validation error. All the other tests pass. |
In the test program for this PR, there are non-header fields of the struct type Are your changes intentionally trying to generalize this restriction? If so, I am curious why you want to do so? Here is where the restriction is documented for the v1model architecture: https://github.com/p4lang/behavioral-model/blob/main/docs/simple_switch.md#restrictions-on-type-h Here is the PR for the PSA specification planning to document this restriction for the PSA architecture: p4lang/p4-spec#722 |
@jafingerhut , |
I understand that it might be fairly straightforward to remove the restrictions on type H in v1model. Because the type M for user-defined metadata exists and is also available as output from the parser and input to the control, I don't see a motivating reason why the restriction on H should be removed -- just document that users put those in type M instead. Digging through my memory, I am not sure, but one possible reason for having two separate user-defined types H and M is that their directions can be defined differently, e.g. out for H from parser, inout for M from parser. |
I think that we can propose the same solution us I did it for log_msg. It could be some new pass which translates simple data types into another header. I will do it later with low priority. |
@VolodymyrPeschanenkoIntel Are you still interested in pursuing these changes? Or perhaps OK to close this PR as abandoned? Note: If you do still wish to pursue it, I am still curious what the motivation for the generalization is. Perhaps it is simply "I know how to generalize it, so I wanted to?" |
The problem was in some additional checking which disable possibility to use struct with simple types as an output parameter for a parser. behavioral-model supports such possibility.