From bc6720b9f6554c4ee4800adeb818cc007729103c Mon Sep 17 00:00:00 2001 From: Ben Clayton Date: Thu, 27 Apr 2023 19:29:45 +0000 Subject: [PATCH] tint/type: Remove Source from Struct & StructMember type::Struct is the base class of sem::Struct. type::Struct does not have a Declaration() member, so it does not make sense for it to have a Source. Given that sem::Struct has a Declaration() method, use this to obtain the source. Same logic applies to StructMember. Change-Id: I693f659c35216ebe5eac5ea2a5b6457773077ddc Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/129480 Commit-Queue: Ben Clayton Reviewed-by: Dan Sinclair Kokoro: Ben Clayton --- src/tint/resolver/builtin_structs.cc | 2 -- src/tint/resolver/inferred_type_test.cc | 11 ++++----- src/tint/resolver/resolver.cc | 15 ++++++----- src/tint/resolver/validator.cc | 33 +++++++++++++------------ src/tint/sem/struct.cc | 6 ++--- src/tint/sem/struct.h | 4 --- src/tint/sem/struct_test.cc | 16 ++++++------ src/tint/transform/transform_test.cc | 4 +-- src/tint/type/struct.cc | 15 ++++------- src/tint/type/struct.h | 16 ++---------- src/tint/type/struct_test.cc | 18 +++++++------- src/tint/type/type_test.cc | 12 +++------ 12 files changed, 60 insertions(+), 92 deletions(-) diff --git a/src/tint/resolver/builtin_structs.cc b/src/tint/resolver/builtin_structs.cc index b817a578e3..4e45c16c32 100644 --- a/src/tint/resolver/builtin_structs.cc +++ b/src/tint/resolver/builtin_structs.cc @@ -45,7 +45,6 @@ type::Struct* BuildStruct(ProgramBuilder& b, offset = utils::RoundUp(align, offset); max_align = std::max(max_align, align); members.Push(b.create( - /* source */ Source{}, /* name */ b.Sym(m.name), /* type */ m.type, /* index */ static_cast(members.Length()), @@ -58,7 +57,6 @@ type::Struct* BuildStruct(ProgramBuilder& b, uint32_t size_without_padding = offset; uint32_t size_with_padding = utils::RoundUp(max_align, offset); return b.create( - /* source */ Source{}, /* name */ b.Sym(name), /* members */ std::move(members), /* align */ max_align, diff --git a/src/tint/resolver/inferred_type_test.cc b/src/tint/resolver/inferred_type_test.cc index 2fb844605e..4545419ea2 100644 --- a/src/tint/resolver/inferred_type_test.cc +++ b/src/tint/resolver/inferred_type_test.cc @@ -150,12 +150,11 @@ TEST_F(ResolverInferredTypeTest, InferStruct_Pass) { auto* member = Member("x", ty.i32()); auto* str = Structure("S", utils::Vector{member}); - auto* expected_type = - create(str, str->source, str->name->symbol, - utils::Vector{create( - member, member->source, member->name->symbol, create(), - 0u, 0u, 0u, 4u, type::StructMemberAttributes{})}, - 0u, 4u, 4u); + auto* expected_type = create( + str, str->name->symbol, + utils::Vector{create(member, member->name->symbol, create(), + 0u, 0u, 0u, 4u, type::StructMemberAttributes{})}, + 0u, 4u, 4u); auto* ctor_expr = Call(ty.Of(str)); diff --git a/src/tint/resolver/resolver.cc b/src/tint/resolver/resolver.cc index 3505f08a4e..de29692a0f 100644 --- a/src/tint/resolver/resolver.cc +++ b/src/tint/resolver/resolver.cc @@ -4189,9 +4189,9 @@ sem::Struct* Resolver::Structure(const ast::Struct* str) { } auto* sem_member = builder_->create( - member, member->source, member->name->symbol, type, - static_cast(sem_members.Length()), static_cast(offset), - static_cast(align), static_cast(size), attributes); + member, member->name->symbol, type, static_cast(sem_members.Length()), + static_cast(offset), static_cast(align), + static_cast(size), attributes); builder_->Sem().Add(member, sem_member); sem_members.Push(sem_member); @@ -4214,14 +4214,13 @@ sem::Struct* Resolver::Structure(const ast::Struct* str) { } auto* out = builder_->create( - str, str->source, str->name->symbol, std::move(sem_members), - static_cast(struct_align), static_cast(struct_size), - static_cast(size_no_padding)); + str, str->name->symbol, std::move(sem_members), static_cast(struct_align), + static_cast(struct_size), static_cast(size_no_padding)); for (size_t i = 0; i < sem_members.Length(); i++) { auto* mem_type = sem_members[i]->Type(); if (mem_type->Is()) { - atomic_composite_info_.Add(out, &sem_members[i]->Source()); + atomic_composite_info_.Add(out, &sem_members[i]->Declaration()->source); break; } else { if (auto found = atomic_composite_info_.Get(mem_type)) { @@ -4566,7 +4565,7 @@ bool Resolver::ApplyAddressSpaceUsageToType(builtin::AddressSpace address_space, utils::StringStream err; err << "while analyzing structure member " << sem_.TypeNameOf(str) << "." << member->Name().Name(); - AddNote(err.str(), member->Source()); + AddNote(err.str(), member->Declaration()->source); return false; } } diff --git a/src/tint/resolver/validator.cc b/src/tint/resolver/validator.cc index 1d6d7cfa13..63f2b4d882 100644 --- a/src/tint/resolver/validator.cc +++ b/src/tint/resolver/validator.cc @@ -445,7 +445,7 @@ bool Validator::AddressSpaceLayout(const type::Type* store_ty, // Recurse into the member type. if (!AddressSpaceLayout(m->Type(), address_space, m->Declaration()->type->source)) { - AddNote("see layout of struct:\n" + str->Layout(), str->Source()); + AddNote("see layout of struct:\n" + str->Layout(), str->Declaration()->source); note_usage(); return false; } @@ -461,13 +461,13 @@ bool Validator::AddressSpaceLayout(const type::Type* store_ty, "' is currently at offset " + std::to_string(m->Offset()) + ". Consider setting @align(" + std::to_string(required_align) + ") on this member", - m->Source()); + m->Declaration()->source); - AddNote("see layout of struct:\n" + str->Layout(), str->Source()); + AddNote("see layout of struct:\n" + str->Layout(), str->Declaration()->source); if (auto* member_str = m->Type()->As()) { AddNote("and layout of struct member:\n" + member_str->Layout(), - member_str->Source()); + member_str->Declaration()->source); } note_usage(); @@ -483,19 +483,19 @@ bool Validator::AddressSpaceLayout(const type::Type* store_ty, !enabled_extensions_.Contains( builtin::Extension::kChromiumInternalRelaxedUniformLayout)) { AddError( - "uniform storage requires that the number of bytes between the " - "start of the previous member of type struct and the current " - "member be a multiple of 16 bytes, but there are currently " + + "uniform storage requires that the number of bytes between the start of " + "the previous member of type struct and the current member be a multiple " + "of 16 bytes, but there are currently " + std::to_string(prev_to_curr_offset) + " bytes between '" + member_name_of(prev_member) + "' and '" + member_name_of(m) + "'. Consider setting @align(16) on this member", - m->Source()); + m->Declaration()->source); - AddNote("see layout of struct:\n" + str->Layout(), str->Source()); + AddNote("see layout of struct:\n" + str->Layout(), str->Declaration()->source); auto* prev_member_str = prev_member->Type()->As(); AddNote("and layout of previous member struct:\n" + prev_member_str->Layout(), - prev_member_str->Source()); + prev_member_str->Declaration()->source); note_usage(); return false; } @@ -1213,8 +1213,8 @@ bool Validator::EntryPoint(const sem::Function* func, ast::PipelineStage stage) if (auto* str = ty->As()) { for (auto* member : str->Members()) { if (!validate_entry_point_attributes_inner( - member->Declaration()->attributes, member->Type(), member->Source(), - param_or_ret, + member->Declaration()->attributes, member->Type(), + member->Declaration()->source, param_or_ret, /*is_struct_member*/ true, member->Attributes().location)) { AddNote("while analyzing entry point '" + decl->name->symbol.Name() + "'", decl->source); @@ -2066,7 +2066,7 @@ bool Validator::Alias(const ast::Alias*) const { bool Validator::Structure(const sem::Struct* str, ast::PipelineStage stage) const { if (str->Members().IsEmpty()) { - AddError("structures must have at least one member", str->Source()); + AddError("structures must have at least one member", str->Declaration()->source); return false; } @@ -2076,7 +2076,7 @@ bool Validator::Structure(const sem::Struct* str, ast::PipelineStage stage) cons if (r->Count()->Is()) { if (member != str->Members().Back()) { AddError("runtime arrays may only appear as the last member of a struct", - member->Source()); + member->Declaration()->source); return false; } } @@ -2088,7 +2088,7 @@ bool Validator::Structure(const sem::Struct* str, ast::PipelineStage stage) cons } else if (!IsFixedFootprint(member->Type())) { AddError( "a struct that contains a runtime array cannot be nested inside another struct", - member->Source()); + member->Declaration()->source); return false; } @@ -2107,7 +2107,8 @@ bool Validator::Structure(const sem::Struct* str, ast::PipelineStage stage) cons has_location = true; TINT_ASSERT(Resolver, member->Attributes().location.has_value()); if (!LocationAttribute(location, member->Attributes().location.value(), - member->Type(), locations, stage, member->Source())) { + member->Type(), locations, stage, + member->Declaration()->source)) { return false; } return true; diff --git a/src/tint/sem/struct.cc b/src/tint/sem/struct.cc index 6856d0f0b2..e6c0e73d60 100644 --- a/src/tint/sem/struct.cc +++ b/src/tint/sem/struct.cc @@ -22,20 +22,18 @@ TINT_INSTANTIATE_TYPEINFO(tint::sem::StructMember); namespace tint::sem { Struct::Struct(const ast::Struct* declaration, - tint::Source source, Symbol name, utils::VectorRef members, uint32_t align, uint32_t size, uint32_t size_no_padding) - : Base(source, name, members, align, size, size_no_padding), declaration_(declaration) { + : Base(name, members, align, size, size_no_padding), declaration_(declaration) { TINT_ASSERT(Semantic, declaration != nullptr); } Struct::~Struct() = default; StructMember::StructMember(const ast::StructMember* declaration, - tint::Source source, Symbol name, const type::Type* type, uint32_t index, @@ -43,7 +41,7 @@ StructMember::StructMember(const ast::StructMember* declaration, uint32_t align, uint32_t size, const type::StructMemberAttributes& attributes) - : Base(source, name, type, index, offset, align, size, attributes), declaration_(declaration) { + : Base(name, type, index, offset, align, size, attributes), declaration_(declaration) { TINT_ASSERT(Semantic, declaration != nullptr); } diff --git a/src/tint/sem/struct.h b/src/tint/sem/struct.h index c927875bdf..fd59dcda58 100644 --- a/src/tint/sem/struct.h +++ b/src/tint/sem/struct.h @@ -43,14 +43,12 @@ class Struct final : public utils::Castable { public: /// Constructor /// @param declaration the AST structure declaration - /// @param source the source of the structure /// @param name the name of the structure /// @param members the structure members /// @param align the byte alignment of the structure /// @param size the byte size of the structure /// @param size_no_padding size of the members without the end of structure alignment padding Struct(const ast::Struct* declaration, - tint::Source source, Symbol name, utils::VectorRef members, uint32_t align, @@ -78,7 +76,6 @@ class StructMember final : public utils::Castable(Ident(name), utils::Empty, utils::Empty); auto* ptr = impl; - auto* s = create(impl, impl->source, impl->name->symbol, utils::Empty, - 4u /* align */, 8u /* size */, 16u /* size_no_padding */); + auto* s = create(impl, impl->name->symbol, utils::Empty, 4u /* align */, + 8u /* size */, 16u /* size_no_padding */); EXPECT_EQ(s->Declaration(), ptr); EXPECT_EQ(s->Align(), 4u); EXPECT_EQ(s->Size(), 8u); @@ -36,11 +36,11 @@ TEST_F(SemStructTest, Creation) { TEST_F(SemStructTest, Equals) { auto* a_impl = create(Ident("a"), utils::Empty, utils::Empty); - auto* a = create(a_impl, a_impl->source, a_impl->name->symbol, utils::Empty, - 4u /* align */, 4u /* size */, 4u /* size_no_padding */); + auto* a = create(a_impl, a_impl->name->symbol, utils::Empty, 4u /* align */, + 4u /* size */, 4u /* size_no_padding */); auto* b_impl = create(Ident("b"), utils::Empty, utils::Empty); - auto* b = create(b_impl, b_impl->source, b_impl->name->symbol, utils::Empty, - 4u /* align */, 4u /* size */, 4u /* size_no_padding */); + auto* b = create(b_impl, b_impl->name->symbol, utils::Empty, 4u /* align */, + 4u /* size */, 4u /* size_no_padding */); EXPECT_TRUE(a->Equals(*a)); EXPECT_FALSE(a->Equals(*b)); @@ -50,8 +50,8 @@ TEST_F(SemStructTest, Equals) { TEST_F(SemStructTest, FriendlyName) { auto name = Sym("my_struct"); auto* impl = create(Ident(name), utils::Empty, utils::Empty); - auto* s = create(impl, impl->source, impl->name->symbol, utils::Empty, - 4u /* align */, 4u /* size */, 4u /* size_no_padding */); + auto* s = create(impl, impl->name->symbol, utils::Empty, 4u /* align */, + 4u /* size */, 4u /* size_no_padding */); EXPECT_EQ(s->FriendlyName(), "my_struct"); } diff --git a/src/tint/transform/transform_test.cc b/src/tint/transform/transform_test.cc index 1201e98f33..2da1ec5535 100644 --- a/src/tint/transform/transform_test.cc +++ b/src/tint/transform/transform_test.cc @@ -120,8 +120,8 @@ TEST_F(CreateASTTypeForTest, AliasedArrayWithComplexOverrideLength) { TEST_F(CreateASTTypeForTest, Struct) { auto str = create([](ProgramBuilder& b) { auto* decl = b.Structure("S", {}); - return b.create(decl, decl->source, decl->name->symbol, utils::Empty, - 4u /* align */, 4u /* size */, 4u /* size_no_padding */); + return b.create(decl, decl->name->symbol, utils::Empty, 4u /* align */, + 4u /* size */, 4u /* size_no_padding */); }); ast::CheckIdentifier(str, "S"); diff --git a/src/tint/type/struct.cc b/src/tint/type/struct.cc index 58d58e6bc4..c9864cbae3 100644 --- a/src/tint/type/struct.cc +++ b/src/tint/type/struct.cc @@ -52,14 +52,12 @@ type::Flags FlagsFrom(utils::VectorRef members) { } // namespace -Struct::Struct(tint::Source source, - Symbol name, +Struct::Struct(Symbol name, utils::VectorRef members, uint32_t align, uint32_t size, uint32_t size_no_padding) : Base(utils::Hash(utils::TypeInfo::Of().full_hashcode, name), FlagsFrom(members)), - source_(source), name_(name), members_(std::move(members)), align_(align), @@ -169,19 +167,17 @@ Struct* Struct::Clone(CloneContext& ctx) const { for (const auto& mem : members_) { members.Push(mem->Clone(ctx)); } - return ctx.dst.mgr->Get(source_, sym, members, align_, size_, size_no_padding_); + return ctx.dst.mgr->Get(sym, members, align_, size_, size_no_padding_); } -StructMember::StructMember(tint::Source source, - Symbol name, +StructMember::StructMember(Symbol name, const type::Type* type, uint32_t index, uint32_t offset, uint32_t align, uint32_t size, const StructMemberAttributes& attributes) - : source_(source), - name_(name), + : name_(name), type_(type), index_(index), offset_(offset), @@ -194,8 +190,7 @@ StructMember::~StructMember() = default; StructMember* StructMember::Clone(CloneContext& ctx) const { auto sym = ctx.dst.st->Register(name_.Name()); auto* ty = type_->Clone(ctx); - return ctx.dst.mgr->Get(source_, sym, ty, index_, offset_, align_, size_, - attributes_); + return ctx.dst.mgr->Get(sym, ty, index_, offset_, align_, size_, attributes_); } } // namespace tint::type diff --git a/src/tint/type/struct.h b/src/tint/type/struct.h index fc214ba9ee..08119d0ca5 100644 --- a/src/tint/type/struct.h +++ b/src/tint/type/struct.h @@ -49,15 +49,13 @@ enum class PipelineStageUsage { class Struct : public utils::Castable { public: /// Constructor - /// @param source the source of the structure /// @param name the name of the structure /// @param members the structure members /// @param align the byte alignment of the structure /// @param size the byte size of the structure /// @param size_no_padding size of the members without the end of structure /// alignment padding - Struct(tint::Source source, - Symbol name, + Struct(Symbol name, utils::VectorRef members, uint32_t align, uint32_t size, @@ -70,9 +68,6 @@ class Struct : public utils::Castable { /// @returns true if the this type is equal to @p other bool Equals(const UniqueNode& other) const override; - /// @returns the source of the structure - tint::Source Source() const { return source_; } - /// @returns the name of the structure Symbol Name() const { return name_; } @@ -153,7 +148,6 @@ class Struct : public utils::Castable { Struct* Clone(CloneContext& ctx) const override; private: - const tint::Source source_; const Symbol name_; const utils::Vector members_; const uint32_t align_; @@ -180,7 +174,6 @@ struct StructMemberAttributes { class StructMember : public utils::Castable { public: /// Constructor - /// @param source the source of the struct member /// @param name the name of the structure member /// @param type the type of the member /// @param index the index of the member in the structure @@ -188,8 +181,7 @@ class StructMember : public utils::Castable { /// @param align the byte alignment of the member /// @param size the byte size of the member /// @param attributes the optional attributes - StructMember(tint::Source source, - Symbol name, + StructMember(Symbol name, const type::Type* type, uint32_t index, uint32_t offset, @@ -200,9 +192,6 @@ class StructMember : public utils::Castable { /// Destructor ~StructMember() override; - /// @returns the source the struct member - const tint::Source& Source() const { return source_; } - /// @returns the name of the structure member Symbol Name() const { return name_; } @@ -236,7 +225,6 @@ class StructMember : public utils::Castable { StructMember* Clone(CloneContext& ctx) const; private: - const tint::Source source_; const Symbol name_; const type::Struct* struct_; const type::Type* type_; diff --git a/src/tint/type/struct_test.cc b/src/tint/type/struct_test.cc index d7f68a15b1..bb88740f79 100644 --- a/src/tint/type/struct_test.cc +++ b/src/tint/type/struct_test.cc @@ -24,7 +24,7 @@ using TypeStructTest = TestHelper; TEST_F(TypeStructTest, Creation) { auto name = Sym("S"); - auto* s = create(Source{}, name, utils::Empty, 4u /* align */, 8u /* size */, + auto* s = create(name, utils::Empty, 4u /* align */, 8u /* size */, 16u /* size_no_padding */); EXPECT_EQ(s->Align(), 4u); EXPECT_EQ(s->Size(), 8u); @@ -32,9 +32,9 @@ TEST_F(TypeStructTest, Creation) { } TEST_F(TypeStructTest, Equals) { - auto* a = create(Source{}, Sym("a"), utils::Empty, 4u /* align */, 4u /* size */, + auto* a = create(Sym("a"), utils::Empty, 4u /* align */, 4u /* size */, 4u /* size_no_padding */); - auto* b = create(Source{}, Sym("b"), utils::Empty, 4u /* align */, 4u /* size */, + auto* b = create(Sym("b"), utils::Empty, 4u /* align */, 4u /* size */, 4u /* size_no_padding */); EXPECT_TRUE(a->Equals(*a)); @@ -44,8 +44,8 @@ TEST_F(TypeStructTest, Equals) { TEST_F(TypeStructTest, FriendlyName) { auto name = Sym("my_struct"); - auto* s = create(Source{}, name, utils::Empty, 4u /* align */, 4u /* size */, - 4u /* size_no_padding */); + auto* s = + create(name, utils::Empty, 4u /* align */, 4u /* size */, 4u /* size_no_padding */); EXPECT_EQ(s->FriendlyName(), "my_struct"); } @@ -209,10 +209,10 @@ TEST_F(TypeStructTest, Clone) { attrs_location_2.location = 2; auto* s = create( - Source{}, Sym("my_struct"), - utils::Vector{create(Source{}, Sym("b"), create(create(), 3u), - 0u, 0u, 16u, 12u, attrs_location_2), - create(Source{}, Sym("a"), create(), 1u, 16u, 4u, 4u, + Sym("my_struct"), + utils::Vector{create(Sym("b"), create(create(), 3u), 0u, 0u, 16u, + 12u, attrs_location_2), + create(Sym("a"), create(), 1u, 16u, 4u, 4u, type::StructMemberAttributes{})}, 4u /* align */, 8u /* size */, 16u /* size_no_padding */); diff --git a/src/tint/type/type_test.cc b/src/tint/type/type_test.cc index a09ab85ad6..b76a43a008 100644 --- a/src/tint/type/type_test.cc +++ b/src/tint/type/type_test.cc @@ -45,11 +45,9 @@ struct TypeTest : public TestHelper { const Matrix* mat4x3_af = create(vec3_af, 4u); const Reference* ref_u32 = create(u32, builtin::AddressSpace::kPrivate, builtin::Access::kReadWrite); - const Struct* str_f32 = create(Source{}, - Sym("str_f32"), + const Struct* str_f32 = create(Sym("str_f32"), utils::Vector{ create( - /* source */ Source{}, /* name */ Sym("x"), /* type */ f32, /* index */ 0u, @@ -61,11 +59,9 @@ struct TypeTest : public TestHelper { /* align*/ 4u, /* size*/ 4u, /* size_no_padding*/ 4u); - const Struct* str_f16 = create(Source{}, - Sym("str_f16"), + const Struct* str_f16 = create(Sym("str_f16"), utils::Vector{ create( - /* source */ Source{}, /* name */ Sym("x"), /* type */ f16, /* index */ 0u, @@ -77,11 +73,9 @@ struct TypeTest : public TestHelper { /* align*/ 4u, /* size*/ 4u, /* size_no_padding*/ 4u); - Struct* str_af = create(Source{}, - Sym("str_af"), + Struct* str_af = create(Sym("str_af"), utils::Vector{ create( - /* source */ Source{}, /* name */ Sym("x"), /* type */ af, /* index */ 0u,