validation: Reject struct builtins on wrong stages
Change-Id: I047f28c5be6a7d878b9f7ad37b6accc842b4fe98
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/65840
Auto-Submit: James Price <jrprice@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
diff --git a/src/resolver/builtins_validation_test.cc b/src/resolver/builtins_validation_test.cc
index e29b6f2..733093a 100644
--- a/src/resolver/builtins_validation_test.cc
+++ b/src/resolver/builtins_validation_test.cc
@@ -217,7 +217,7 @@
"fragment pipeline stage");
}
-TEST_F(ResolverBuiltinsValidationTest, FragDepthIsInputStruct_Ignored) {
+TEST_F(ResolverBuiltinsValidationTest, FragDepthIsInputStruct_Fail) {
// struct MyInputs {
// [[builtin(frag_depth)]] ff: f32;
// };
@@ -231,8 +231,28 @@
Func("fragShader", {Param("arg", ty.Of(s))}, ty.f32(), {Return(1.0f)},
{Stage(ast::PipelineStage::kFragment)}, {Location(0)});
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(r()->error(),
+ "12:34 error: builtin(frag_depth) cannot be used in input of "
+ "fragment pipeline stage\n"
+ "note: while analysing entry point 'fragShader'");
+}
+
+TEST_F(ResolverBuiltinsValidationTest, StructBuiltinInsideEntryPoint_Ignored) {
+ // struct S {
+ // [[builtin(vertex_index)]] idx: u32;
+ // };
+ // [[stage(fragment)]]
+ // fn fragShader() { var s : S; }
+
+ Structure("S",
+ {Member("idx", ty.u32(), {Builtin(ast::Builtin::kVertexIndex)})});
+
+ Func("fragShader", {}, ty.void_(), {Decl(Var("s", ty.type_name("S")))},
+ {Stage(ast::PipelineStage::kFragment)});
EXPECT_TRUE(r()->Resolve());
}
+
} // namespace StageTest
TEST_F(ResolverBuiltinsValidationTest, PositionNotF32_Struct_Fail) {
diff --git a/src/resolver/resolver.cc b/src/resolver/resolver.cc
index 9617d5d..dd9904c 100644
--- a/src/resolver/resolver.cc
+++ b/src/resolver/resolver.cc
@@ -1277,8 +1277,7 @@
bool Resolver::ValidateBuiltinDecoration(const ast::BuiltinDecoration* deco,
const sem::Type* storage_type,
- const bool is_input,
- const bool is_struct_member) {
+ const bool is_input) {
auto* type = storage_type->UnwrapRef();
const auto stage = current_function_
? current_function_->declaration->pipeline_stage()
@@ -1386,16 +1385,12 @@
break;
}
- // ignore builtin attribute on struct members to facillate data movement
- // between stages
- if (!is_struct_member) {
- if (is_stage_mismatch) {
- AddError(deco_to_str(deco) + " cannot be used in " +
- (is_input ? "input of " : "output of ") + stage_name.str() +
- " pipeline stage",
- deco->source());
- return false;
- }
+ if (is_stage_mismatch) {
+ AddError(deco_to_str(deco) + " cannot be used in " +
+ (is_input ? "input of " : "output of ") + stage_name.str() +
+ " pipeline stage",
+ deco->source());
+ return false;
}
return true;
@@ -1556,10 +1551,9 @@
return false;
}
- if (!ValidateBuiltinDecoration(
- builtin, ty,
- /* is_input */ param_or_ret == ParamOrRetType::kParameter,
- /* is_struct_member */ is_struct_member)) {
+ if (!ValidateBuiltinDecoration(builtin, ty,
+ /* is_input */ param_or_ret ==
+ ParamOrRetType::kParameter)) {
return false;
}
builtins.emplace(builtin->value());
@@ -4098,8 +4092,7 @@
}
} else if (auto* builtin = deco->As<ast::BuiltinDecoration>()) {
if (!ValidateBuiltinDecoration(builtin, member->Type(),
- /* is_input */ false,
- /* is_struct_member */ true)) {
+ /* is_input */ false)) {
return false;
}
if (builtin->value() == ast::Builtin::kPosition) {
diff --git a/src/resolver/resolver.h b/src/resolver/resolver.h
index 18965be..3b05f2c 100644
--- a/src/resolver/resolver.h
+++ b/src/resolver/resolver.h
@@ -283,8 +283,7 @@
bool ValidateAssignment(const ast::AssignmentStatement* a);
bool ValidateBuiltinDecoration(const ast::BuiltinDecoration* deco,
const sem::Type* storage_type,
- const bool is_input,
- const bool is_struct_member);
+ const bool is_input);
bool ValidateCall(ast::CallExpression* call);
bool ValidateCallStatement(ast::CallStatement* stmt);
bool ValidateEntryPoint(const ast::Function* func, const FunctionInfo* info);
diff --git a/src/resolver/struct_pipeline_stage_use_test.cc b/src/resolver/struct_pipeline_stage_use_test.cc
index 7c0c022..9cd4871 100644
--- a/src/resolver/struct_pipeline_stage_use_test.cc
+++ b/src/resolver/struct_pipeline_stage_use_test.cc
@@ -140,8 +140,8 @@
auto* s = Structure(
"S", {Member("a", ty.vec4<f32>(), {Builtin(ast::Builtin::kPosition)})});
- Func("vert_main", {Param("param", ty.Of(s))}, ty.Of(s),
- {Return(Construct(ty.Of(s)))}, {Stage(ast::PipelineStage::kVertex)});
+ Func("vert_main", {}, ty.Of(s), {Return(Construct(ty.Of(s)))},
+ {Stage(ast::PipelineStage::kVertex)});
Func("frag_main", {Param("param", ty.Of(s))}, ty.void_(), {},
{Stage(ast::PipelineStage::kFragment)});
@@ -151,8 +151,7 @@
auto* sem = TypeOf(s)->As<sem::Struct>();
ASSERT_NE(sem, nullptr);
EXPECT_THAT(sem->PipelineStageUses(),
- UnorderedElementsAre(sem::PipelineStageUsage::kVertexInput,
- sem::PipelineStageUsage::kVertexOutput,
+ UnorderedElementsAre(sem::PipelineStageUsage::kVertexOutput,
sem::PipelineStageUsage::kFragmentInput));
}
