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>

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

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::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

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::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

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<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.

src/validator/validator_decoration_test.cc

@@ -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

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::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{

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) {
+bool ValidatorImpl::ValidateEntryPoint(const ast::FunctionList&) {
-  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;
-      }
-    }
-  }
   return true;
 }
 

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",