validation: Require [[location]] for [[interpolate]]
The spec now has this requirement:
https://github.com/gpuweb/gpuweb/pull/2251
Fixed: tint:982
Change-Id: I5ac7fdba336116bbcd0d805ba8b52ddab505df55
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/68921
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
diff --git a/src/resolver/decoration_validation_test.cc b/src/resolver/decoration_validation_test.cc
index d4641a5..1f41c7a 100644
--- a/src/resolver/decoration_validation_test.cc
+++ b/src/resolver/decoration_validation_test.cc
@@ -276,7 +276,7 @@
TestParams{DecorationKind::kBinding, false},
TestParams{DecorationKind::kBuiltin, true},
TestParams{DecorationKind::kGroup, false},
- TestParams{DecorationKind::kInterpolate, true},
+ // kInterpolate tested separately (requires [[location]])
TestParams{DecorationKind::kInvariant, true},
TestParams{DecorationKind::kLocation, true},
TestParams{DecorationKind::kOverride, false},
@@ -470,7 +470,7 @@
TestParams{DecorationKind::kBinding, false},
TestParams{DecorationKind::kBuiltin, true},
TestParams{DecorationKind::kGroup, false},
- TestParams{DecorationKind::kInterpolate, true},
+ // kInterpolate tested separately (requires [[location]])
TestParams{DecorationKind::kInvariant, true},
TestParams{DecorationKind::kLocation, false},
TestParams{DecorationKind::kOverride, false},
@@ -618,7 +618,7 @@
TestParams{DecorationKind::kBinding, false},
TestParams{DecorationKind::kBuiltin, true},
TestParams{DecorationKind::kGroup, false},
- TestParams{DecorationKind::kInterpolate, true},
+ // kInterpolate tested separately (requires [[location]])
// kInvariant tested separately (requires position builtin)
TestParams{DecorationKind::kLocation, true},
TestParams{DecorationKind::kOverride, false},
@@ -1392,6 +1392,47 @@
"flat interpolation attribute");
}
+TEST_F(InterpolateTest, MissingLocationAttribute_Parameter) {
+ Func("main",
+ ast::VariableList{
+ Param("a", ty.vec4<f32>(),
+ {Builtin(ast::Builtin::kPosition),
+ Interpolate(Source{{12, 34}}, ast::InterpolationType::kFlat,
+ ast::InterpolationSampling::kNone)})},
+ ty.void_(), {},
+ ast::DecorationList{Stage(ast::PipelineStage::kFragment)});
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(r()->error(),
+ "12:34 error: interpolate attribute must only be used with "
+ "[[location]]");
+}
+
+TEST_F(InterpolateTest, MissingLocationAttribute_ReturnType) {
+ Func("main", {}, ty.vec4<f32>(), {Return(Construct(ty.vec4<f32>()))},
+ ast::DecorationList{Stage(ast::PipelineStage::kVertex)},
+ {Builtin(ast::Builtin::kPosition),
+ Interpolate(Source{{12, 34}}, ast::InterpolationType::kFlat,
+ ast::InterpolationSampling::kNone)});
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(r()->error(),
+ "12:34 error: interpolate attribute must only be used with "
+ "[[location]]");
+}
+
+TEST_F(InterpolateTest, MissingLocationAttribute_Struct) {
+ Structure(
+ "S", {Member("a", ty.f32(),
+ {Interpolate(Source{{12, 34}}, ast::InterpolationType::kFlat,
+ ast::InterpolationSampling::kNone)})});
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(r()->error(),
+ "12:34 error: interpolate attribute must only be used with "
+ "[[location]]");
+}
+
} // namespace
} // namespace InterpolateTests
diff --git a/src/resolver/resolver_validation.cc b/src/resolver/resolver_validation.cc
index cfe76bd..a9f4013 100644
--- a/src/resolver/resolver_validation.cc
+++ b/src/resolver/resolver_validation.cc
@@ -1158,6 +1158,15 @@
}
}
+ if (interpolate_attribute) {
+ if (!pipeline_io_attribute ||
+ !pipeline_io_attribute->Is<ast::LocationDecoration>()) {
+ AddError("interpolate attribute must only be used with [[location]]",
+ interpolate_attribute->source);
+ return false;
+ }
+ }
+
if (invariant_attribute) {
bool has_position = false;
if (pipeline_io_attribute) {
@@ -1927,8 +1936,10 @@
}
}
+ auto has_location = false;
auto has_position = false;
const ast::InvariantDecoration* invariant_attribute = nullptr;
+ const ast::InterpolateDecoration* interpolate_attribute = nullptr;
for (auto* deco : member->Declaration()->decorations) {
if (!deco->IsAnyOf<ast::BuiltinDecoration, //
ast::InternalDecoration, //
@@ -1951,6 +1962,7 @@
if (auto* invariant = deco->As<ast::InvariantDecoration>()) {
invariant_attribute = invariant;
} else if (auto* location = deco->As<ast::LocationDecoration>()) {
+ has_location = true;
if (!ValidateLocationDecoration(location, member->Type(), locations,
member->Declaration()->source)) {
return false;
@@ -1964,6 +1976,7 @@
has_position = true;
}
} else if (auto* interpolate = deco->As<ast::InterpolateDecoration>()) {
+ interpolate_attribute = interpolate;
if (!ValidateInterpolateDecoration(interpolate, member->Type())) {
return false;
}
@@ -1976,6 +1989,12 @@
return false;
}
+ if (interpolate_attribute && !has_location) {
+ AddError("interpolate attribute must only be used with [[location]]",
+ interpolate_attribute->source);
+ return false;
+ }
+
if (auto* member_struct_type = member->Type()->As<sem::Struct>()) {
if (auto* member_struct_type_block_decoration =
ast::GetDecoration<ast::StructBlockDecoration>(
