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

Matching control types that have default parameters #1348

Open
jaehyun1ee opened this issue Nov 25, 2024 · 2 comments
Open

Matching control types that have default parameters #1348

jaehyun1ee opened this issue Nov 25, 2024 · 2 comments

Comments

@jaehyun1ee
Copy link
Contributor

P4 allows default control parameters.

The below program default-control-argument.p4 from the p4c test suite expects that control declaration MyC matches the control type signature C<H, M>. (The compiler frontend treats this program as valid.)

control C<H, M>(
    inout H hdr,
    inout M meta,
    in intrinsic_metadata_t intr_md = {0, 0});

package P<H, M>(C<H, M> c);

struct hdr_t { }
struct meta_t { }

control MyC(inout hdr_t hdr, inout meta_t meta) {
   apply {}
}

P(MyC()) main;

However, I wonder if the above program should actually be accepted as valid, as per the spec. Default parameters can be omitted when invoking the control instance, but I believe they cannot be omitted when declaring a control object.

The above code is also misleading because,

  • The control type declaration control<H, M> implies that we can explicitly supply an argument corresponding to the intr_md parameter.
  • However, if we were to supply a third argument for MyC, it would result in an error since the control declaration MyC expects only two.

A valid program would write something like:

control MyC(inout hdr_t hdr, inout meta_t meta, in instrinsic_metadata_t intr_md = { 0, 0 }) {
   apply {}
}
@vlstill
Copy link
Contributor

vlstill commented Jan 14, 2025

I agree that allowing MyC to match C leads to very weird behavior.

Is it possible to pass a control to another control (using control declaration name in the parameter list of the outer control -- e.g. control MyC0(inout hdr_t hdr, inout meta_t meta, C<hdr_t, meta_t> sub))? That is the only place where I can imagine MyC being invoked through C outside of package (where it is not really invoked as package is an architecture-defining construct).

Regarding the default arguments in definition, I wonder if we should take the C++ approach here, saying that the definition must not define the default arguments if there is a declaration/signature. The downside is that there can also be controls which aren't intended to match any signature at all because they are directly invoked from other control, and in these cases especially we might want to include default arguments (but they can actually match a signature, even if that was not indented, because the matching is signature based).

On the other hand, if we allow both definition and signature to define the default arguments, what would we do if they are different? It should not mean that they don't match in my opinion, because matching happens on types and default values should not, in my opinion, be part of the type.

@vlstill
Copy link
Contributor

vlstill commented Jan 14, 2025

The problem here is that the signature is essentially an interface (in the sense it is used in OOP), not a declaration. Surprisingly, for example in C#, it is possible to define different default value in an interface and in the implementation and the used one depends on the static type:

https://godbolt.org/z/vGYnG4crM

using System;

interface Foo {
    abstract int foo(int num = 0);
}

class Bar : Foo {
    public int foo(int num = 4) { return num; }
}

class Program
{
    static void Main() {
        Foo x = new Bar();
        Bar y = new Bar();
        Console.WriteLine(x.foo()); // 0
        Console.WriteLine(y.foo()); // 4
    }
}

It is even possible to omit the default in the derived, in which case the call through derived has to have the argument:

https://godbolt.org/z/33GdGbP6o

using System;

interface Foo {
    abstract int foo(int num = 0);
}

class Bar : Foo {
    public int foo(int num) { return num; }
}

class Program
{
    static void Main() {
        Foo x = new Bar();
        Bar y = new Bar();
        Console.WriteLine(x.foo()); // 0
        // Console.WriteLine(y.foo()); // 4
    }
}

But omitting the arguments leads to an error:

https://godbolt.org/z/9saPEfhPo

using System;

interface Foo {
    abstract int foo(int num = 0);
}

class Bar : Foo { // <source>(7,13): error CS0535: 'Bar' does not implement interface member 'Foo.foo(int)'
    public int foo() { return num; }
}

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

No branches or pull requests

2 participants