Validate attributes on function parameters
Entry point function parameter can only have builtin and location
attributes (unless a disable validation attribute is present). Other
functions cannot have any decorations on their parameters.
Fix parameter creation in the CanonicalizeEntryPointIO transform for a
case where it was not stripping attributes that are not valid for
function parameters.
Change-Id: I000908aa2dc007c52d100c5f65bca051a49bec9a
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/55863
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: James Price <jrprice@google.com>
Auto-Submit: 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 ddf7b71..8164445 100644
--- a/src/resolver/decoration_validation_test.cc
+++ b/src/resolver/decoration_validation_test.cc
@@ -126,6 +126,105 @@
return {};
}
+using FunctionParameterDecorationTest = TestWithParams;
+TEST_P(FunctionParameterDecorationTest, IsValid) {
+ auto& params = GetParam();
+
+ Func("main",
+ ast::VariableList{Param("a", ty.vec4<f32>(),
+ createDecorations({}, *this, params.kind))},
+ ty.void_(), {});
+
+ if (params.should_pass) {
+ EXPECT_TRUE(r()->Resolve()) << r()->error();
+ } else {
+ EXPECT_FALSE(r()->Resolve()) << r()->error();
+ EXPECT_EQ(r()->error(),
+ "error: decoration is not valid for function parameters");
+ }
+}
+INSTANTIATE_TEST_SUITE_P(
+ ResolverDecorationValidationTest,
+ FunctionParameterDecorationTest,
+ testing::Values(TestParams{DecorationKind::kAlign, false},
+ TestParams{DecorationKind::kBinding, false},
+ TestParams{DecorationKind::kBuiltin, 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},
+ TestParams{DecorationKind::kStride, false},
+ TestParams{DecorationKind::kStructBlock, false},
+ TestParams{DecorationKind::kWorkgroup, false},
+ TestParams{DecorationKind::kBindingAndGroup, false}));
+
+using EntryPointParameterDecorationTest = TestWithParams;
+TEST_P(EntryPointParameterDecorationTest, IsValid) {
+ auto& params = GetParam();
+
+ Func("main",
+ ast::VariableList{Param("a", ty.vec4<f32>(),
+ createDecorations({}, *this, params.kind))},
+ ty.void_(), {},
+ ast::DecorationList{Stage(ast::PipelineStage::kFragment)});
+
+ if (params.should_pass) {
+ EXPECT_TRUE(r()->Resolve()) << r()->error();
+ } else {
+ EXPECT_FALSE(r()->Resolve()) << r()->error();
+ EXPECT_EQ(r()->error(),
+ "error: decoration is not valid for function parameters");
+ }
+}
+INSTANTIATE_TEST_SUITE_P(
+ ResolverDecorationValidationTest,
+ EntryPointParameterDecorationTest,
+ testing::Values(TestParams{DecorationKind::kAlign, false},
+ TestParams{DecorationKind::kBinding, false},
+ TestParams{DecorationKind::kBuiltin, true},
+ TestParams{DecorationKind::kGroup, false},
+ 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},
+ TestParams{DecorationKind::kBindingAndGroup, false}));
+
+TEST_F(EntryPointParameterDecorationTest, DuplicateDecoration) {
+ Func("main", ast::VariableList{}, ty.f32(), ast::StatementList{Return(1.f)},
+ {Stage(ast::PipelineStage::kFragment)},
+ {
+ Location(Source{{12, 34}}, 2),
+ Location(Source{{56, 78}}, 3),
+ });
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(r()->error(),
+ R"(56:78 error: duplicate location decoration
+12:34 note: first decoration declared here)");
+}
+
+TEST_F(EntryPointParameterDecorationTest, DuplicateInternalDecoration) {
+ auto* s =
+ Param("s", ty.sampler(ast::SamplerKind::kSampler),
+ ast::DecorationList{
+ create<ast::BindingDecoration>(0),
+ create<ast::GroupDecoration>(0),
+ ASTNodes().Create<ast::DisableValidationDecoration>(
+ ID(), ast::DisabledValidation::kBindingPointCollision),
+ ASTNodes().Create<ast::DisableValidationDecoration>(
+ ID(), ast::DisabledValidation::kEntryPointParameter),
+ });
+ Func("f", {s}, ty.void_(), {}, {Stage(ast::PipelineStage::kFragment)});
+
+ EXPECT_TRUE(r()->Resolve()) << r()->error();
+}
+
using FunctionReturnTypeDecorationTest = TestWithParams;
TEST_P(FunctionReturnTypeDecorationTest, IsValid) {
auto& params = GetParam();
@@ -385,22 +484,6 @@
12:34 note: first decoration declared here)");
}
-TEST_F(VariableDecorationTest, DuplicateInternalDecoration) {
- auto* s =
- Param("s", ty.sampler(ast::SamplerKind::kSampler),
- ast::DecorationList{
- create<ast::BindingDecoration>(0),
- create<ast::GroupDecoration>(0),
- ASTNodes().Create<ast::DisableValidationDecoration>(
- ID(), ast::DisabledValidation::kBindingPointCollision),
- ASTNodes().Create<ast::DisableValidationDecoration>(
- ID(), ast::DisabledValidation::kEntryPointParameter),
- });
- Func("f", {s}, ty.void_(), {});
-
- EXPECT_TRUE(r()->Resolve()) << r()->error();
-}
-
using ConstantDecorationTest = TestWithParams;
TEST_P(ConstantDecorationTest, IsValid) {
auto& params = GetParam();
diff --git a/src/resolver/resolver.cc b/src/resolver/resolver.cc
index ce2e608..405f15c 100644
--- a/src/resolver/resolver.cc
+++ b/src/resolver/resolver.cc
@@ -927,15 +927,30 @@
return true;
}
-bool Resolver::ValidateParameter(const VariableInfo* info) {
+bool Resolver::ValidateParameter(const ast::Function* func,
+ const VariableInfo* info) {
if (!ValidateVariable(info)) {
return false;
}
for (auto* deco : info->declaration->decorations()) {
+ if (!func->IsEntryPoint()) {
+ AddError("decoration is not valid for function parameters",
+ deco->source());
+ return false;
+ }
+
if (auto* builtin = deco->As<ast::BuiltinDecoration>()) {
if (!ValidateBuiltinDecoration(builtin, info->type)) {
return false;
}
+ } else if (!deco->IsAnyOf<ast::LocationDecoration,
+ ast::InternalDecoration>() &&
+ !IsValidationDisabled(
+ info->declaration->decorations(),
+ ast::DisabledValidation::kEntryPointParameter)) {
+ AddError("decoration is not valid for function parameters",
+ deco->source());
+ return false;
}
}
return true;
@@ -1041,7 +1056,7 @@
}
for (auto* param : func->params()) {
- if (!ValidateParameter(variable_to_info_.at(param))) {
+ if (!ValidateParameter(func, variable_to_info_.at(param))) {
return false;
}
}
diff --git a/src/resolver/resolver.h b/src/resolver/resolver.h
index 531912f..336b44c 100644
--- a/src/resolver/resolver.h
+++ b/src/resolver/resolver.h
@@ -279,7 +279,7 @@
bool ValidateMatrix(const sem::Matrix* matirx_type, const Source& source);
bool ValidateMatrixConstructor(const ast::TypeConstructorExpression* ctor,
const sem::Matrix* matrix_type);
- bool ValidateParameter(const VariableInfo* info);
+ bool ValidateParameter(const ast::Function* func, const VariableInfo* info);
bool ValidateReturn(const ast::ReturnStatement* ret);
bool ValidateStatements(const ast::StatementList& stmts);
bool ValidateStorageTexture(const ast::StorageTexture* t);
diff --git a/src/transform/canonicalize_entry_point_io.cc b/src/transform/canonicalize_entry_point_io.cc
index ebac0b9..eaf9ce9 100644
--- a/src/transform/canonicalize_entry_point_io.cc
+++ b/src/transform/canonicalize_entry_point_io.cc
@@ -151,6 +151,13 @@
<< "nested pipeline IO struct";
}
+ ast::DecorationList new_decorations = RemoveDecorations(
+ &ctx, member->Declaration()->decorations(),
+ [](const ast::Decoration* deco) {
+ return !deco->IsAnyOf<ast::BuiltinDecoration,
+ ast::LocationDecoration>();
+ });
+
if (cfg->builtin_style == BuiltinStyle::kParameter &&
ast::HasDecoration<ast::BuiltinDecoration>(
member->Declaration()->decorations())) {
@@ -158,19 +165,12 @@
// parameters, then move it to the parameter list.
auto* member_ty = CreateASTTypeFor(&ctx, member->Type());
auto new_param_name = ctx.dst->Sym();
- new_parameters.push_back(ctx.dst->Param(
- new_param_name, member_ty,
- ctx.Clone(member->Declaration()->decorations())));
+ new_parameters.push_back(
+ ctx.dst->Param(new_param_name, member_ty, new_decorations));
init_values.push_back(ctx.dst->Expr(new_param_name));
continue;
}
- ast::DecorationList new_decorations = RemoveDecorations(
- &ctx, member->Declaration()->decorations(),
- [](const ast::Decoration* deco) {
- return !deco->IsAnyOf<ast::BuiltinDecoration,
- ast::LocationDecoration>();
- });
auto member_name = ctx.Clone(member->Declaration()->symbol());
auto* member_type = ctx.Clone(member->Declaration()->type());
new_struct_members.push_back(
