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

Save the key names in @name annotations: fix for issue #259 #267

Merged
merged 1 commit into from
Jan 30, 2017

Conversation

mihaibudiu
Copy link
Contributor

This fixes issue #259 by preserving the key names for all tables using @name annotations.
This has to be run very early in the front-end, before doing any renaming; otherwise identifier names may change.
This does not really solve the problem, but it just passes the buck to issue #219. We should use these @name annotations when generating the PI.
Unfortunately most reference outputs have changed.

@ChrisDodd ChrisDodd self-assigned this Jan 30, 2017
Copy link
Contributor

@ChrisDodd ChrisDodd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code seems excessively verbose to my taste -- it seems like a more concise implementation (without all the converting nulls to empty strings and then testing for empty strings, and extra indirection calls) would be more readable. As is, it looks like there are a bunch of special cases it has to deal with but there is really only one, which is somewhat obscured as a result.

@ChrisDodd ChrisDodd merged commit bd7905c into p4lang:master Jan 30, 2017
@cc10512
Copy link
Contributor

cc10512 commented Jan 30, 2017

I am puzzled why we need to hand annotate all these names manually, and essentially repeat the name for the key in a @name annotation. The front end should be able to generate the annotations before any transformation, and then we would need/require hand annotations only for expressions or other corner cases.

@mihaibudiu
Copy link
Contributor Author

They are not annotated manually; the -first.p4 and -midend.p4 files are actually generated by the compiler exactly as you describe. They contain the program representation at a specific point in the compiler pipeline.

@mihaibudiu
Copy link
Contributor Author

@ChrisDodd: There's no nullptr cstring, so I didn't know how to do it less verbose. I would appreciate a suggestion.

@cc10512
Copy link
Contributor

cc10512 commented Jan 30, 2017

Oh, I missed that these were "samples". Thanks for pointing it out.

@ChrisDodd
Copy link
Contributor

ChrisDodd commented Jan 30, 2017 via email

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

Successfully merging this pull request may close these issues.

3 participants