p4lang / p4-spec

Apache License 2.0
177 stars 80 forks source link

Name duplication and name shadowing #974

Closed MollyDream closed 11 months ago

MollyDream commented 3 years ago

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 (https://github.com/p4lang/p4c/issues/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 commented 2 years ago

@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 commented 2 years ago

The old report https://github.com/p4lang/p4c/issues/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: https://github.com/p4lang/p4c/issues/2755

jnfoster commented 11 months ago

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.