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.

Sign up for GitHub

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

Jump to bottom

Compiler Bug: At this point in the compilation typechecking should not infer new types anymore #4288

Open
p-sawicki opened this issue Dec 13, 2023 · 4 comments
Open

Compiler Bug: At this point in the compilation typechecking should not infer new types anymore #4288

p-sawicki opened this issue Dec 13, 2023 · 4 comments
Labels
bug This behavior is unintended and should be fixed. fixed This topic is considered to be fixed.

Comments

@p-sawicki
Copy link
Contributor

The program

#include <core.p4>

control Deparser(packet_out packet);
package SimpleArch(Deparser dep);

header hdr_t {
   bit<8> f0;
}

control MyDeparser(packet_out packet) (hdr_t h) {
    apply {
      packet.emit(h);
    }
}

SimpleArch(MyDeparser({0})) main;

crashes in frontend with the message

Compiler bug triggered, please report this error: 
exception type: Util::CompilerBug
error: In file: ./p4c/frontends/p4/typeChecking/typeChecker.cpp:172
Compiler Bug: main.p4(12): At this point in the compilation typechecking should not infer new types anymore, but it did: node packet.emit<hdr_t>({0}); changed to packet.emit<hdr_t>({
  f0:0; });
      packet.emit(h);
      ^^^^^^^^^^^^^^
main.p4(12)
      packet.emit(h);
      ^^^^^^^^^^^^^^
@jafingerhut
Copy link
Contributor

I agree that improving the compiler to no longer experience a CompilerBug for such a program would be nice.

I only wanted to mention that probably one of the reasons that this has not been reported before is that it is extremely common for a deparser to take a parameter that is a struct type containing the header variables to be emitted, whereas this program has a deparser that takes exactly one header-type parameter to be emitted. Thus it seems that while enhancing the compiler to avoid this CompilerBug is a nice-to-have, it might not be critical for actual P4 implementations to have.

@mihaibudiu
Copy link
Contributor

The typechecker makes no assumptions on the number of controls or their arguments, this must be something else.
It's probably related to the constructor argument which is instantiated with a header, there aren't many tests with constructor arguments.

@mihaibudiu
Copy link
Contributor

Yes, in fact the error message says that the error is that type inference changes the constructor argument to {f0: 0}. That is right, but it probably happens too late.

@mihaibudiu
Copy link
Contributor

Sorry, not constructor argument, but the function argument.

@mihaibudiu mihaibudiu mentioned this issue Dec 13, 2023
Merged
@fruffy fruffy added bug This behavior is unintended and should be fixed. fixed This topic is considered to be fixed. labels Dec 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug This behavior is unintended and should be fixed. fixed This topic is considered to be fixed.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@jafingerhut @fruffy @mihaibudiu @p-sawicki