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

Should petr4 give an error for program with 2 actions, same name & signature, different bodies? #3

Closed
jafingerhut opened this issue May 12, 2019 · 6 comments
Assignees

Comments

@jafingerhut
Copy link

I do not see how the attached program should not be ambiguous or give some kind of error, unless the intent is that the later definition of action foo is supposed to override or shadow the earlier one. Even if that is the case, the JSON output by petr4 includes both definitions of foo, which doesn't seem correct.

Related to these 2 issues on p4c, but the program attached to this issue is slightly different than any of the ones on those issues:

The latest p4test as of 2019-May-10 does give an error message when attempting to compile the attached program.
actions-same-name3.p4.txt

@jafingerhut
Copy link
Author

Similarly, two functions, controls, or parsers with the same signature but different bodies cause the latest p4test as of 2019-May-10 to give errors, but petr4 quietly accepts them. Attaching the programs for reference.

parsers-same-name1.p4.txt
functions-same-name1.p4.txt
controls-same-name1.p4.txt

@jnfoster
Copy link
Contributor

This public version of petr4 (we have a development version that is private) only includes the parser. So what these examples show, assuming it's not a bug in our parser, is that the programs that the open-source C++ code is rejecting are being rejected in semantic analysis, not in the parser (which I believe Mihai conjectured).

We do have a type-checker in preparation, which we plan to release soon. It should give a complete front-end equivalent to the open-source C++ code.

@jafingerhut
Copy link
Author

Got it. I had been guessing that the current public petr4 had some semantic analysis as well. I will hold off on creating more issues like this unless I find something that looks amiss in petr4's syntax parsing.

@jnfoster
Copy link
Contributor

Actually, this comment suggests that at least one of the examples should be a parse error.

p4lang/p4c#1932 (comment)

It may well be a bug in our parser...

@jafingerhut
Copy link
Author

The way I understood that comment of Mihai's is that he wishes some of those error messages were better, but it is difficult to make them clearer the way they use Bison and the lexer, I think by installing defined type names as new recognized lexer symbols.

@hackedy hackedy assigned hackedy and unassigned hackedy May 5, 2020
@hackedy hackedy self-assigned this Jun 19, 2020
@hackedy
Copy link
Collaborator

hackedy commented Jun 22, 2020

Closing as a duplicate of #135

@hackedy hackedy closed this as completed Jun 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants