Move struct validation from Validator to Resolver
* Moved Validator::ValidateConstructedType, which only validated
structs, to Resolver as ValidateStructure.
* Moved relevant tests to new files, and also updated all failing tests
to validate Source location.
* Fixed other tests that broke now that we're validating structs.
Bug: tint:642
Change-Id: Iefc08ef548f52d8c3798d814d2183c56d1236c2d
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/45160
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index d23767a..faec6b1 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -467,6 +467,7 @@
inspector/inspector_test.cc
intrinsic_table_test.cc
program_test.cc
+ resolver/decoration_validation_test.cc
resolver/intrinsic_test.cc
resolver/is_storeable_test.cc
resolver/resolver_test_helper.cc
@@ -474,6 +475,7 @@
resolver/resolver_test.cc
resolver/struct_layout_test.cc
resolver/struct_storage_class_use_test.cc
+ resolver/type_validation_test.cc
resolver/validation_test.cc
scope_stack_test.cc
semantic/sem_intrinsic_test.cc
diff --git a/src/diagnostic/diagnostic.h b/src/diagnostic/diagnostic.h
index 79d978f..4b0c824 100644
--- a/src/diagnostic/diagnostic.h
+++ b/src/diagnostic/diagnostic.h
@@ -110,21 +110,35 @@
/// adds the error message without a source to the end of this list.
/// @param err_msg the error message
- void add_error(const std::string& err_msg) {
+ void add_error(std::string err_msg) {
diag::Diagnostic error{};
error.severity = diag::Severity::Error;
- error.message = err_msg;
+ error.message = std::move(err_msg);
add(std::move(error));
}
/// adds the error message with the given Source to the end of this list.
/// @param err_msg the error message
/// @param source the source of the error diagnostic
- void add_error(const std::string& err_msg, const Source& source) {
+ void add_error(std::string err_msg, const Source& source) {
diag::Diagnostic error{};
error.severity = diag::Severity::Error;
error.source = source;
- error.message = err_msg;
+ error.message = std::move(err_msg);
+ add(std::move(error));
+ }
+
+ /// adds the error message with the given code and Source to the end of this
+ /// list.
+ /// @param code the error code
+ /// @param err_msg the error message
+ /// @param source the source of the error diagnostic
+ void add_error(const char* code, std::string err_msg, const Source& source) {
+ diag::Diagnostic error{};
+ error.code = code;
+ error.severity = diag::Severity::Error;
+ error.source = source;
+ error.message = std::move(err_msg);
add(std::move(error));
}
diff --git a/src/inspector/inspector_test.cc b/src/inspector/inspector_test.cc
index c1a3a42..4363815 100644
--- a/src/inspector/inspector_test.cc
+++ b/src/inspector/inspector_test.cc
@@ -222,6 +222,17 @@
return struct_type;
}
+ /// Returns true if the struct with `member_types` requires a block decoration
+ /// @param member_types a vector of member types
+ /// @returns true if block decoration is required
+ bool StructRequiresBlockDecoration(
+ std::vector<type::Type*> member_types) const {
+ // Structure needs a [[block]] attribute if the last member is a
+ // dynamically-sized array.
+ return member_types.back()->Is<type::Array>(
+ [](auto&& a) { return a->IsRuntimeArray(); });
+ }
+
/// Generates types appropriate for using in a storage buffer
/// @param name name for the type
/// @param member_types a vector of member types
@@ -231,7 +242,8 @@
std::tuple<type::Struct*, type::AccessControl*> MakeStorageBufferTypes(
const std::string& name,
std::vector<type::Type*> member_types) {
- auto* struct_type = MakeStructType(name, member_types, false);
+ bool is_block = StructRequiresBlockDecoration(member_types);
+ auto* struct_type = MakeStructType(name, member_types, is_block);
auto* access_type = create<type::AccessControl>(
ast::AccessControl::kReadWrite, struct_type);
return {struct_type, std::move(access_type)};
@@ -246,7 +258,8 @@
std::tuple<type::Struct*, type::AccessControl*>
MakeReadOnlyStorageBufferTypes(const std::string& name,
std::vector<type::Type*> member_types) {
- auto* struct_type = MakeStructType(name, member_types, false);
+ bool is_block = StructRequiresBlockDecoration(member_types);
+ auto* struct_type = MakeStructType(name, member_types, is_block);
auto* access_type =
create<type::AccessControl>(ast::AccessControl::kReadOnly, struct_type);
return {struct_type, std::move(access_type)};
diff --git a/src/resolver/decoration_validation_test.cc b/src/resolver/decoration_validation_test.cc
new file mode 100644
index 0000000..b1a45c1
--- /dev/null
+++ b/src/resolver/decoration_validation_test.cc
@@ -0,0 +1,205 @@
+// 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/constant_id_decoration.h"
+#include "src/ast/stage_decoration.h"
+#include "src/ast/struct_block_decoration.h"
+#include "src/ast/workgroup_decoration.h"
+#include "src/resolver/resolver.h"
+#include "src/resolver/resolver_test_helper.h"
+
+#include "gmock/gmock.h"
+
+namespace tint {
+namespace {
+
+enum class DecorationKind {
+ kAccess,
+ kAlign,
+ kBinding,
+ kBuiltin,
+ kConstantId,
+ kGroup,
+ kLocation,
+ kOffset,
+ kSize,
+ kStage,
+ kStride,
+ kStructBlock,
+ kWorkgroup,
+};
+struct TestParams {
+ DecorationKind kind;
+ bool should_pass;
+};
+class TestWithParams : public resolver::TestHelper,
+ public testing::TestWithParam<TestParams> {};
+
+ast::Decoration* createDecoration(const Source& source,
+ ProgramBuilder& builder,
+ DecorationKind kind) {
+ switch (kind) {
+ case DecorationKind::kAccess:
+ return builder.create<ast::AccessDecoration>(
+ source, ast::AccessControl::kReadOnly);
+ case DecorationKind::kAlign:
+ return builder.create<ast::StructMemberAlignDecoration>(source, 4u);
+ case DecorationKind::kBinding:
+ return builder.create<ast::BindingDecoration>(source, 1);
+ case DecorationKind::kBuiltin:
+ return builder.create<ast::BuiltinDecoration>(source,
+ ast::Builtin::kPosition);
+ case DecorationKind::kConstantId:
+ return builder.create<ast::ConstantIdDecoration>(source, 0u);
+ case DecorationKind::kGroup:
+ return builder.create<ast::GroupDecoration>(source, 1u);
+ case DecorationKind::kLocation:
+ return builder.create<ast::LocationDecoration>(source, 1);
+ case DecorationKind::kOffset:
+ return builder.create<ast::StructMemberOffsetDecoration>(source, 4u);
+ case DecorationKind::kSize:
+ return builder.create<ast::StructMemberSizeDecoration>(source, 4u);
+ case DecorationKind::kStage:
+ return builder.create<ast::StageDecoration>(source,
+ ast::PipelineStage::kCompute);
+ case DecorationKind::kStride:
+ return builder.create<ast::StrideDecoration>(source, 4u);
+ case DecorationKind::kStructBlock:
+ return builder.create<ast::StructBlockDecoration>(source);
+ case DecorationKind::kWorkgroup:
+ return builder.create<ast::WorkgroupDecoration>(source, 1u, 1u, 1u);
+ }
+ return nullptr;
+}
+
+using ArrayDecorationTest = TestWithParams;
+
+TEST_P(ArrayDecorationTest, IsValid) {
+ auto params = GetParam();
+
+ ast::StructMemberList members{Member(
+ "a", create<type::Array>(ty.f32(), 0,
+ ast::DecorationList{createDecoration(
+ Source{{12, 34}}, *this, params.kind)}))};
+ auto* s = create<ast::Struct>(
+ members, ast::DecorationList{create<ast::StructBlockDecoration>()});
+ auto* s_ty = ty.struct_("mystruct", s);
+ AST().AddConstructedType(s_ty);
+
+ WrapInFunction();
+
+ 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 array types");
+ }
+}
+INSTANTIATE_TEST_SUITE_P(
+ ResolverDecorationValidationTest,
+ ArrayDecorationTest,
+ 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},
+ TestParams{DecorationKind::kStage, false},
+ TestParams{DecorationKind::kStride, true},
+ TestParams{DecorationKind::kStructBlock, false},
+ TestParams{DecorationKind::kWorkgroup, false}));
+
+using StructDecorationTest = TestWithParams;
+TEST_P(StructDecorationTest, IsValid) {
+ auto params = GetParam();
+
+ auto* s = create<ast::Struct>(ast::StructMemberList{},
+ ast::DecorationList{createDecoration(
+ Source{{12, 34}}, *this, params.kind)});
+ auto* s_ty = ty.struct_("mystruct", s);
+ AST().AddConstructedType(s_ty);
+
+ WrapInFunction();
+
+ 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 struct declarations");
+ }
+}
+INSTANTIATE_TEST_SUITE_P(
+ ResolverDecorationValidationTest,
+ StructDecorationTest,
+ 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},
+ TestParams{DecorationKind::kStage, false},
+ TestParams{DecorationKind::kStride, false},
+ TestParams{DecorationKind::kStructBlock, true},
+ TestParams{DecorationKind::kWorkgroup, false}));
+
+using StructMemberDecorationTest = TestWithParams;
+TEST_P(StructMemberDecorationTest, IsValid) {
+ auto params = GetParam();
+
+ ast::StructMemberList members{
+ Member("a", ty.i32(),
+ ast::DecorationList{
+ createDecoration(Source{{12, 34}}, *this, params.kind)})};
+ auto* s = create<ast::Struct>(members, ast::DecorationList{});
+ auto* s_ty = ty.struct_("mystruct", s);
+ AST().AddConstructedType(s_ty);
+
+ WrapInFunction();
+
+ 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 structure members");
+ }
+}
+INSTANTIATE_TEST_SUITE_P(
+ ResolverDecorationValidationTest,
+ StructMemberDecorationTest,
+ testing::Values(TestParams{DecorationKind::kAccess, false},
+ TestParams{DecorationKind::kAlign, true},
+ TestParams{DecorationKind::kBinding, false},
+ TestParams{DecorationKind::kBuiltin, true},
+ TestParams{DecorationKind::kConstantId, false},
+ TestParams{DecorationKind::kGroup, false},
+ TestParams{DecorationKind::kLocation, true},
+ TestParams{DecorationKind::kOffset, true},
+ TestParams{DecorationKind::kSize, true},
+ TestParams{DecorationKind::kStage, false},
+ TestParams{DecorationKind::kStride, false},
+ TestParams{DecorationKind::kStructBlock, false},
+ TestParams{DecorationKind::kWorkgroup, false}));
+
+} // namespace
+} // namespace tint
diff --git a/src/resolver/resolver.cc b/src/resolver/resolver.cc
index 5d6b756..90a4afc 100644
--- a/src/resolver/resolver.cc
+++ b/src/resolver/resolver.cc
@@ -27,6 +27,7 @@
#include "src/ast/if_statement.h"
#include "src/ast/loop_statement.h"
#include "src/ast/return_statement.h"
+#include "src/ast/struct_block_decoration.h"
#include "src/ast/switch_statement.h"
#include "src/ast/unary_op_expression.h"
#include "src/ast/variable_decl_statement.h"
@@ -1391,6 +1392,61 @@
return create_semantic(utils::RoundUp(el_align, el_size));
}
+bool Resolver::ValidateStructure(const type::Struct* st) {
+ for (auto* member : st->impl()->members()) {
+ if (auto* r = member->type()->UnwrapAll()->As<type::Array>()) {
+ if (r->IsRuntimeArray()) {
+ if (member != st->impl()->members().back()) {
+ diagnostics_.add_error(
+ "v-0015",
+ "runtime arrays may only appear as the last member of a struct",
+ member->source());
+ return false;
+ }
+ if (!st->IsBlockDecorated()) {
+ diagnostics_.add_error("v-0015",
+ "a struct containing a runtime-sized array "
+ "requires the [[block]] attribute: '" +
+ builder_->Symbols().NameFor(st->symbol()) +
+ "'",
+ member->source());
+ return false;
+ }
+
+ for (auto* deco : r->decorations()) {
+ if (!deco->Is<ast::StrideDecoration>()) {
+ diagnostics_.add_error("decoration is not valid for array types",
+ deco->source());
+ return false;
+ }
+ }
+ }
+ }
+
+ for (auto* deco : member->decorations()) {
+ if (!(deco->Is<ast::BuiltinDecoration>() ||
+ deco->Is<ast::LocationDecoration>() ||
+ deco->Is<ast::StructMemberOffsetDecoration>() ||
+ deco->Is<ast::StructMemberSizeDecoration>() ||
+ deco->Is<ast::StructMemberAlignDecoration>())) {
+ diagnostics_.add_error("decoration is not valid for structure members",
+ deco->source());
+ return false;
+ }
+ }
+ }
+
+ for (auto* deco : st->impl()->decorations()) {
+ if (!(deco->Is<ast::StructBlockDecoration>())) {
+ diagnostics_.add_error("decoration is not valid for struct declarations",
+ deco->source());
+ return false;
+ }
+ }
+
+ return true;
+}
+
Resolver::StructInfo* Resolver::Structure(type::Struct* str) {
auto info_it = struct_info_.find(str);
if (info_it != struct_info_.end()) {
@@ -1398,6 +1454,10 @@
return info_it->second;
}
+ if (!ValidateStructure(str)) {
+ return nullptr;
+ }
+
semantic::StructMemberList sem_members;
sem_members.reserve(str->impl()->members().size());
diff --git a/src/resolver/resolver.h b/src/resolver/resolver.h
index 63e5850..97e915a 100644
--- a/src/resolver/resolver.h
+++ b/src/resolver/resolver.h
@@ -216,6 +216,10 @@
/// returned.
const semantic::Array* Array(type::Array*);
+ /// @returns returns true if input struct is valid
+ /// @param st the struct to validate
+ bool ValidateStructure(const type::Struct* st);
+
/// @returns the StructInfo for the structure `str`, building it if it hasn't
/// been constructed already. If an error is raised, nullptr is returned.
StructInfo* Structure(type::Struct* str);
diff --git a/src/resolver/struct_layout_test.cc b/src/resolver/struct_layout_test.cc
index f77e5e3..6b29a88 100644
--- a/src/resolver/struct_layout_test.cc
+++ b/src/resolver/struct_layout_test.cc
@@ -15,6 +15,7 @@
#include "src/resolver/resolver.h"
#include "gmock/gmock.h"
+#include "src/ast/struct_block_decoration.h"
#include "src/resolver/resolver_test_helper.h"
#include "src/semantic/struct.h"
@@ -125,9 +126,12 @@
}
TEST_F(ResolverStructLayoutTest, ImplicitStrideArrayRuntimeSized) {
- auto* s = Structure("S", {
- Member("c", ty.array<f32>()),
- });
+ auto* s =
+ Structure("S",
+ {
+ Member("c", ty.array<f32>()),
+ },
+ ast::DecorationList{create<ast::StructBlockDecoration>()});
ASSERT_TRUE(r()->Resolve()) << r()->error();
@@ -143,9 +147,12 @@
}
TEST_F(ResolverStructLayoutTest, ExplicitStrideArrayRuntimeSized) {
- auto* s = Structure("S", {
- Member("c", ty.array<f32>(/*stride*/ 32)),
- });
+ auto* s =
+ Structure("S",
+ {
+ Member("c", ty.array<f32>(/*stride*/ 32)),
+ },
+ ast::DecorationList{create<ast::StructBlockDecoration>()});
ASSERT_TRUE(r()->Resolve()) << r()->error();
diff --git a/src/resolver/type_validation_test.cc b/src/resolver/type_validation_test.cc
new file mode 100644
index 0000000..23caf52
--- /dev/null
+++ b/src/resolver/type_validation_test.cc
@@ -0,0 +1,152 @@
+// 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/stage_decoration.h"
+#include "src/ast/struct_block_decoration.h"
+#include "src/resolver/resolver.h"
+#include "src/resolver/resolver_test_helper.h"
+
+#include "gmock/gmock.h"
+
+namespace tint {
+namespace {
+
+class ResolverTypeValidationTest : public resolver::TestHelper,
+ public testing::Test {};
+
+TEST_F(ResolverTypeValidationTest, RuntimeArrayIsLast_Pass) {
+ // [[Block]]
+ // struct Foo {
+ // vf: f32;
+ // rt: array<f32>;
+ // };
+
+ ast::DecorationList decos;
+ decos.push_back(create<ast::StructBlockDecoration>());
+ auto* st =
+ create<ast::Struct>(ast::StructMemberList{Member("vf", ty.f32()),
+ Member("rt", ty.array<f32>())},
+ decos);
+
+ auto* struct_type = ty.struct_("Foo", st);
+ AST().AddConstructedType(struct_type);
+
+ WrapInFunction();
+
+ EXPECT_TRUE(r()->Resolve()) << r()->error();
+}
+
+TEST_F(ResolverTypeValidationTest, RuntimeArrayIsLastNoBlock_Fail) {
+ // struct Foo {
+ // vf: f32;
+ // rt: array<f32>;
+ // };
+
+ ast::DecorationList decos;
+ auto* st = create<ast::Struct>(
+ ast::StructMemberList{Member("vf", ty.f32()),
+ Member(Source{{12, 34}}, "rt", ty.array<f32>())},
+ decos);
+
+ auto* struct_type = ty.struct_("Foo", st);
+ AST().AddConstructedType(struct_type);
+
+ WrapInFunction();
+
+ EXPECT_FALSE(r()->Resolve()) << r()->error();
+ EXPECT_EQ(r()->error(),
+ "12:34 error v-0015: a struct containing a runtime-sized array "
+ "requires the [[block]] attribute: 'Foo'");
+}
+
+TEST_F(ResolverTypeValidationTest, RuntimeArrayIsNotLast_Fail) {
+ // [[Block]]
+ // struct Foo {
+ // rt: array<f32>;
+ // vf: f32;
+ // };
+
+ ast::DecorationList decos;
+ decos.push_back(create<ast::StructBlockDecoration>());
+
+ auto* rt = Member(Source{{12, 34}}, "rt", ty.array<f32>());
+ auto* st = create<ast::Struct>(
+ ast::StructMemberList{rt, Member("vf", ty.f32())}, decos);
+
+ auto* struct_type = ty.struct_("Foo", st);
+
+ AST().AddConstructedType(struct_type);
+
+ WrapInFunction();
+
+ EXPECT_FALSE(r()->Resolve()) << r()->error();
+ EXPECT_EQ(r()->error(),
+ "12:34 error v-0015: runtime arrays may only appear as the last "
+ "member of a struct");
+}
+
+TEST_F(ResolverTypeValidationTest, AliasRuntimeArrayIsNotLast_Fail) {
+ // [[Block]]
+ // type RTArr = array<u32>;
+ // struct s {
+ // b: RTArr;
+ // a: u32;
+ //}
+
+ auto* alias = ty.alias("RTArr", ty.array<u32>());
+
+ ast::DecorationList decos;
+ decos.push_back(create<ast::StructBlockDecoration>());
+ auto* st = create<ast::Struct>(
+ ast::StructMemberList{Member(Source{{12, 34}}, "b", alias),
+ Member("a", ty.u32())},
+ decos);
+
+ auto* struct_type = ty.struct_("s", st);
+ AST().AddConstructedType(struct_type);
+
+ WrapInFunction();
+
+ EXPECT_FALSE(r()->Resolve()) << r()->error();
+ EXPECT_EQ(
+ r()->error(),
+ "12:34 error v-0015: runtime arrays may only appear as the last member "
+ "of a struct");
+}
+
+TEST_F(ResolverTypeValidationTest, AliasRuntimeArrayIsLast_Pass) {
+ // [[Block]]
+ // type RTArr = array<u32>;
+ // struct s {
+ // a: u32;
+ // b: RTArr;
+ //}
+
+ auto* alias = ty.alias("RTArr", ty.array<u32>());
+
+ ast::DecorationList decos;
+ decos.push_back(create<ast::StructBlockDecoration>());
+ auto* st = create<ast::Struct>(
+ ast::StructMemberList{Member("a", ty.u32()), Member("b", alias)}, decos);
+
+ auto* struct_type = ty.struct_("s", st);
+ AST().AddConstructedType(struct_type);
+
+ WrapInFunction();
+
+ EXPECT_TRUE(r()->Resolve()) << r()->error();
+}
+
+} // namespace
+} // namespace tint
diff --git a/src/transform/bound_array_accessors_test.cc b/src/transform/bound_array_accessors_test.cc
index 7eedc18..344a75e 100644
--- a/src/transform/bound_array_accessors_test.cc
+++ b/src/transform/bound_array_accessors_test.cc
@@ -535,6 +535,7 @@
TEST_F(BoundArrayAccessorsTest, RuntimeArray_Clamps) {
auto* src = R"(
+[[block]]
struct S {
a : f32;
b : array<f32>;
@@ -547,6 +548,7 @@
)";
auto* expect = R"(
+[[block]]
struct S {
a : f32;
b : array<f32>;
diff --git a/src/validator/validator_decoration_test.cc b/src/validator/validator_decoration_test.cc
index a9b8de7..52343f8 100644
--- a/src/validator/validator_decoration_test.cc
+++ b/src/validator/validator_decoration_test.cc
@@ -86,44 +86,6 @@
return nullptr;
}
-using ArrayDecorationTest = ValidatorDecorationsTestWithParams;
-TEST_P(ArrayDecorationTest, Decoration_IsValid) {
- auto params = GetParam();
-
- ast::StructMemberList members{Member(
- "a", create<type::Array>(
- ty.f32(), 0,
- ast::DecorationList{createDecoration(*this, params.kind)}))};
- auto* s = create<ast::Struct>(
- members, ast::DecorationList{create<ast::StructBlockDecoration>()});
- auto* s_ty = ty.struct_("mystruct", s);
-
- ValidatorImpl& v = Build();
-
- if (params.should_pass) {
- EXPECT_TRUE(v.ValidateConstructedType(s_ty));
- } else {
- EXPECT_FALSE(v.ValidateConstructedType(s_ty));
- EXPECT_EQ(v.error(), "decoration is not valid for array types");
- }
-}
-INSTANTIATE_TEST_SUITE_P(
- ValidatorTest,
- ArrayDecorationTest,
- 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},
- DecorationTestParams{DecorationKind::kStage, false},
- DecorationTestParams{DecorationKind::kStride, true},
- DecorationTestParams{DecorationKind::kStructBlock, false},
- DecorationTestParams{DecorationKind::kWorkgroup, false}));
-
using FunctionDecorationTest = ValidatorDecorationsTestWithParams;
TEST_P(FunctionDecorationTest, Decoration_IsValid) {
auto params = GetParam();
@@ -195,77 +157,6 @@
DecorationTestParams{DecorationKind::kStructBlock, false},
DecorationTestParams{DecorationKind::kWorkgroup, false}));
-using StructDecorationTest = ValidatorDecorationsTestWithParams;
-TEST_P(StructDecorationTest, Decoration_IsValid) {
- auto params = GetParam();
-
- auto* s = create<ast::Struct>(
- ast::StructMemberList{},
- ast::DecorationList{createDecoration(*this, params.kind)});
- auto* s_ty = ty.struct_("mystruct", s);
-
- ValidatorImpl& v = Build();
-
- if (params.should_pass) {
- EXPECT_TRUE(v.ValidateConstructedType(s_ty));
- } else {
- EXPECT_FALSE(v.ValidateConstructedType(s_ty));
- EXPECT_EQ(v.error(), "decoration is not valid for struct declarations");
- }
-}
-INSTANTIATE_TEST_SUITE_P(
- ValidatorTest,
- StructDecorationTest,
- 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},
- DecorationTestParams{DecorationKind::kStage, false},
- DecorationTestParams{DecorationKind::kStride, false},
- DecorationTestParams{DecorationKind::kStructBlock, true},
- DecorationTestParams{DecorationKind::kWorkgroup, false}));
-
-using StructMemberDecorations = ValidatorDecorationsTestWithParams;
-TEST_P(StructMemberDecorations, Decoration_IsValid) {
- auto params = GetParam();
-
- ast::StructMemberList members{
- Member("a", ty.i32(),
- ast::DecorationList{createDecoration(*this, params.kind)})};
- auto* s = create<ast::Struct>(members, ast::DecorationList{});
- auto* s_ty = ty.struct_("mystruct", s);
-
- ValidatorImpl& v = Build();
-
- if (params.should_pass) {
- EXPECT_TRUE(v.ValidateConstructedType(s_ty));
- } else {
- EXPECT_FALSE(v.ValidateConstructedType(s_ty));
- EXPECT_EQ(v.error(), "decoration is not valid for structure members");
- }
-}
-INSTANTIATE_TEST_SUITE_P(
- ValidatorTest,
- StructMemberDecorations,
- testing::Values(DecorationTestParams{DecorationKind::kAccess, false},
- DecorationTestParams{DecorationKind::kAlign, true},
- DecorationTestParams{DecorationKind::kBinding, false},
- DecorationTestParams{DecorationKind::kBuiltin, true},
- DecorationTestParams{DecorationKind::kConstantId, false},
- DecorationTestParams{DecorationKind::kGroup, false},
- DecorationTestParams{DecorationKind::kLocation, true},
- DecorationTestParams{DecorationKind::kOffset, true},
- DecorationTestParams{DecorationKind::kSize, true},
- DecorationTestParams{DecorationKind::kStage, false},
- DecorationTestParams{DecorationKind::kStride, false},
- DecorationTestParams{DecorationKind::kStructBlock, false},
- DecorationTestParams{DecorationKind::kWorkgroup, false}));
-
using VariableDecorationTest = ValidatorDecorationsTestWithParams;
TEST_P(VariableDecorationTest, Decoration_IsValid) {
auto params = GetParam();
diff --git a/src/validator/validator_impl.cc b/src/validator/validator_impl.cc
index add2fc1..d57a3c4 100644
--- a/src/validator/validator_impl.cc
+++ b/src/validator/validator_impl.cc
@@ -72,9 +72,8 @@
// Validate global declarations in the order they appear in the module.
for (auto* decl : program_->AST().GlobalDeclarations()) {
if (auto* ty = decl->As<type::Type>()) {
- if (!ValidateConstructedType(ty)) {
- return false;
- }
+ // Validated by Resolver (Struct types only)
+ return true;
} else if (auto* func = decl->As<ast::Function>()) {
current_function_ = func;
if (!ValidateFunction(func)) {
@@ -97,60 +96,6 @@
return true;
}
-bool ValidatorImpl::ValidateConstructedType(const type::Type* type) {
- if (auto* st = type->As<type::Struct>()) {
- for (auto* member : st->impl()->members()) {
- if (auto* r = member->type()->UnwrapAll()->As<type::Array>()) {
- if (r->IsRuntimeArray()) {
- if (member != st->impl()->members().back()) {
- add_error(member->source(), "v-0015",
- "runtime arrays may only appear as the last member of "
- "a struct");
- return false;
- }
- if (!st->IsBlockDecorated()) {
- add_error(member->source(), "v-0015",
- "a struct containing a runtime-sized array "
- "requires the [[block]] attribute: '" +
- program_->Symbols().NameFor(st->symbol()) + "'");
- return false;
- }
-
- for (auto* deco : r->decorations()) {
- if (!deco->Is<ast::StrideDecoration>()) {
- add_error(deco->source(),
- "decoration is not valid for array types");
- return false;
- }
- }
- }
- }
-
- for (auto* deco : member->decorations()) {
- if (!(deco->Is<ast::BuiltinDecoration>() ||
- deco->Is<ast::LocationDecoration>() ||
- deco->Is<ast::StructMemberOffsetDecoration>() ||
- deco->Is<ast::StructMemberSizeDecoration>() ||
- deco->Is<ast::StructMemberAlignDecoration>())) {
- add_error(deco->source(),
- "decoration is not valid for structure members");
- return false;
- }
- }
- }
-
- for (auto* deco : st->impl()->decorations()) {
- if (!(deco->Is<ast::StructBlockDecoration>())) {
- add_error(deco->source(),
- "decoration is not valid for struct declarations");
- return false;
- }
- }
- }
-
- return true;
-}
-
bool ValidatorImpl::ValidateGlobalVariable(const ast::Variable* var) {
auto* sem = program_->Sem().Get(var);
if (!sem) {
diff --git a/src/validator/validator_impl.h b/src/validator/validator_impl.h
index e5f02c3..1d2338e 100644
--- a/src/validator/validator_impl.h
+++ b/src/validator/validator_impl.h
@@ -131,12 +131,6 @@
/// @param funcs the functions to check
/// @returns true if the valdiation was successful
bool ValidateEntryPoint(const ast::FunctionList& funcs);
-
- /// Validates a constructed type
- /// @param type the type to check
- /// @returns true if the valdiation was successful
- bool ValidateConstructedType(const type::Type* type);
-
/// Returns true if the given type is storable. This uses and
/// updates `storable_` and `not_storable_`.
/// @param type the given type
diff --git a/src/validator/validator_type_test.cc b/src/validator/validator_type_test.cc
index da372c6..dd991ae 100644
--- a/src/validator/validator_type_test.cc
+++ b/src/validator/validator_type_test.cc
@@ -21,129 +21,6 @@
class ValidatorTypeTest : public ValidatorTestHelper, public testing::Test {};
-TEST_F(ValidatorTypeTest, RuntimeArrayIsLast_Pass) {
- // [[Block]]
- // struct Foo {
- // vf: f32;
- // rt: array<f32>;
- // };
-
- ast::DecorationList decos;
- decos.push_back(create<ast::StructBlockDecoration>());
- auto* st =
- create<ast::Struct>(ast::StructMemberList{Member("vf", ty.f32()),
- Member("rt", ty.array<f32>())},
- decos);
-
- auto* struct_type = ty.struct_("Foo", st);
-
- AST().AddConstructedType(struct_type);
-
- ValidatorImpl& v = Build();
-
- EXPECT_TRUE(v.ValidateConstructedType(struct_type));
-}
-
-TEST_F(ValidatorTypeTest, RuntimeArrayIsLastNoBlock_Fail) {
- // struct Foo {
- // vf: f32;
- // rt: array<f32>;
- // };
-
- ast::DecorationList decos;
- auto* st =
- create<ast::Struct>(ast::StructMemberList{Member("vf", ty.f32()),
- Member("rt", ty.array<f32>())},
- decos);
-
- auto* struct_type = ty.struct_("Foo", st);
- AST().AddConstructedType(struct_type);
-
- ValidatorImpl& v = Build();
-
- EXPECT_FALSE(v.ValidateConstructedType(struct_type));
- EXPECT_EQ(v.error(),
- "v-0015: a struct containing a runtime-sized array requires the "
- "[[block]] attribute: 'Foo'");
-}
-
-TEST_F(ValidatorTypeTest, RuntimeArrayIsNotLast_Fail) {
- // [[Block]]
- // struct Foo {
- // rt: array<f32>;
- // vf: f32;
- // };
-
- ast::DecorationList decos;
- decos.push_back(create<ast::StructBlockDecoration>());
-
- SetSource(Source::Location{12, 34});
- auto* rt = Member("rt", ty.array<f32>());
- SetSource(Source{});
- auto* st = create<ast::Struct>(
- ast::StructMemberList{rt, Member("vf", ty.f32())}, decos);
-
- auto* struct_type = ty.struct_("Foo", st);
-
- AST().AddConstructedType(struct_type);
-
- ValidatorImpl& v = Build();
-
- EXPECT_FALSE(v.ValidateConstructedType(struct_type));
- EXPECT_EQ(v.error(),
- "12:34 v-0015: runtime arrays may only appear as the last member "
- "of a struct");
-}
-
-TEST_F(ValidatorTypeTest, AliasRuntimeArrayIsNotLast_Fail) {
- // [[Block]]
- // type RTArr = array<u32>;
- // struct s {
- // b: RTArr;
- // a: u32;
- //}
-
- auto* alias = ty.alias("RTArr", ty.array<u32>());
-
- ast::DecorationList decos;
- decos.push_back(create<ast::StructBlockDecoration>());
- auto* st = create<ast::Struct>(
- ast::StructMemberList{Member("b", alias), Member("a", ty.u32())}, decos);
-
- auto* struct_type = ty.struct_("s", st);
- AST().AddConstructedType(struct_type);
-
- ValidatorImpl& v = Build();
-
- EXPECT_FALSE(v.ValidateConstructedType(struct_type));
- EXPECT_EQ(v.error(),
- "v-0015: runtime arrays may only appear as the last member "
- "of a struct");
-}
-
-TEST_F(ValidatorTypeTest, AliasRuntimeArrayIsLast_Pass) {
- // [[Block]]
- // type RTArr = array<u32>;
- // struct s {
- // a: u32;
- // b: RTArr;
- //}
-
- auto* alias = ty.alias("RTArr", ty.array<u32>());
-
- ast::DecorationList decos;
- decos.push_back(create<ast::StructBlockDecoration>());
- auto* st = create<ast::Struct>(
- ast::StructMemberList{Member("a", ty.u32()), Member("b", alias)}, decos);
-
- auto* struct_type = ty.struct_("s", st);
- AST().AddConstructedType(struct_type);
-
- ValidatorImpl& v = Build();
-
- EXPECT_TRUE(v.ValidateConstructedType(struct_type));
-}
-
TEST_F(ValidatorTypeTest, RuntimeArrayInFunction_Fail) {
/// [[stage(vertex)]]
// fn func -> void { var a : array<i32>; }
diff --git a/src/writer/msl/generator_impl_type_test.cc b/src/writer/msl/generator_impl_type_test.cc
index d13ad94..2540a8f 100644
--- a/src/writer/msl/generator_impl_type_test.cc
+++ b/src/writer/msl/generator_impl_type_test.cc
@@ -410,14 +410,17 @@
// array_z: size(4), align(4)
auto* array_z = ty.array<f32>();
- auto* s = Structure("S", {
- Member("a", ty.i32()),
- Member("b", array_x),
- Member("c", ty.f32()),
- Member("d", array_y),
- Member("e", ty.f32()),
- Member("f", array_z),
- });
+ auto* s =
+ Structure("S",
+ {
+ Member("a", ty.i32()),
+ Member("b", array_x),
+ Member("c", ty.f32()),
+ Member("d", array_y),
+ Member("e", ty.f32()),
+ Member("f", array_z),
+ },
+ ast::DecorationList{create<ast::StructBlockDecoration>()});
Global("G", s, ast::StorageClass::kStorage);
diff --git a/src/writer/spirv/builder_type_test.cc b/src/writer/spirv/builder_type_test.cc
index 5e990a1..6ebb64d 100644
--- a/src/writer/spirv/builder_type_test.cc
+++ b/src/writer/spirv/builder_type_test.cc
@@ -438,11 +438,14 @@
auto* arr_arr_mat2x3 = ty.array(ty.mat2x3<f32>(), 1); // Doubly nested array
auto* rtarr_mat4x4 = ty.array(ty.mat4x4<f32>(), 0); // Runtime array
- auto* s = Structure("S", {
- Member("a", arr_mat2x2),
- Member("b", arr_arr_mat2x3),
- Member("c", rtarr_mat4x4),
- });
+ auto* s =
+ Structure("S",
+ {
+ Member("a", arr_mat2x2),
+ Member("b", arr_arr_mat2x3),
+ Member("c", rtarr_mat4x4),
+ },
+ ast::DecorationList{create<ast::StructBlockDecoration>()});
spirv::Builder& b = Build();
@@ -469,7 +472,8 @@
OpMemberName %1 1 "b"
OpMemberName %1 2 "c"
)");
- EXPECT_EQ(DumpInstructions(b.annots()), R"(OpMemberDecorate %1 0 Offset 0
+ EXPECT_EQ(DumpInstructions(b.annots()), R"(OpDecorate %1 Block
+OpMemberDecorate %1 0 Offset 0
OpMemberDecorate %1 0 ColMajor
OpMemberDecorate %1 0 MatrixStride 8
OpDecorate %2 ArrayStride 16
diff --git a/test/BUILD.gn b/test/BUILD.gn
index 2285415..d595209 100644
--- a/test/BUILD.gn
+++ b/test/BUILD.gn
@@ -169,13 +169,15 @@
"../src/intrinsic_table_test.cc",
"../src/program_builder_test.cc",
"../src/program_test.cc",
+ "../src/resolver/decoration_validation_test.cc",
"../src/resolver/intrinsic_test.cc",
"../src/resolver/is_storeable_test.cc",
+ "../src/resolver/resolver_test.cc",
"../src/resolver/resolver_test_helper.cc",
"../src/resolver/resolver_test_helper.h",
- "../src/resolver/resolver_test.cc",
"../src/resolver/struct_layout_test.cc",
"../src/resolver/struct_storage_class_use_test.cc",
+ "../src/resolver/type_validation_test.cc",
"../src/resolver/validation_test.cc",
"../src/scope_stack_test.cc",
"../src/semantic/sem_intrinsic_test.cc",
