diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 7d9003e980..56fa8d320b 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -522,7 +522,6 @@ if(${TINT_BUILD_TESTS}) 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 1669a502d6..1b43289e57 100644 --- a/src/resolver/decoration_validation_test.cc +++ b/src/resolver/decoration_validation_test.cc @@ -272,5 +272,39 @@ INSTANTIATE_TEST_SUITE_P( 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::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 ba839defb7..018e6bb38b 100644 --- a/src/resolver/function_validation_test.cc +++ b/src/resolver/function_validation_test.cc @@ -181,5 +181,25 @@ TEST_F(ResolverFunctionValidationTest, "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::DecorationList{ + create(ast::PipelineStage::kVertex), + create(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 d09ea623cd..87322b60e3 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::ValidateFunction(const ast::Function* func) { } bool Resolver::ValidateEntryPoint(const ast::Function* func) { + auto stage_deco_count = 0; + for (auto* deco : func->decorations()) { + if (deco->Is()) { + stage_deco_count++; + } else if (!deco->Is()) { + 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 069eb8a933..0000000000 --- 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 {}; - -ast::Decoration* createDecoration(ProgramBuilder& builder, - DecorationKind kind) { - switch (kind) { - case DecorationKind::kAccess: - return builder.create( - ast::AccessControl::kReadOnly); - case DecorationKind::kAlign: - return builder.create(4u); - case DecorationKind::kBinding: - return builder.create(1); - case DecorationKind::kBuiltin: - return builder.create(ast::Builtin::kPosition); - case DecorationKind::kConstantId: - return builder.create(0u); - case DecorationKind::kGroup: - return builder.create(1u); - case DecorationKind::kLocation: - return builder.create(1); - case DecorationKind::kOffset: - return builder.create(4u); - case DecorationKind::kSize: - return builder.create(4u); - case DecorationKind::kStage: - return builder.create(ast::PipelineStage::kCompute); - case DecorationKind::kStride: - return builder.create(4u); - case DecorationKind::kStructBlock: - return builder.create(); - case DecorationKind::kWorkgroup: - return builder.create(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::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 89ba7e49e2..00c7401568 100644 --- a/src/validator/validator_function_test.cc +++ b/src/validator/validator_function_test.cc @@ -56,28 +56,6 @@ TEST_F(ValidateFunctionTest, 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::DecorationList{ - create(ast::PipelineStage::kVertex), - create(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 1e4558a674..7d405faf95 100644 --- a/src/validator/validator_impl.cc +++ b/src/validator/validator_impl.cc @@ -101,25 +101,7 @@ bool ValidatorImpl::ValidateGlobalVariable(const ast::Variable* var) { 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()) { - stage_deco_count++; - } else if (!deco->Is()) { - 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 87c8a8f7c8..cb6546ab04 100644 --- a/test/BUILD.gn +++ b/test/BUILD.gn @@ -225,7 +225,6 @@ source_set("tint_unittests_core_src") { "../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",