[tint] Always evaluate template arguments
When short-circuiting an expression, do not skip evaluation of any
template arguments inside it. This means we always evaluate and
validate array element counts within short-circuited expressions.
Change-Id: Ic38cd7cc3816b700c1f4914f1e64159a70a37521
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/181961
Auto-Submit: James Price <jrprice@google.com>
Commit-Queue: James Price <jrprice@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
diff --git a/src/tint/lang/core/constant/eval_binary_op_test.cc b/src/tint/lang/core/constant/eval_binary_op_test.cc
index ef9f163..8a1bb14 100644
--- a/src/tint/lang/core/constant/eval_binary_op_test.cc
+++ b/src/tint/lang/core/constant/eval_binary_op_test.cc
@@ -2541,6 +2541,66 @@
}
////////////////////////////////////////////////
+// Short-Circuit templated identifier arguments
+////////////////////////////////////////////////
+
+TEST_F(ConstEvalTest, ShortCircuit_And_ArrayElementCountTooSmall) {
+ // const one = 1;
+ // const result = (one == 0) && array<bool, 3-4>()[0];
+ GlobalConst("one", Expr(1_a));
+ auto* lhs = Equal("one", 0_a);
+ auto* count = Sub(3_a, 4_a);
+ auto* rhs = IndexAccessor(Call(ty.array(ty.bool_(), count)), 0_a);
+ auto* binary = LogicalAnd(lhs, rhs);
+ GlobalConst("result", binary);
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(r()->error(), "error: array count (-1) must be greater than 0");
+}
+
+TEST_F(ConstEvalTest, ShortCircuit_Or_ArrayElementCountTooSmall) {
+ // const one = 1;
+ // const result = (one == 1) || array<bool, 3-4>()[0];
+ GlobalConst("one", Expr(1_a));
+ auto* lhs = Equal("one", 1_a);
+ auto* count = Sub(3_a, 4_a);
+ auto* rhs = IndexAccessor(Call(ty.array(ty.bool_(), count)), 0_a);
+ auto* binary = LogicalOr(lhs, rhs);
+ GlobalConst("result", binary);
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(r()->error(), "error: array count (-1) must be greater than 0");
+}
+
+TEST_F(ConstEvalTest, ShortCircuit_And_InvalidArrayElementCount) {
+ // const one = 1;
+ // const result = (one == 0) && array<bool, u32(sqrt(-1))>()[0];
+ GlobalConst("one", Expr(1_a));
+ auto* lhs = Equal("one", 0_a);
+ auto* count = Call("u32", Call("sqrt", -1_a));
+ auto* rhs = IndexAccessor(Call(ty.array(ty.bool_(), count)), 0_a);
+ auto* binary = LogicalAnd(lhs, rhs);
+ GlobalConst("result", binary);
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(r()->error(), "error: sqrt must be called with a value >= 0");
+}
+
+TEST_F(ConstEvalTest, ShortCircuit_Or_InvalidArrayElementCount) {
+ // const one = 1;
+ // const result = (one == 1) || array<bool, u32(sqrt(-1))>()[0];
+ GlobalConst("one", Expr(1_a));
+ auto* lhs = Equal("one", 1_a);
+ auto* count = Call("u32", Call("sqrt", -1_a));
+ auto* rhs = IndexAccessor(Call(ty.array(ty.bool_(), count)), 0_a);
+ auto* binary = LogicalOr(lhs, rhs);
+ GlobalConst("result", binary);
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(r()->error(), "error: sqrt must be called with a value >= 0");
+}
+
+////////////////////////////////////////////////
// Short-Circuit Nested
////////////////////////////////////////////////
diff --git a/src/tint/lang/wgsl/resolver/resolver.cc b/src/tint/lang/wgsl/resolver/resolver.cc
index 8e4b9b3..91258cb 100644
--- a/src/tint/lang/wgsl/resolver/resolver.cc
+++ b/src/tint/lang/wgsl/resolver/resolver.cc
@@ -1592,6 +1592,13 @@
auto r = ast::TraverseExpressions( //
(*binary)->rhs, [&](const ast::Expression* e) {
not_evaluated_.Add(e);
+ if (e->Is<ast::IdentifierExpression>()) {
+ // Template arguments are still evaluated when the outer identifier
+ // expression is skipped. This happens in expressions like:
+ // false && array<T, N>()[i]
+ // where we still need to evaluate and validate `N`.
+ return ast::TraverseAction::Skip;
+ }
return ast::TraverseAction::Descend;
});
if (!r) {
@@ -4171,10 +4178,7 @@
switch (count_sem->Stage()) {
case core::EvaluationStage::kNotEvaluated:
- // Happens in expressions like:
- // false && array<T, N>()[i]
- // The end result will not be used, so just make N=1.
- return b.create<core::type::ConstantArrayCount>(static_cast<uint32_t>(1));
+ ICE(count_expr->source) << "array element count was not evaluated";
case core::EvaluationStage::kOverride: {
// array count is an override expression.
