Odd behavior with same-named actions

[jafingerhut]

I will attach example programs and full description after getting an issue number.

[jafingerhut]

This is not an urgent issue (at least not for any reasons I am aware of). I happened across this odd behavior while investigating a tiny bit further the issue of what top level objects are allowed to have the same name (issue #1932).

Program action-same-name1.p4 has two actions with the same name, but
different signatures, both with no directionless parameters,
declared within a control.

#include <core.p4>
#include <v1model.p4>

header h1_t { bit<8> f1; }
struct headers_t { h1_t h1; }
struct metadata_t { }

control ingressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_metadata_t stdmeta) {
    action foo (in bit<8> x, out bit<8> y) { y = (x >> 2); }
    action foo (inout bit<8> x) { x = (x >> 3); }
    apply {
        foo(hdr.h1.f1);
        foo(hdr.h1.f1, hdr.h1.f1);
    }
}
parser parserImpl(packet_in packet, out headers_t hdr, inout metadata_t meta, inout standard_metadata_t stdmeta) { state start { transition accept; } }
control verifyChecksum(inout headers_t hdr, inout metadata_t meta) { apply { } }
control egressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_metadata_t stdmeta) { apply { } }
control updateChecksum(inout headers_t hdr, inout metadata_t meta) { apply { } }
control deparserImpl(packet_out packet, in headers_t hdr) { apply { } }
V1Switch(parserImpl(), verifyChecksum(), ingressImpl(), egressImpl(), updateChecksum(), deparserImpl()) main;

p4test gives a an error 'foo: Duplicates declaration foo', which seems
reasonable to me. I do not know if that behavior is considered
correct or not.

Program action-same-name2.p4 is the same, except that the declarations
of the two actions has been moved to the top level of the program.

p4test gives no error messages at all.

Not only that, but the last front-end and mid-end pass versions of the
code both have two actions with the same @name(".foo") annotation on
two different actions inside of the ingressImpl control. The
resulting P4Info file has only one action named "foo" in it.

For one thing, it isn't clear to me whether the P4Info file even needs
to contain actions that are not associated with any tables, which
these actions are not.

Regardless of whether they should be in the P4Info file, if a user
wrote a P4_16 program with two objects having the same "kind" and
@name annotation in the same lexical scope, that seems like it should
be a name conflict bug that the compiler should give an error for.

The fact that the compiler is automatically generating such a program
with a name conflict thus also seems like a bug.

All input files, p4test command line parameters used, and output files I saw generated by p4test are in the attached zip file.

issue1936.zip

[jafingerhut]

I retested the example programs in the issue1936.zip file attached earlier, with several versions of p4c.

With the 2019-05-10 version, it gave the same results as described above, which should not be surprising, given that is likely what I tested with before creating the issue.

With the 2019-06-21 version just before the commit mentioned below, same results.

With the version of p4c built from this version of the source code:

commit 8eb631f9238416e4f13e2f222116be7a68570137
Author: Mihai Budiu <mbudiu@vmware.com>
Date:   Fri Jun 21 15:57:34 2019 -0700

both of the programs action-same-name1.p4 and action-same-name2.p4 give similar error messages when attempting to compile them, that there are duplicate things named 'foo'.

I am still not sure what is intended to be the correct behavior of the compiler on these programs, but wanted to update where things are today.

[jafingerhut]

This issue has had the 'fixed' label added to it, but it isn't clear to me which PR is the proposed fix for it. Does anyone know?

PR #1934 caused the behavior of the example program to change, but it isn't clear to me whether the behavior after that PR was merged is considered correct. It seems to depend upon whether the language should support overloading of multiple actions with the same name but different signatures.

[mihaibudiu]

The merge above suggests that the fix is in #1934

[mihaibudiu]

if this issue is fixed, please close it. if not, please remove the fixed label.

[jafingerhut]

I would gladly do so, if I knew whether the issue represents a bug, or correct behavior.

Why don't I already know?

Because I do not know whether it is supposed to be allowed in P4_16 to have multiple actions with the same name, but different arities. This is one part of this issue: p4lang/p4-spec#769

If the answer is "at least for actions, the spec should disallow defining multiple actions in the same scope, e.g. in the same control, with the same name, regardless of whether they have the same arity or different arity", then we can close this issue as fixed.

If the answer is "for actions, the spec should allow defining multiple actions with the same name but different arities, in the same control", then there is still a bug, and the PR did not address that problem with p4c.

I will not hold you to your previous statements if you now disagree with them, but at least on one other issue you have expressed an opinion that it should be perfectly fine for two actions to have the same name, if they are for different tables.

[mihaibudiu]

From the point of view of the language it's fine to have multiple actions as long as you can tell which one is being called. The control-plane API is a different matter. It may place independent constraints on what is allowed or not. But I don't think that should be aprt of the language spec - perhaps of the control-plane API spec. Note that multiple control-plane APIs are possible, so if P4Runtime forbids some combination it does not mean that other control-plane implementations cannot allow them.

[jafingerhut]

With the latest p4test as of today, 2019-Jul-09, the two test programs in the original issue give error messages with p4test, with no options to generate P4Runtime files.

Are these errors because of P4Runtime-specific code in p4test?

$ p4test actions-same-name1.p4 
actions-same-name1.p4(10): [--Werror=legacy] error: foo: Duplicates declaration foo
    action foo (inout bit<8> x) { x = (x >> 3); }
           ^^^
actions-same-name1.p4(9)
    action foo (in bit<8> x, out bit<8> y) { y = (x >> 2); }
           ^^^
error: 1 errors encountered, aborting compilation

$ p4test actions-same-name2.p4
actions-same-name2.p4(9): [--Werror=duplicate] error: foo duplicates foo.
action foo (inout bit<8> x) { x = (x >> 3); }
       ^^^
actions-same-name2.p4(8)
action foo (in bit<8> x, out bit<8> y) { y = (x >> 2); }
       ^^^

[hesingh]

The first error is catching a dup declaration. This is from core p4c functinality. The error is not caused by any p4runtime code. See

https://github.com/p4lang/p4c/blob/master/ir/indexed_vector.h#L49

The second error is from this line of p4c code.

https://github.com/p4lang/p4c/blob/master/frontends/p4/validateParsedProgram.cpp#L114

[mihaibudiu]

So should we close this?

[jafingerhut]

I have retested the two test programs in the ZIP file issue1936.zip attached to an earlier comment with the latest p4c, and it seems to give the same reasonable error messages as I reported in this earlier comment: #1936 (comment)

I am closing this issue.