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

Name duplication and name shadowing #974

Closed
MollyDream opened this issue Oct 22, 2021 · 3 comments
Closed

Name duplication and name shadowing #974

MollyDream opened this issue Oct 22, 2021 · 3 comments
Labels

Comments

@MollyDream
Copy link
Contributor

The p4 spec does not specify the behavior of name duplication (reusing names in the same scope) and name shadowing (reusing names in the inner scope) in P4.
Litmus Test 1

void f1() {
  int x;
  int x;
}

Litmus Test 2

void f2() {
  int x;
  { int x; }
}

Litmus Test 3

void f3(inout bit<8> x) {
  bit<8> x;
}

Litmus Test 4

void f4(inout bit<8> x) {
  { bit<8> x; }
}

Since there are no definitions in P4, we have to resort to a similar language for reference, namely C. For the examples above, test 1 and 3 are considered name duplication, and 2 and 4 considered name shadowing in C. Also in C, both the test 1 and 3 are invalid, and both 2 and 4 are valid.
An old issue report of p4c (p4lang/p4c#1932) states that name duplication is generally not allowed, and the only exceptions are overloading, which, based on the current spec, is now restrained to only functions and methods. (That post is outdated in that overloading is not allowed on any other types, so controls, actions, parsers and packages should not share the same names and the P4c compiler should reject those test cases.)
However, there are p4c test examples similar to test 3 that are allowed, like shadow-after-use.p4 and shadow3.p4. The naming of these examples already implies that P4 is right now regarding test 3 as shadowing, which differs from C. Personally I think the two x's in test 3 are in the same scope, so I think it is more likely name duplication. I think it would be valuable to reevaluate this design decision and to clarify the definitions of name duplication in the spec.
Also, it is unclear if there are any constraints on name shadowing in P4, though it seems that there should be none. For example, name shadowing of two different types should also be valid.

@MollyDream
Copy link
Contributor Author

@mbudiu-vmw Do you have any comments on this? Based on the post, I think the spec should

Clarify the definition of name duplication and name shadowing
State clearly the situations (overloading) where name duplication is allowed
State clearly the situations (everywhere?) where name shadowing is allowed
This is supposed to happen in Section 6.8.

@jfingerh
Copy link
Contributor

The old report p4lang/p4c#1932 mentioned above did result in p4c changing almost all cases of duplicate names for at least top level names to be a compile-time error. The one notable exception to that is action names, if I recall correctly. p4c internally can take one action defined in the original P4 program, and create duplicates of it at the same scope, e.g. if it is invoked from multiple tables in the same control. To me this seems error-prone (e.g. the issue I link below is related to this), and it would be nice some day if we had a clear implementation approach for this that didn't lead to such subtle bugs.

This issue is not the same as the one addressed here, but I wanted to at least point out that duplicate @name annotations may also have subtleties here, and/or bugs in the current p4c implementation: p4lang/p4c#2755

@jnfoster
Copy link
Collaborator

In the interest of tidying up the set of active issues on the P4 specification repository, I'm marking this as "stalled" and closing it. Of course, we can always re-open it in the future if there is interest in resurrecting it.

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

No branches or pull requests

3 participants