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

Wrong scope when initializing an abstract method #5084

Open
jaehyun1ee opened this issue Jan 6, 2025 · 3 comments
Open

Wrong scope when initializing an abstract method #5084

jaehyun1ee opened this issue Jan 6, 2025 · 3 comments
Labels
p4-spec Topics related to the P4 specification (https://github.com/p4lang/p4-spec/).

Comments

@jaehyun1ee
Copy link

In section 11.3.1. Instantiating objects with abstract methods, the spec mentions that:

The abstract methods can only use the supplied arguments or refer to values that are in the top-level scope.
When calling another method of the same instance the this keyword is used to indicate the current object instance.

However, tests issue2273-1.p4 and virtual3.p4 in testdata/p4_16_samples seem to violate this. The frontend accepts abstract method implementations referring to control-local variables or control parameters, when they should be rejected as per the spec.

// issue2273-1.p4
control ingress(inout headers hdr) {
    Stack<bit<16>>(2048) stack;
    StackAction<bit<16>, bit<16>>(stack) write = {
        void apply(inout bit<16> value) {
            value = hdr.data.h1; // invalid access to hdr
        }
        void overflow(inout bit<16> value, out bit<16> rv) {
            rv = 0x0f0f;
        }
    };
    // ...
}
// virtual3.p4
control c(inout bit<16> p) {
    bit<16>     local;

    Virtual() cntr = {
        bit<16> f(in bit<16> ix) {
            return (ix + local); // invalid access to local
        }
    };
    // ...
}
@ChrisDodd
Copy link
Contributor

ChrisDodd commented Jan 6, 2025

This would seem to be a typo in the spec -- it should be "in the enclosing scope" or "in the scope in which the extern instance declaration appears"

@jaehyun1ee
Copy link
Author

FYI, there is a related p4-spec issue that has undergone ldwg discussion. It proposes to define the scope as, (i) the top level, (ii) supplied arguments to the method, and (iii) the object initializer block.

@fruffy fruffy added the p4-spec Topics related to the P4 specification (https://github.com/p4lang/p4-spec/). label Jan 6, 2025
@vlstill
Copy link
Contributor

vlstill commented Jan 6, 2025

This would seem to be a typo in the spec -- it should be "in the enclosing scope" or "in the scope in which the extern instance declaration appears"

Note that if that was allowed, it would allow runtime values that are not arguments to come into an abstract methods (as far as I can see, the initializers added in p4lang/p4-spec#1355 can't be used to supply runtime values). That is a significant extension of the behavior allowed by the spec now.

That said, if I recall the Tofino's RegisterAction's use of abstract methods, I expect that they would make use of the ability to capture runtime values from the enclosing control block.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p4-spec Topics related to the P4 specification (https://github.com/p4lang/p4-spec/).
Projects
None yet
Development

No branches or pull requests

4 participants