p4lang
Clarify instantiation of objects with abstract methods
Following #1346, this PR adds more rules to the semantics of abstract method initialization.
First, it restricts that the abstract method being implemented, must have the same parameter names as declared in the object declaration.
This is to (i) disambiguate what abstract method is being implemented, and (ii) provide a clear interface for users.
For example, one may define an extern object with overloaded abstract methods, as follows.
extern Virtual {
Virtual();
abstract bit<32> g(inout bit<32> x); // (1)
abstract bit<32> g(inout bit<32> y); // (2)
}
And in the instantiation site,
Virtual() virt = {
bit<32> g(inout bit<32> z) { return x - 1; } // (a)
bit<32> g(inout bit<32> w) { return x + 1; } // (b)
};
Without a restriction on matching parameter names, it is unclear whether (a) implements (1) or (2), and (b) implements (1) or (2).
This can further cause confusion when invoking the abstract methods. P4 defines overload resolution via parameter names. If we were to allow such a program, then a user would have to invoke virt.g(z = 32w42); and vir.g(w = 32w42);, while the extern object declaration suggests that it should be callable by virt.g(x = 32w42); or virt.g(y = 32w42); instead. i.e., there is a gap between the interface defined in the extern object declaration and what is actually implemented.
The consequences of adding this restriction to the spec would be: virtual.p4, issue2175-1.p4, issue2175-3.p4, and issue2175-4.p4 in the p4c test suite would have to be rejected by the frontend.
Second, it adds the initializer block to the scope of abstract method.
When implementing an abstract method, the scope is limited to the supplied arguments or values that are in the top-level scope.
But this overlooks the case where an instantiation declaration is used inside an object initializer block. Instantiation declaration may be used inside an object initializer block, which is used by program virtual2.p4.
objDeclaration
: functionDeclaration
| instantiation
;
extern Virtual {
Virtual();
void run(in bit<16> ix);
@synchronous(run) abstract bit<16> f(in bit<16> ix);
}
extern State {
State(int<16> size);
bit<16> get(in bit<16> index);
}
...
Virtual() cntr = {
State(16s1024) state;
bit<16> f(in bit<16> ix) {
return state.get(ix);
}
};
Notice that f uses state in its implementation.
Strictly applying what is written in the spec, the program should not be accepted, since f refers to state that is neither in the top-level scope or the parameter list. i.e., the current spec syntactically allows instantiation inside object initializer but semantically disallows any way to refer to the instantiated instance.
So this PR adds the object initializer block to the scope as well.
I think it should follow roughly the same rules as overload resolution -- so if the functionDeclaration unambiguously matches one in the extern (ignoring argument names), that should be fine. If the argument names match exactly to resolve any ambiguity that should be fine too, but if there are multiple abstract functions with the same function name that differ only in argument names, and the names don't match, that should be an error.
I think it is a good idea. Just to clarify, below is my mental model of how it should work.
Overload resolution in P4 is based on argument arity and, if arguments are named, then we use the names to disambiguate. So the strategy would be:
(i) First lookup an abstract method with method name and arity. (ii) If (i) yields a single method, then matching is done. (iii) If (i) yields multiple methods, lookup again with method name, parameter names, and arity. (iv) If (iii) yields a single method, then matching is done. (v) If (iii) still yields multiple methods, we emit an error due to ambiguity.
extern Virtual() {
Virtual();
abstract bit<32> f(bit<32> a, bit<32> b); // (1)
abstract bit<32> f(bit<32> c, bit<32> d); // (2)
abstract bit<32> f(bit<32> a, bit<32> b, bit<32> c); // (3)
}
...
Virtual() cntr = {
bit<32> f(bit<32> c, bit<32> d) { // (a)
return 32w42;
}
bit<32> f(bit<32> d, bit<32> e, bit<32> f) { // (b)
return 32w42;
}
bit<32> f(bit<32> a, bit<32> b) { // (c)
return 32w42;
}
};
This would match (1) to (c), (2) to (a), and (3) to (b).
But at the same time, I also think that restricting the abstract method implementations to use the same parameter names as they were declared, helps maintain a consistent call interface.
Referring to the example above, after instantiating the object, we would have to call cntr.f(d = 32w1, e = 32w2, f = 32w3); (if using named arguments), which is different from what was proposed in the object declaration, cntr.f(a = 32w1, b = 32w2, c = 32w3);.
Hmm. I think you're probably right that having different argument names should at least be a warning, as it makes calls with named arguments confusing as, depending on context, you might need to use either set of names -- calling cntr directly would use the names as you've done, while passing cntr as a directionless argument to a control with a Virtual v argument would require a call like v.f(a = 1, b = 2, c = 3); in that control.
LDWG discussion:
For accessing abstract methods, the changes proposed to codify what is already in the P4C compiler and should go in.
For the other bit, abstract methods should not be overloaded in a way where the only distinguishing factor is parameter names. Programmers should be free to choose their own names for parameters in implementations of overloaded methods. This is at odds with parameter names being the only distinguishing feature between abstract overloads.
@jonathan-dilorenzo @rcgoodfellow This has been discussed, updated by Jaeyhun, and approved by two LDWG members. We think this is ready to merge, but would prefer if a co-chair could do so for final check.
@jonathan-dilorenzo Jaehyun believes this is ready to be merged. TAL and ideally merge.
@jonathan-dilorenzo Reminder to please take a look and merge if you approve.