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

[WIP - DO NOT MERGE] Add native preprocessor #68

Closed
wants to merge 1 commit into from

Conversation

jblazquez
Copy link

@ChrisDodd @mbudiu-vmw

This is the prototype implementation for the native preprocessor proposal I posted here: p4lang/p4-spec#30

All existing unit tests pass except those that depend on the exact output of error messages, which show minor visual differences between the C preprocessor and this one. For example, the C preprocessor doesn't preserve whitespace between tokens (this one does) and it also removes comments (this one doesn't).

This PR doesn't include changes to the test data (#include -> #import, among other things) because that would clutter it too much.

The preprocessor ended up being decently small. Hopefully it gives a good idea of the effort required to implement the proposal.

@@ -0,0 +1,510 @@
/*
Copyright 2013-present Barefoot Networks, Inc.
Copy link
Contributor

Choose a reason for hiding this comment

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

You should probably no use Barefoot here.

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, I copied this from another file and didn't notice it.

@sethfowler
Copy link
Contributor

This seems like a great patch; should we move forward on this?

@ChrisDodd
Copy link
Contributor

Problem is that as written, this preprocessor is quite different from the C preprocessor.

@sethfowler
Copy link
Contributor

Hmm, I see. I just took a look at p4lang/p4-spec#30 where this is discussed and it seems like it sort of petered out.

If we did want to go in this direction, perhaps the easiest thing to do would be to use an existing BSD-licensed CPP implementation, which we could gradually evolve in whatever direction the P4 language design committee desired.

Obviously not a high priority thing.

@jnfoster
Copy link
Contributor

Should we close this pull request since (although it has many nice features) the Language Design Working Group has not chosen to take this for P4-16?

@ChrisDodd
Copy link
Contributor

This is now thoroughly out of date, so I'm going to close it.

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.

5 participants