validator: Disallow [[override]] on non-const vars
Only allow them on constants, where no other decoration is valid.
Change-Id: I83f19667adb1dd4ebbba86827324a45a8f1a80a4
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/50041
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: James Price <jrprice@google.com>
diff --git a/src/resolver/decoration_validation_test.cc b/src/resolver/decoration_validation_test.cc
index 1ff15c5..be3926e 100644
--- a/src/resolver/decoration_validation_test.cc
+++ b/src/resolver/decoration_validation_test.cc
@@ -34,9 +34,9 @@
kAlign,
kBinding,
kBuiltin,
- kConstantId,
kGroup,
kLocation,
+ kOverride,
kOffset,
kSize,
kStage,
@@ -63,12 +63,12 @@
return builder.create<ast::BindingDecoration>(source, 1);
case DecorationKind::kBuiltin:
return builder.Builtin(source, ast::Builtin::kPosition);
- case DecorationKind::kConstantId:
- return builder.create<ast::OverrideDecoration>(source, 0u);
case DecorationKind::kGroup:
return builder.create<ast::GroupDecoration>(source, 1u);
case DecorationKind::kLocation:
return builder.Location(source, 1);
+ case DecorationKind::kOverride:
+ return builder.create<ast::OverrideDecoration>(source, 0u);
case DecorationKind::kOffset:
return builder.create<ast::StructMemberOffsetDecoration>(source, 4u);
case DecorationKind::kSize:
@@ -108,9 +108,9 @@
TestParams{DecorationKind::kAlign, false},
TestParams{DecorationKind::kBinding, false},
TestParams{DecorationKind::kBuiltin, true},
- TestParams{DecorationKind::kConstantId, false},
TestParams{DecorationKind::kGroup, false},
TestParams{DecorationKind::kLocation, true},
+ TestParams{DecorationKind::kOverride, false},
TestParams{DecorationKind::kOffset, false},
TestParams{DecorationKind::kSize, false},
TestParams{DecorationKind::kStage, false},
@@ -150,9 +150,9 @@
TestParams{DecorationKind::kAlign, false},
TestParams{DecorationKind::kBinding, false},
TestParams{DecorationKind::kBuiltin, false},
- TestParams{DecorationKind::kConstantId, false},
TestParams{DecorationKind::kGroup, false},
TestParams{DecorationKind::kLocation, false},
+ TestParams{DecorationKind::kOverride, false},
TestParams{DecorationKind::kOffset, false},
TestParams{DecorationKind::kSize, false},
TestParams{DecorationKind::kStage, false},
@@ -184,9 +184,9 @@
TestParams{DecorationKind::kAlign, false},
TestParams{DecorationKind::kBinding, false},
TestParams{DecorationKind::kBuiltin, false},
- TestParams{DecorationKind::kConstantId, false},
TestParams{DecorationKind::kGroup, false},
TestParams{DecorationKind::kLocation, false},
+ TestParams{DecorationKind::kOverride, false},
TestParams{DecorationKind::kOffset, false},
TestParams{DecorationKind::kSize, false},
TestParams{DecorationKind::kStage, false},
@@ -222,9 +222,9 @@
TestParams{DecorationKind::kAlign, true},
TestParams{DecorationKind::kBinding, false},
TestParams{DecorationKind::kBuiltin, true},
- TestParams{DecorationKind::kConstantId, false},
TestParams{DecorationKind::kGroup, false},
TestParams{DecorationKind::kLocation, true},
+ TestParams{DecorationKind::kOverride, false},
TestParams{DecorationKind::kOffset, true},
TestParams{DecorationKind::kSize, true},
TestParams{DecorationKind::kStage, false},
@@ -257,9 +257,44 @@
TestParams{DecorationKind::kAlign, false},
TestParams{DecorationKind::kBinding, true},
TestParams{DecorationKind::kBuiltin, true},
- TestParams{DecorationKind::kConstantId, true},
TestParams{DecorationKind::kGroup, true},
TestParams{DecorationKind::kLocation, true},
+ TestParams{DecorationKind::kOverride, false},
+ TestParams{DecorationKind::kOffset, false},
+ TestParams{DecorationKind::kSize, false},
+ TestParams{DecorationKind::kStage, false},
+ TestParams{DecorationKind::kStride, false},
+ TestParams{DecorationKind::kStructBlock, false},
+ TestParams{DecorationKind::kWorkgroup, false}));
+
+using ConstantDecorationTest = TestWithParams;
+TEST_P(ConstantDecorationTest, IsValid) {
+ auto& params = GetParam();
+
+ GlobalConst("a", ty.f32(), nullptr,
+ ast::DecorationList{
+ createDecoration(Source{{12, 34}}, *this, params.kind)});
+
+ WrapInFunction();
+
+ if (params.should_pass) {
+ EXPECT_TRUE(r()->Resolve()) << r()->error();
+ } else {
+ EXPECT_FALSE(r()->Resolve()) << r()->error();
+ EXPECT_EQ(r()->error(),
+ "12:34 error: decoration is not valid for constants");
+ }
+}
+INSTANTIATE_TEST_SUITE_P(
+ ResolverDecorationValidationTest,
+ ConstantDecorationTest,
+ testing::Values(TestParams{DecorationKind::kAccess, false},
+ TestParams{DecorationKind::kAlign, false},
+ TestParams{DecorationKind::kBinding, false},
+ TestParams{DecorationKind::kBuiltin, false},
+ TestParams{DecorationKind::kGroup, false},
+ TestParams{DecorationKind::kLocation, false},
+ TestParams{DecorationKind::kOverride, true},
TestParams{DecorationKind::kOffset, false},
TestParams{DecorationKind::kSize, false},
TestParams{DecorationKind::kStage, false},
@@ -291,9 +326,9 @@
TestParams{DecorationKind::kAlign, false},
TestParams{DecorationKind::kBinding, false},
TestParams{DecorationKind::kBuiltin, false},
- TestParams{DecorationKind::kConstantId, false},
TestParams{DecorationKind::kGroup, false},
TestParams{DecorationKind::kLocation, false},
+ TestParams{DecorationKind::kOverride, false},
TestParams{DecorationKind::kOffset, false},
TestParams{DecorationKind::kSize, false},
// Skip kStage as we always apply it in this test
diff --git a/src/resolver/resolver.cc b/src/resolver/resolver.cc
index 603f3dd..8d3f35f 100644
--- a/src/resolver/resolver.cc
+++ b/src/resolver/resolver.cc
@@ -501,11 +501,16 @@
for (auto* deco : var->decorations()) {
Mark(deco);
- if (!(deco->Is<ast::BindingDecoration>() ||
- deco->Is<ast::BuiltinDecoration>() ||
- deco->Is<ast::OverrideDecoration>() ||
- deco->Is<ast::GroupDecoration>() ||
- deco->Is<ast::LocationDecoration>())) {
+ if (var->is_const()) {
+ if (!deco->Is<ast::OverrideDecoration>()) {
+ diagnostics_.add_error("decoration is not valid for constants",
+ deco->source());
+ return false;
+ }
+ } else if (!(deco->Is<ast::BindingDecoration>() ||
+ deco->Is<ast::BuiltinDecoration>() ||
+ deco->Is<ast::GroupDecoration>() ||
+ deco->Is<ast::LocationDecoration>())) {
diagnostics_.add_error("decoration is not valid for variables",
deco->source());
return false;
