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

P4 @name annotations contain perhaps-unsupported characters #347

Closed
jafingerhut opened this issue Feb 24, 2023 · 6 comments
Closed

P4 @name annotations contain perhaps-unsupported characters #347

jafingerhut opened this issue Feb 24, 2023 · 6 comments
Assignees

Comments

@jafingerhut
Copy link
Contributor

It is not clear to me yet, but perhaps open source p4c and/or its P4Runtime API implementation might not support characters in @name annotations that are not legal P4 identifiers. For example, it might not support the | character in this annotation:

@name("pa_validation|dash_pa_validation")

See this p4c issue where @fruffy has been facing difficulties in this area: p4lang/p4c#3885

@chrispsommers
Copy link
Collaborator

Hi Andy, @marian-pritsak would have more to say on this, but so far I am not aware of any problems resulting from the current DASH annotations. Also be aware that there is/was work underway to address #276 by adding custom annotations to direct the SAI code generator and remove the @name(x|y) approach.

@fruffy
Copy link

fruffy commented Feb 25, 2023

Thanks for opening this issue @jafingerhut. Yes, this issue is largely internal and will only become a problem when the IR is serialized into .p4 or .json file.

@r12f
Copy link
Collaborator

r12f commented Dec 13, 2023

ok, with the latest updates in DASH, all usage of "|" or ":" in the name are removed. we should not have this issue anymore. @jafingerhut and @fruffy do you have any other concerns on the name?

@fruffy
Copy link

fruffy commented Dec 13, 2023

Yes, we also fixed this problem with a couple compiler fixes for the inlining pass. Thanks for keeping track of this!

@r12f
Copy link
Collaborator

r12f commented Dec 14, 2023

Np at all! I guess we can finally get this long running issue closed! 🎉

@KrisNey-MSFT
Copy link
Collaborator

Closing per thread

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants