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

Emit a reasonable error if someone uses type in place where expression is exprected #4411

Merged
merged 3 commits into from
Feb 13, 2024
Merged

Emit a reasonable error if someone uses type in place where expression is exprected #4411

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7eecdbc
Emit a reasonable error if someone uses type in place where expressio…
vlstill Feb 12, 2024
ef57382
Add test
vlstill Feb 12, 2024
5fadbaa
Add two related tests, add comments with PR link to the tests
vlstill Feb 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions frontends/p4/typeChecking/typeChecker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3052,6 +3052,9 @@ const IR::Node *TypeInference::postorder(IR::PathExpression *expression) {
if (type != nullptr)
// may be nullptr because typechecking may have failed
type = cloneWithFreshTypeVariables(type->to<IR::Type_MethodBase>());
} else if (decl->is<IR::Type_Declaration>()) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am wondering whether this might not be too restrictive. On the other hand a PathExpression reference to a Type_Declaration should probably parsed as Type_Name

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly my thought. And a PathExpression is an expression, so it can be used to build more complex expressions. Having a type there directly does not make much sense.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any name that is in-scope as a type will be lexed as a TYPE_IDENTIFIER and recognized as a typeName in the parser, so should create a Type_Name rather than a Path. The exception is when the type is used before its declaration, which (I think) can only happen with type parameters.

typeError("%1%: Type cannot be used here, expecting an expression.", expression);
return expression;
}

if (type == nullptr) {
Expand Down
9 changes: 9 additions & 0 deletions testdata/p4_16_errors/type-in-expr-lex0.p4
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// tests problems related to https://github.com/p4lang/p4c/pull/4411

#include <core.p4>

control ctrl<H>(bool val);

package pkg<H>(
ctrl<H> val = ctrl(H > 3)
);
10 changes: 10 additions & 0 deletions testdata/p4_16_errors/type-in-expr-lex1.p4
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// tests problems related to https://github.com/p4lang/p4c/pull/4411

#include <core.p4>

parser pars<H>(in bit<16> buf, out H hdrs) {
bool var;
state start {
var = buf > H;
}
}
10 changes: 10 additions & 0 deletions testdata/p4_16_errors/type-in-expr.p4
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// tests problem solved by https://github.com/p4lang/p4c/pull/4411

#include <core.p4>

@foo[bar=4<H]
control p<H>(in H hdrs, out bool flag)
{
apply {
}
}
4 changes: 4 additions & 0 deletions testdata/p4_16_errors_outputs/type-in-expr-lex0.p4-stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
type-in-expr-lex0.p4(8):syntax error, unexpected >, expecting (
ctrl<H> val = ctrl(H >
^
[--Werror=overlimit] error: 1 errors encountered, aborting compilation
4 changes: 4 additions & 0 deletions testdata/p4_16_errors_outputs/type-in-expr-lex1.p4-stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
type-in-expr-lex1.p4(8):syntax error, unexpected ;, expecting (
var = buf > H;
^
[--Werror=overlimit] error: 1 errors encountered, aborting compilation
7 changes: 7 additions & 0 deletions testdata/p4_16_errors_outputs/type-in-expr.p4
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#include <core.p4>

@foo[bar=4 < H] control p<H>(in H hdrs, out bool flag) {
apply {
}
}

12 changes: 12 additions & 0 deletions testdata/p4_16_errors_outputs/type-in-expr.p4-stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
type-in-expr.p4(5): [--Werror=type-error] error: H: Type cannot be used here, expecting an expression.
@foo[bar=4<H]
^
type-in-expr.p4(5): [--Werror=type-error] error: 4 < H: structured annotation must be compile-time constant values
@foo[bar=4<H]
^^^
type-in-expr.p4(5): [--Werror=type-error] error: H: Type cannot be used here, expecting an expression.
@foo[bar=4<H]
^
type-in-expr.p4(6): [--Werror=type-error] error: Error while analyzing control p
control p<H>(in H hdrs, out bool flag)
^
Loading