Move entry point validation from Validator to Resolver
Move logic and relevant tests.
Remove validator/validator_decoration_test.cc.
Bug: tint:642
Change-Id: Ie89118a11233931605f18e762274bb2afcaaa460
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/46941
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 7d9003e..56fa8d3 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -522,7 +522,6 @@
utils/tmpfile_test.cc
utils/tmpfile.h
utils/unique_vector_test.cc
- validator/validator_decoration_test.cc
validator/validator_function_test.cc
validator/validator_test.cc
validator/validator_test_helper.cc
diff --git a/src/resolver/decoration_validation_test.cc b/src/resolver/decoration_validation_test.cc
index 1669a50..1b43289 100644
--- a/src/resolver/decoration_validation_test.cc
+++ b/src/resolver/decoration_validation_test.cc
@@ -272,5 +272,39 @@
TestParams{DecorationKind::kStructBlock, false},
TestParams{DecorationKind::kWorkgroup, false}));
+using FunctionDecorationTest = TestWithParams;
+TEST_P(FunctionDecorationTest, IsValid) {
+ auto params = GetParam();
+
+ Func("foo", ast::VariableList{}, ty.void_(), ast::StatementList{},
+ ast::DecorationList{
+ create<ast::StageDecoration>(ast::PipelineStage::kCompute),
+ createDecoration(Source{{12, 34}}, *this, params.kind)});
+
+ 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 functions");
+ }
+}
+INSTANTIATE_TEST_SUITE_P(
+ ValidatorTest,
+ FunctionDecorationTest,
+ testing::Values(TestParams{DecorationKind::kAccess, false},
+ 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::kOffset, false},
+ TestParams{DecorationKind::kSize, false},
+ // Skip kStage as we always apply it in this test
+ TestParams{DecorationKind::kStride, false},
+ TestParams{DecorationKind::kStructBlock, false},
+ TestParams{DecorationKind::kWorkgroup, true}));
+
} // namespace
} // namespace tint
diff --git a/src/resolver/function_validation_test.cc b/src/resolver/function_validation_test.cc
index ba839de..018e6bb 100644
--- a/src/resolver/function_validation_test.cc
+++ b/src/resolver/function_validation_test.cc
@@ -181,5 +181,25 @@
"return type, returned 'u32', expected 'myf32'");
}
+TEST_F(ResolverFunctionValidationTest, PipelineStage_MustBeUnique_Fail) {
+ // [[stage(fragment)]]
+ // [[stage(vertex)]]
+ // fn main() -> void { return; }
+ Func(Source{Source::Location{12, 34}}, "main", ast::VariableList{},
+ ty.void_(),
+ ast::StatementList{
+ create<ast::ReturnStatement>(),
+ },
+ ast::DecorationList{
+ create<ast::StageDecoration>(ast::PipelineStage::kVertex),
+ create<ast::StageDecoration>(ast::PipelineStage::kFragment),
+ });
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(r()->error(),
+ "12:34 error v-0020: only one stage decoration permitted per entry "
+ "point");
+}
+
} // namespace
} // namespace tint
diff --git a/src/resolver/resolver.cc b/src/resolver/resolver.cc
index d09ea62..87322b6 100644
--- a/src/resolver/resolver.cc
+++ b/src/resolver/resolver.cc
@@ -32,6 +32,7 @@
#include "src/ast/switch_statement.h"
#include "src/ast/unary_op_expression.h"
#include "src/ast/variable_decl_statement.h"
+#include "src/ast/workgroup_decoration.h"
#include "src/semantic/array.h"
#include "src/semantic/call.h"
#include "src/semantic/function.h"
@@ -328,6 +329,23 @@
}
bool Resolver::ValidateEntryPoint(const ast::Function* func) {
+ auto stage_deco_count = 0;
+ for (auto* deco : func->decorations()) {
+ if (deco->Is<ast::StageDecoration>()) {
+ stage_deco_count++;
+ } else if (!deco->Is<ast::WorkgroupDecoration>()) {
+ diagnostics_.add_error("decoration is not valid for functions",
+ deco->source());
+ return false;
+ }
+ }
+ if (stage_deco_count > 1) {
+ diagnostics_.add_error(
+ "v-0020", "only one stage decoration permitted per entry point",
+ func->source());
+ return false;
+ }
+
// Use a lambda to validate the entry point decorations for a type.
// Persistent state is used to track which builtins and locations have already
// been seen, in order to catch conflicts.
diff --git a/src/validator/validator_decoration_test.cc b/src/validator/validator_decoration_test.cc
deleted file mode 100644
index 069eb8a..0000000
--- a/src/validator/validator_decoration_test.cc
+++ /dev/null
@@ -1,125 +0,0 @@
-// Copyright 2021 The Tint Authors.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-#include "src/ast/access_decoration.h"
-#include "src/ast/binding_decoration.h"
-#include "src/ast/builtin_decoration.h"
-#include "src/ast/constant_id_decoration.h"
-#include "src/ast/group_decoration.h"
-#include "src/ast/location_decoration.h"
-#include "src/ast/stage_decoration.h"
-#include "src/ast/struct_block_decoration.h"
-#include "src/ast/struct_member_align_decoration.h"
-#include "src/ast/struct_member_offset_decoration.h"
-#include "src/ast/struct_member_size_decoration.h"
-#include "src/ast/workgroup_decoration.h"
-#include "src/validator/validator_test_helper.h"
-
-namespace tint {
-namespace {
-
-enum class DecorationKind {
- kAccess,
- kAlign,
- kBinding,
- kBuiltin,
- kConstantId,
- kGroup,
- kLocation,
- kOffset,
- kSize,
- kStage,
- kStride,
- kStructBlock,
- kWorkgroup,
-};
-struct DecorationTestParams {
- DecorationKind kind;
- bool should_pass;
-};
-class ValidatorDecorationsTestWithParams
- : public ValidatorTestHelper,
- public testing::TestWithParam<DecorationTestParams> {};
-
-ast::Decoration* createDecoration(ProgramBuilder& builder,
- DecorationKind kind) {
- switch (kind) {
- case DecorationKind::kAccess:
- return builder.create<ast::AccessDecoration>(
- ast::AccessControl::kReadOnly);
- case DecorationKind::kAlign:
- return builder.create<ast::StructMemberAlignDecoration>(4u);
- case DecorationKind::kBinding:
- return builder.create<ast::BindingDecoration>(1);
- case DecorationKind::kBuiltin:
- return builder.create<ast::BuiltinDecoration>(ast::Builtin::kPosition);
- case DecorationKind::kConstantId:
- return builder.create<ast::ConstantIdDecoration>(0u);
- case DecorationKind::kGroup:
- return builder.create<ast::GroupDecoration>(1u);
- case DecorationKind::kLocation:
- return builder.create<ast::LocationDecoration>(1);
- case DecorationKind::kOffset:
- return builder.create<ast::StructMemberOffsetDecoration>(4u);
- case DecorationKind::kSize:
- return builder.create<ast::StructMemberSizeDecoration>(4u);
- case DecorationKind::kStage:
- return builder.create<ast::StageDecoration>(ast::PipelineStage::kCompute);
- case DecorationKind::kStride:
- return builder.create<ast::StrideDecoration>(4u);
- case DecorationKind::kStructBlock:
- return builder.create<ast::StructBlockDecoration>();
- case DecorationKind::kWorkgroup:
- return builder.create<ast::WorkgroupDecoration>(1u, 1u, 1u);
- }
- return nullptr;
-}
-
-using FunctionDecorationTest = ValidatorDecorationsTestWithParams;
-TEST_P(FunctionDecorationTest, Decoration_IsValid) {
- auto params = GetParam();
-
- Func("foo", ast::VariableList{}, ty.void_(), ast::StatementList{},
- ast::DecorationList{
- create<ast::StageDecoration>(ast::PipelineStage::kCompute),
- createDecoration(*this, params.kind)});
-
- ValidatorImpl& v = Build();
-
- if (params.should_pass) {
- EXPECT_TRUE(v.Validate());
- } else {
- EXPECT_FALSE(v.Validate());
- EXPECT_EQ(v.error(), "decoration is not valid for functions");
- }
-}
-INSTANTIATE_TEST_SUITE_P(
- ValidatorTest,
- FunctionDecorationTest,
- testing::Values(DecorationTestParams{DecorationKind::kAccess, false},
- DecorationTestParams{DecorationKind::kAlign, false},
- DecorationTestParams{DecorationKind::kBinding, false},
- DecorationTestParams{DecorationKind::kBuiltin, false},
- DecorationTestParams{DecorationKind::kConstantId, false},
- DecorationTestParams{DecorationKind::kGroup, false},
- DecorationTestParams{DecorationKind::kLocation, false},
- DecorationTestParams{DecorationKind::kOffset, false},
- DecorationTestParams{DecorationKind::kSize, false},
- // Skip kStage as we always apply it in this test
- DecorationTestParams{DecorationKind::kStride, false},
- DecorationTestParams{DecorationKind::kStructBlock, false},
- DecorationTestParams{DecorationKind::kWorkgroup, true}));
-
-} // namespace
-} // namespace tint
diff --git a/src/validator/validator_function_test.cc b/src/validator/validator_function_test.cc
index 89ba7e4..00c7401 100644
--- a/src/validator/validator_function_test.cc
+++ b/src/validator/validator_function_test.cc
@@ -56,28 +56,6 @@
EXPECT_TRUE(v.Validate());
}
-TEST_F(ValidateFunctionTest, PipelineStage_MustBeUnique_Fail) {
- // [[stage(fragment)]]
- // [[stage(vertex)]]
- // fn main() -> void { return; }
- Func(Source{Source::Location{12, 34}}, "main", ast::VariableList{},
- ty.void_(),
- ast::StatementList{
- create<ast::ReturnStatement>(),
- },
- ast::DecorationList{
- create<ast::StageDecoration>(ast::PipelineStage::kVertex),
- create<ast::StageDecoration>(ast::PipelineStage::kFragment),
- });
-
- ValidatorImpl& v = Build();
-
- EXPECT_FALSE(v.Validate());
- EXPECT_EQ(
- v.error(),
- "12:34 v-0020: only one stage decoration permitted per entry point");
-}
-
TEST_F(ValidateFunctionTest, NoPipelineEntryPoints) {
Func("vtx_func", ast::VariableList{}, ty.void_(),
ast::StatementList{
diff --git a/src/validator/validator_impl.cc b/src/validator/validator_impl.cc
index 1e4558a..7d405fa 100644
--- a/src/validator/validator_impl.cc
+++ b/src/validator/validator_impl.cc
@@ -101,25 +101,7 @@
return true;
}
-bool ValidatorImpl::ValidateEntryPoint(const ast::FunctionList& funcs) {
- for (auto* func : funcs) {
- if (func->IsEntryPoint()) {
- auto stage_deco_count = 0;
- for (auto* deco : func->decorations()) {
- if (deco->Is<ast::StageDecoration>()) {
- stage_deco_count++;
- } else if (!deco->Is<ast::WorkgroupDecoration>()) {
- add_error(func->source(), "decoration is not valid for functions");
- return false;
- }
- }
- if (stage_deco_count > 1) {
- add_error(func->source(), "v-0020",
- "only one stage decoration permitted per entry point");
- return false;
- }
- }
- }
+bool ValidatorImpl::ValidateEntryPoint(const ast::FunctionList&) {
return true;
}
diff --git a/test/BUILD.gn b/test/BUILD.gn
index 87c8a8f..cb6546a 100644
--- a/test/BUILD.gn
+++ b/test/BUILD.gn
@@ -225,7 +225,6 @@
"../src/utils/tmpfile.h",
"../src/utils/tmpfile_test.cc",
"../src/utils/unique_vector_test.cc",
- "../src/validator/validator_decoration_test.cc",
"../src/validator/validator_function_test.cc",
"../src/validator/validator_test.cc",
"../src/validator/validator_test_helper.cc",
