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

Fix extra toP4 whitespace #3606

Merged
merged 4 commits into from
Oct 24, 2022
Merged

Fix extra toP4 whitespace #3606

merged 4 commits into from
Oct 24, 2022

Conversation

fruffy
Copy link
Collaborator

@fruffy fruffy commented Oct 23, 2022

Fixes toP4's extra whitespace that is added at the end of the program and after annotations. This PR changes lots of files, spot checking will be necessary.

@fruffy fruffy force-pushed the top4_whitespace branch 2 times, most recently from f2266c1 to b137f5f Compare October 23, 2022 23:51
@fruffy fruffy marked this pull request as ready for review October 24, 2022 00:16
@fruffy fruffy force-pushed the top4_whitespace branch 2 times, most recently from 1b59e8e to 20eb355 Compare October 24, 2022 00:46
@jafingerhut
Copy link
Contributor

I looked over the changed test programs in the p4_16_samples directory of this PR. It appears that basically these test programs included v1model.p4, but were not written to use the v1model architecture, and your changes modify them so that they do use the v1model architecture.

I did not carefully check the original reason that these test programs were written. It would be good if we did not unintionally change the reason they were created, and what cases within the compiler's implementation that they cover. If these changes did happen to do that, another possible change to the test program would be to no longer include the v1model.p4 file.

@fruffy
Copy link
Collaborator Author

fruffy commented Oct 24, 2022

I chose to make them complete v1model programs to add more test cases for the behavioral model (still missing the extension). However, just removing the include also works. I will also move those files to a separate PR since the diff has blown up.

@fruffy
Copy link
Collaborator Author

fruffy commented Oct 24, 2022

Migrated the test file cleanup to #3608.

@fruffy fruffy changed the title Fix up incorrect v1model programs - fix toP4 whitespace Fix extra toP4 whitespace Oct 24, 2022
@fruffy fruffy merged commit 5a4299f into main Oct 24, 2022
@fruffy fruffy deleted the top4_whitespace branch October 26, 2022 00:22
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