Skip to content
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

Ignore typeless structs in unstructured annotations #5058

Merged
merged 1 commit into from
Dec 19, 2024

Conversation

ChrisDodd
Copy link
Contributor

This pass expects struct expressions in unstructured annotations to have explicit types. Using { } in unstructured annotation is actually allowed by the language spec, it does not make sense for the frontend to prevent this (in fact any sequence of tokens can be used as long as parentheses are balanced).

@ChrisDodd ChrisDodd requested a review from fruffy December 9, 2024 23:07
@fruffy fruffy added the core Topics concerning the core segments of the compiler (frontend, midend, parser) label Dec 11, 2024
@ChrisDodd ChrisDodd requested review from asl and jafingerhut December 12, 2024 23:08
@ChrisDodd ChrisDodd force-pushed the emre-annot-typecheck branch from d247542 to 3f4ca76 Compare December 12, 2024 23:52
This pass expects struct expressions in unstructured annotations to have
explicit types. Using { } in unstructured annotation is actually allowed
by the language spec, it does not make sense for the frontend to prevent
this (in fact any sequence of tokens can be used as long as parentheses
are balanced). nvp4c will need to use untyped struct expressions in this
context to allow the user to provide values for the fields of headers.

Signed-off-by: Chris Dodd <[email protected]>
@asl
Copy link
Contributor

asl commented Dec 13, 2024

Is there any test for this fix?

@ChrisDodd
Copy link
Contributor Author

ChrisDodd commented Dec 13, 2024

Is there any test for this fix?

Actually testing this requires a backend that has an annotation that contains this form (an untyped brace-enclose kv-list as an expression within an unstructured annotation) and has an appropriate PARSE_X to get it parsed. If you just use the p4test backend, all unstructured annotations remain unparsed.

We could add some annotation parsers to p4test -- somthing like PARSE_EXPRESSION_LIST("expr_list") would allow writing testcases that have @expr_list( ..some expressions.. ) and would test all the various paths for parsed unstructured annotations. Just on general principles, we should add an annotation for every generic PARSE_X macro defined in the common code and write some testcases that excercise them.

@asl
Copy link
Contributor

asl commented Dec 18, 2024

Actually testing this requires a backend that has an annotation that contains this form (an untyped brace-enclose kv-list as an expression within an unstructured annotation) and has an appropriate PARSE_X to get it parsed. If you just use the p4test backend, all unstructured annotations remain unparsed.

Ah right. Completely forgot that it's only end-to-end testing that is available...

@ChrisDodd ChrisDodd added this pull request to the merge queue Dec 19, 2024
Merged via the queue into p4lang:main with commit d8c1d1b Dec 19, 2024
19 checks passed
@ChrisDodd ChrisDodd deleted the emre-annot-typecheck branch December 19, 2024 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Topics concerning the core segments of the compiler (frontend, midend, parser)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants