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

Using enum as a type generates syntax error. #492

Closed
hanw opened this issue Apr 17, 2017 · 17 comments
Closed

Using enum as a type generates syntax error. #492

hanw opened this issue Apr 17, 2017 · 17 comments
Labels
wontfix This is a problem that will not be fixed.

Comments

@hanw
Copy link
Contributor

hanw commented Apr 17, 2017

 enum HashAlgo {
    CRC32
}
extern Hash<Algo, O> {
    Hash();
    O getHash<T, D, M>(in T base, in D data, in M max);
}
control ingress<H, M> (inout H hdr, inout M meta);
package PSA<H, M> (ingress<H,M> ig);
// user program
struct ParsedHeaders {
    bit<32> hdr;
}
struct Metadata {
    bit<32> md;
}
control MyIngress (inout ParsedHeaders p, inout Metadata m) (Hash<HashAlgo.CRC32, bit<32>> h) {
    action compute_hash() {
        bit<32> b = p.hdr;
        tuple<bit<32>, bit<32>> d = {p.hdr, m.md};
        bit<32> max = m.md;
        bit<32> hash;
        hash = h.getHash(b, d, max);
    }
    apply{
        compute_hash();
    }
}
Hash<HashAlgo.CRC32, bit<32>> hash;
MyIngress(hash) ig;
PSA(ig) main;

Issue:
action_selector does not accept an enum as the type for instantiation.

Error message:

../test/gtest/arch_test.cpp(559):syntax error, unexpected '.', expecting ',' or '>'
        control MyIngress (inout ParsedHeaders p, inout Metadata m) (Hash<HashAlgo.
@mihaibudiu
Copy link
Contributor

You see what a terrible error message this is from gtest, it does not really tell you where the error is.

I cannot reproduce this error, I am getting the following:

issue492.p4(30): warning: Table test1 does not have an `default_action' property
    table test1 {
          ^^^^^
issue492.p4(35): warning: selector shadows selector
        selector = action_selector(HashAlgorithm.crc16, 32w1024, 32w10);
        ^^^^^^^^
issue492.p4(12)
    selector
    ^^^^^^^^

@sethfowler
Copy link
Contributor

What do you mean "from GTest"? That's just p4c's normal error reporting. The line that's being reported is correct.

@mihaibudiu
Copy link
Contributor

../test/gtest/arch_test.cpp(559):syntax error, unexpected '.', expecting ',' or '>'

@sethfowler
Copy link
Contributor

Yes, that is correct. That error message points to the line where the error is in the P4 program, which is also a line in a .cpp file since the P4 program is defined inline in the test.

@mihaibudiu
Copy link
Contributor

Yes, but I don't have that C++ program, since the code is not checked-in.
How about the error message I posted which happens if you don't use gtest?

@jnfoster
Copy link
Contributor

Mihai, I am on 482a3bd.

I see this error message for Han's original code snippet, which is like yours but also dumps core:

han.p4(30): warning: Table test1 does not have an `default_action' property
    table test1 {
          ^^^^^
han.p4(35): warning: selector shadows selector
        selector = action_selector(HashAlgorithm.crc16, 32w1024, 32w10);
        ^^^^^^^^
han.p4(12)
    selector
    ^^^^^^^^
han.p4(42): error: PackageBlock: Expected 6 arguments for main package
PSA(ig) main;
        ^^^^
terminate called after throwing an instance of 'Util::CompilerBug'
  what():  COMPILER BUG: ../ir/ir.cpp:172
No parameter named eg

Aborted (core dumped)

With the newer code since he updated the issue, I see the kind of error he first reported:

han.p4(17):syntax error, unexpected '.', expecting ',' or '>'
control MyIngress (inout ParsedHeaders p, inout Metadata m) (Hash<HashAlgo.
                                                                          ^
error: 1 errors encountered, aborting compilation

@mihaibudiu
Copy link
Contributor

I don't understand where this error is coming from. Look at the code, there is no line like the one quoted in the sources. There is no identifier HashAlgo.

@jnfoster
Copy link
Contributor

He updated the code to

 enum HashAlgo {
    CRC32
}
extern Hash<Algo, O> {
    Hash();
    O getHash<T, D, M>(in T base, in D data, in M max);
}
control ingress<H, M> (inout H hdr, inout M meta);
package PSA<H, M> (ingress<H,M> ig);
// user program
struct ParsedHeaders {
    bit<32> hdr;
}
struct Metadata {
    bit<32> md;
}
control MyIngress (inout ParsedHeaders p, inout Metadata m) (Hash<HashAlgo.CRC32, bit<32>> h) {
    action compute_hash() {
        bit<32> b = p.hdr;
        tuple<bit<32>, bit<32>> d = {p.hdr, m.md};
        bit<32> max = m.md;
        bit<32> hash;
        hash = h.getHash(b, d, max);
    }
    apply{
        compute_hash();
    }
}
Hash<HashAlgo.CRC32, bit<32>> hash;
MyIngress(hash) ig;
PSA(ig) main;

@mihaibudiu
Copy link
Contributor

@jnfoster: the compiler is dumping core because you can't compile this with p4c-bm2-ss; this is not a program for v1model.
I will file a separate bug for that.

@hanw
Copy link
Contributor Author

hanw commented Apr 17, 2017

I pasted the wrong test case in the original email. Thanks Nate for pointing that out.

@mihaibudiu
Copy link
Contributor

This example is not syntactically correct; you can only use types as type arguments when specializing a type. However, HashAlgo.CRC32 is not a type, it is a constant.

@cc10512
Copy link
Contributor

cc10512 commented Apr 17, 2017

However, @hanw's example is using HashAlgo as the the type, and HashAlgo.CRC32 as a constant parameter. That doesn't work either.

@mihaibudiu
Copy link
Contributor

The example posted won't work in Java or C# either; the generics in P4 are modeled after those languages.
A constant can be used perhaps as a constructor parameter, but not as a template parameter.
Maybe there is another example I have not seen?

@cc10512
Copy link
Contributor

cc10512 commented Apr 17, 2017

Here is the source for all the examples that @hanw is trying to get through the compiler: https://github.com/cc10512/p4c/blob/cc/psa/p4include/psa.p4

@mihaibudiu
Copy link
Contributor

There are generic externs in v1model.p4. In particular, register is a working example.
If you want a constant value, pass it as a constructor parameter.
In our language both templates and constructor arguments are evaluated at compile-time, so the expressivity is the same.

@jnfoster
Copy link
Contributor

Can we safely close this?

@hanw
Copy link
Contributor Author

hanw commented Apr 19, 2017

Closed by passing constant value as a constructor parameter.

@hanw hanw closed this as completed Apr 19, 2017
@fruffy fruffy added wontfix This is a problem that will not be fixed. and removed invalid labels Jan 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This is a problem that will not be fixed.
Projects
None yet
Development

No branches or pull requests

6 participants