Make P4-Constraints Typechecker Idempotent#167
Make P4-Constraints Typechecker Idempotent#167Kaiwen-Guo wants to merge 5 commits intop4lang:masterfrom
Conversation
…4lang#165) Signed-off-by: harith-hacky03 <harithhacky3@gmail.com> Signed-off-by: kg3354 <kg3354@nyu.edu>
…cker idempotent Signed-off-by: kg3354 <kg3354@nyu.edu>
Signed-off-by: kg3354 <kg3354@nyu.edu>
Signed-off-by: James Hu <jhudson15869@gmail.com> Signed-off-by: kg3354 <kg3354@nyu.edu>
Signed-off-by: Steffen Smolka <steffen.smolka@gmail.com> Signed-off-by: kg3354 <kg3354@nyu.edu>
5a5e96c to
1774fdb
Compare
|
Fixed problems with Signed-off-by and formatting |
| left.type_case() == Type::kTernary || | ||
| left.type_case() == Type::kLpm || left.type_case() == Type::kRange || | ||
| left.type_case() == Type::kOptionalMatch; | ||
| } |
There was a problem hiding this comment.
I might be misunderstanding this, but in the case of casting bit<W> (kFixedUnsigned) to exact<W> | ternary<W> | lpm<W> | range<W> | optional<W>, do we need to check that the bit widths match?
e.g. in StrictlyAboveInCastabilityOrder:
switch (left.type_case()) {
case Type::kExact:
case Type::kTernary:
case Type::kLpm:
case Type::kRange:
case Type::kOptionalMatch:
switch (right.type_case()) {
case Type::kFixedUnsigned:
return TypeBitwidth(left) == TypeBitwidth(right); <------ bit width check
case Type::kArbitraryInt:
return true;
default:
return false;
}
There was a problem hiding this comment.
Yeah that's a great point. I now added additionalTypeBitwidth checks during case 2 return
There was a problem hiding this comment.
IIUC, changing InferAndCheckTypes() to be idempotent should cause TypeCastNeverTypeChecks to fail because we are no longer erroring immediately upon encountering a type cast. Based on a quick look it seems to me that the test case Expression expr = ParseTextProtoOrDie<Expression>(R"pb(type_cast { integer_constant: "0" })pb"); is not a valid type cast expression since left is set to TYPE_NOT_SET and right is set to kArbitraryInt when OneLevelAboveInCastabilityOrder() is called, which instantly returns false because TYPE_NOT_SET isn't accounted for in the castability order.
i.e. I think that the test is currently passing because the function returns kInvalidArgument for the wrong reason.
I got the test to fail (In this case return Ok() when an error was expected) using the test case below - let me know if you can reproduce
TEST_F(InferAndCheckTypesTest, TypeCastNeverTypeChecks) {
Expression expr = ParseTextProtoOrDie<Expression>(R"pb(
binary_expression {
binop: EQ
left {
type { exact { bitwidth: 32 } }
key: "exact32"
}
right {
type { exact { bitwidth: 32 } }
type_cast {
type { fixed_unsigned { bitwidth: 32 } }
type_cast {
type { arbitrary_int {} }
integer_constant: "48"
}
}
}
}
)pb");
AddMockSourceLocations(expr);
ASSERT_THAT(InferAndCheckTypes(&expr, kTableInfo),
StatusIs(StatusCode::kInvalidArgument));
}
There was a problem hiding this comment.
This really depends on whether nested type casts should be allowed. If it should not be allowed, then we can add an additional check after getting the inner expression to raise any corresponding errors:
if (inner_expr->expression_case() == ast::Expression::kTypeCast) {
return StaticTypeError(constraint_source, expr->start_location(),
expr->end_location())
<< "nested type casts are not allowed";
}
Make P4-Constraints Typechecker Idempotent
Description
This PR addresses issue #140 by making the P4-Constraints typechecker idempotent, allowing it to be called multiple times on the same expression without failing.
Changes
OneLevelAboveInCastabilityOrderfunction to enforce that type casts only move a single level up in the Hasse diagramkTypeCastcase inInferAndCheckTypesto properly validate type castsHow to Test
IdempotentTypeCastHandlingthat verifies the typechecker can be called multiple times on the same expression