tint/ast: Change StructMember::symbol to StructMember::name

StructMember::name is an ast::Identifier.

The goal here is to have all AST nodes use an identifier instead of
symbols directly. This will greatly simplify the renamer transform,
and gives the symbol a Source location, which is helpful for
diagnostics and tooling.

Change-Id: I1156653a48b02997ec0a9077b174401f65a13457
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/119281
Kokoro: Ben Clayton <bclayton@google.com>
Reviewed-by: Dan Sinclair <dsinclair@chromium.org>

src/tint/ast/struct_member.cc

@@ -23,13 +23,16 @@ namespace tint::ast {
 StructMember::StructMember(ProgramID pid,
                            NodeID nid,
                            const Source& src,
-                           const Symbol& sym,
+                           const Identifier* n,
                            const ast::Type* ty,
                            utils::VectorRef<const Attribute*> attrs)
-    : Base(pid, nid, src), symbol(sym), type(ty), attributes(std::move(attrs)) {
+
+    : Base(pid, nid, src), name(n), type(ty), attributes(std::move(attrs)) {
+    TINT_ASSERT(AST, name);
+    if (name) {
+        TINT_ASSERT(AST, !name->Is<TemplatedIdentifier>());
+    }
     TINT_ASSERT(AST, type);
-    TINT_ASSERT(AST, symbol.IsValid());
-    TINT_ASSERT_PROGRAM_IDS_EQUAL_IF_VALID(AST, symbol, program_id);
     for (auto* attr : attributes) {
         TINT_ASSERT(AST, attr);
         TINT_ASSERT_PROGRAM_IDS_EQUAL_IF_VALID(AST, attr, program_id);
@@ -43,10 +46,10 @@ StructMember::~StructMember() = default;
 const StructMember* StructMember::Clone(CloneContext* ctx) const {
     // Clone arguments outside of create() call to have deterministic ordering
     auto src = ctx->Clone(source);
-    auto sym = ctx->Clone(symbol);
+    auto n = ctx->Clone(name);
     auto* ty = ctx->Clone(type);
     auto attrs = ctx->Clone(attributes);
-    return ctx->dst->create<StructMember>(src, sym, ty, std::move(attrs));
+    return ctx->dst->create<StructMember>(src, n, ty, std::move(attrs));
 }
 
 }  // namespace tint::ast

src/tint/ast/struct_member.h

@@ -21,6 +21,7 @@
 
 // Forward declarations
 namespace tint::ast {
+class Identifier;
 class Type;
 }  // namespace tint::ast
 
@@ -33,13 +34,13 @@ class StructMember final : public Castable<StructMember, Node> {
     /// @param pid the identifier of the program that owns this node
     /// @param nid the unique node identifier
     /// @param src the source of this node for the struct member statement
-    /// @param sym The struct member symbol
+    /// @param name The struct member name
     /// @param type The struct member type
     /// @param attributes The struct member attributes
     StructMember(ProgramID pid,
                  NodeID nid,
                  const Source& src,
-                 const Symbol& sym,
+                 const Identifier* name,
                  const ast::Type* type,
                  utils::VectorRef<const Attribute*> attributes);
     /// Move constructor
@@ -53,8 +54,8 @@ class StructMember final : public Castable<StructMember, Node> {
     /// @return the newly cloned node
     const StructMember* Clone(CloneContext* ctx) const override;
 
-    /// The symbol
+    /// The member name
-    const Symbol symbol;
+    const Identifier* const name;
 
     /// The type
     const ast::Type* const type;

src/tint/ast/struct_member_test.cc

@@ -23,7 +23,7 @@ using StructMemberTest = TestHelper;
 
 TEST_F(StructMemberTest, Creation) {
     auto* st = Member("a", ty.i32(), utils::Vector{MemberSize(4_a)});
-    EXPECT_EQ(st->symbol, Symbol(1, ID()));
+    EXPECT_EQ(st->name->symbol, Symbol(1, ID()));
     EXPECT_TRUE(st->type->Is<ast::I32>());
     EXPECT_EQ(st->attributes.Length(), 1u);
     EXPECT_TRUE(st->attributes[0]->Is<StructMemberSizeAttribute>());
@@ -36,7 +36,7 @@ TEST_F(StructMemberTest, Creation) {
 TEST_F(StructMemberTest, CreationWithSource) {
     auto* st = Member(Source{Source::Range{Source::Location{27, 4}, Source::Location{27, 8}}}, "a",
                       ty.i32());
-    EXPECT_EQ(st->symbol, Symbol(1, ID()));
+    EXPECT_EQ(st->name->symbol, Symbol(1, ID()));
     EXPECT_TRUE(st->type->Is<ast::I32>());
     EXPECT_EQ(st->attributes.Length(), 0u);
     EXPECT_EQ(st->source.range.begin.line, 27u);
@@ -45,11 +45,11 @@ TEST_F(StructMemberTest, CreationWithSource) {
     EXPECT_EQ(st->source.range.end.column, 8u);
 }
 
-TEST_F(StructMemberTest, Assert_Empty_Symbol) {
+TEST_F(StructMemberTest, Assert_Null_Name) {
     EXPECT_FATAL_FAILURE(
         {
             ProgramBuilder b;
-            b.Member("", b.ty.i32());
+            b.Member(static_cast<Identifier*>(nullptr), b.ty.i32());
         },
         "internal compiler error");
 }

src/tint/program_builder.h

@@ -2644,6 +2644,19 @@ class ProgramBuilder {
         return type;
     }
 
+    /// Creates a ast::StructMember
+    /// @param name the struct member name
+    /// @param type the struct member type
+    /// @param attributes the optional struct member attributes
+    /// @returns the struct member pointer
+    template <typename NAME>
+    const ast::StructMember* Member(
+        NAME&& name,
+        const ast::Type* type,
+        utils::VectorRef<const ast::Attribute*> attributes = utils::Empty) {
+        return Member(source_, std::forward<NAME>(name), type, std::move(attributes));
+    }
+
     /// Creates a ast::StructMember
     /// @param source the source information
     /// @param name the struct member name
@@ -2656,21 +2669,7 @@ class ProgramBuilder {
         NAME&& name,
         const ast::Type* type,
         utils::VectorRef<const ast::Attribute*> attributes = utils::Empty) {
-        return create<ast::StructMember>(source, Sym(std::forward<NAME>(name)), type,
+        return create<ast::StructMember>(source, Ident(std::forward<NAME>(name)), type,
-                                         std::move(attributes));
-    }
-
-    /// Creates a ast::StructMember
-    /// @param name the struct member name
-    /// @param type the struct member type
-    /// @param attributes the optional struct member attributes
-    /// @returns the struct member pointer
-    template <typename NAME>
-    const ast::StructMember* Member(
-        NAME&& name,
-        const ast::Type* type,
-        utils::VectorRef<const ast::Attribute*> attributes = utils::Empty) {
-        return create<ast::StructMember>(source_, Sym(std::forward<NAME>(name)), type,
                                          std::move(attributes));
     }
 
@@ -2681,7 +2680,7 @@ class ProgramBuilder {
     /// @returns the struct member pointer
     template <typename NAME>
     const ast::StructMember* Member(uint32_t offset, NAME&& name, const ast::Type* type) {
-        return create<ast::StructMember>(source_, Sym(std::forward<NAME>(name)), type,
+        return create<ast::StructMember>(source_, Ident(std::forward<NAME>(name)), type,
                                          utils::Vector<const ast::Attribute*, 1>{
                                              MemberOffset(AInt(offset)),
                                          });

src/tint/reader/wgsl/parser_impl_struct_body_decl_test.cc

@@ -28,7 +28,7 @@ TEST_F(ParserImplTest, StructBodyDecl_Parses) {
     ASSERT_EQ(m.value.Length(), 1u);
 
     const auto* mem = m.value[0];
-    EXPECT_EQ(mem->symbol, builder.Symbols().Get("a"));
+    EXPECT_EQ(mem->name->symbol, builder.Symbols().Get("a"));
     EXPECT_TRUE(mem->type->Is<ast::I32>());
     EXPECT_EQ(mem->attributes.Length(), 0u);
 }
@@ -44,7 +44,7 @@ TEST_F(ParserImplTest, StructBodyDecl_Parses_TrailingComma) {
     ASSERT_EQ(m.value.Length(), 1u);
 
     const auto* mem = m.value[0];
-    EXPECT_EQ(mem->symbol, builder.Symbols().Get("a"));
+    EXPECT_EQ(mem->name->symbol, builder.Symbols().Get("a"));
     EXPECT_TRUE(mem->type->Is<ast::I32>());
     EXPECT_EQ(mem->attributes.Length(), 0u);
 }

src/tint/reader/wgsl/parser_impl_struct_decl_test.cc

@@ -31,8 +31,8 @@ struct S {
     ASSERT_NE(s.value, nullptr);
     ASSERT_EQ(s->name, p->builder().Symbols().Register("S"));
     ASSERT_EQ(s->members.Length(), 2u);
-    EXPECT_EQ(s->members[0]->symbol, p->builder().Symbols().Register("a"));
+    EXPECT_EQ(s->members[0]->name->symbol, p->builder().Symbols().Register("a"));
-    EXPECT_EQ(s->members[1]->symbol, p->builder().Symbols().Register("b"));
+    EXPECT_EQ(s->members[1]->name->symbol, p->builder().Symbols().Register("b"));
 }
 
 TEST_F(ParserImplTest, StructDecl_Unicode_Parses) {
@@ -65,8 +65,8 @@ struct $struct {
     ASSERT_NE(s.value, nullptr);
     ASSERT_EQ(s->name, p->builder().Symbols().Register(struct_ident));
     ASSERT_EQ(s->members.Length(), 2u);
-    EXPECT_EQ(s->members[0]->symbol, p->builder().Symbols().Register(member_a_ident));
+    EXPECT_EQ(s->members[0]->name->symbol, p->builder().Symbols().Register(member_a_ident));
-    EXPECT_EQ(s->members[1]->symbol, p->builder().Symbols().Register(member_b_ident));
+    EXPECT_EQ(s->members[1]->name->symbol, p->builder().Symbols().Register(member_b_ident));
 }
 
 TEST_F(ParserImplTest, StructDecl_EmptyMembers) {

src/tint/reader/wgsl/parser_impl_struct_member_test.cc

@@ -27,7 +27,7 @@ TEST_F(ParserImplTest, StructMember_Parses) {
     ASSERT_FALSE(m.errored);
     ASSERT_NE(m.value, nullptr);
 
-    EXPECT_EQ(m->symbol, builder.Symbols().Get("a"));
+    EXPECT_EQ(m->name->symbol, builder.Symbols().Get("a"));
     EXPECT_TRUE(m->type->Is<ast::I32>());
     EXPECT_EQ(m->attributes.Length(), 0u);
 
@@ -45,7 +45,7 @@ TEST_F(ParserImplTest, StructMember_ParsesWithAlignAttribute) {
     ASSERT_FALSE(m.errored);
     ASSERT_NE(m.value, nullptr);
 
-    EXPECT_EQ(m->symbol, builder.Symbols().Get("a"));
+    EXPECT_EQ(m->name->symbol, builder.Symbols().Get("a"));
     EXPECT_TRUE(m->type->Is<ast::I32>());
     EXPECT_EQ(m->attributes.Length(), 1u);
     EXPECT_TRUE(m->attributes[0]->Is<ast::StructMemberAlignAttribute>());
@@ -70,7 +70,7 @@ TEST_F(ParserImplTest, StructMember_ParsesWithSizeAttribute) {
     ASSERT_FALSE(m.errored);
     ASSERT_NE(m.value, nullptr);
 
-    EXPECT_EQ(m->symbol, builder.Symbols().Get("a"));
+    EXPECT_EQ(m->name->symbol, builder.Symbols().Get("a"));
     EXPECT_TRUE(m->type->Is<ast::I32>());
     EXPECT_EQ(m->attributes.Length(), 1u);
     ASSERT_TRUE(m->attributes[0]->Is<ast::StructMemberSizeAttribute>());
@@ -94,7 +94,7 @@ TEST_F(ParserImplTest, StructMember_ParsesWithMultipleattributes) {
     ASSERT_FALSE(m.errored);
     ASSERT_NE(m.value, nullptr);
 
-    EXPECT_EQ(m->symbol, builder.Symbols().Get("a"));
+    EXPECT_EQ(m->name->symbol, builder.Symbols().Get("a"));
     EXPECT_TRUE(m->type->Is<ast::I32>());
     EXPECT_EQ(m->attributes.Length(), 2u);
     ASSERT_TRUE(m->attributes[0]->Is<ast::StructMemberSizeAttribute>());

src/tint/resolver/inferred_type_test.cc

@@ -152,7 +152,7 @@ TEST_F(ResolverInferredTypeTest, InferStruct_Pass) {
 
     auto* expected_type = create<sem::Struct>(
         str, str->source, str->name,
-        utils::Vector{create<sem::StructMember>(member, member->source, member->symbol,
+        utils::Vector{create<sem::StructMember>(member, member->source, member->name->symbol,
                                                 create<type::I32>(), 0u, 0u, 0u, 4u, std::nullopt)},
         0u, 4u, 4u);
 

src/tint/resolver/resolver.cc

@@ -3360,8 +3360,9 @@ sem::Struct* Resolver::Structure(const ast::Struct* str) {
 
     for (auto* member : str->members) {
         Mark(member);
-        if (auto added = member_map.Add(member->symbol, member); !added) {
+        Mark(member->name);
-            AddError("redefinition of '" + builder_->Symbols().NameFor(member->symbol) + "'",
+        if (auto added = member_map.Add(member->name->symbol, member); !added) {
+            AddError("redefinition of '" + builder_->Symbols().NameFor(member->name->symbol) + "'",
                      member->source);
             AddNote("previous definition is here", (*added.value)->source);
             return nullptr;
@@ -3518,7 +3519,7 @@ sem::Struct* Resolver::Structure(const ast::Struct* str) {
         }
 
         auto* sem_member = builder_->create<sem::StructMember>(
-            member, member->source, member->symbol, type,
+            member, member->source, member->name->symbol, type,
             static_cast<uint32_t>(sem_members.Length()), static_cast<uint32_t>(offset),
             static_cast<uint32_t>(align), static_cast<uint32_t>(size), location);
         builder_->Sem().Add(member, sem_member);

src/tint/resolver/resolver_test.cc

@@ -1253,7 +1253,7 @@ TEST_F(ResolverTest, Expr_MemberAccessor_Struct) {
     EXPECT_TRUE(sma->Member()->Type()->Is<type::F32>());
     EXPECT_EQ(sma->Object()->Declaration(), mem->object);
     EXPECT_EQ(sma->Member()->Index(), 1u);
-    EXPECT_EQ(sma->Member()->Declaration()->symbol, Symbols().Get("second_member"));
+    EXPECT_EQ(sma->Member()->Declaration()->name->symbol, Symbols().Get("second_member"));
 }
 
 TEST_F(ResolverTest, Expr_MemberAccessor_Struct_Alias) {

src/tint/transform/clamp_frag_depth.cc

@@ -170,7 +170,8 @@ Transform::ApplyResult ClampFragDepth::Apply(const Program* src, const DataMap&,
 
                 utils::Vector<const ast::Expression*, 8u> initializer_args;
                 for (auto* member : struct_ty->members) {
-                    const ast::Expression* arg = b.MemberAccessor("s", ctx.Clone(member->symbol));
+                    const ast::Expression* arg =
+                        b.MemberAccessor("s", ctx.Clone(member->name->symbol));
                     if (ContainsFragDepth(member->attributes)) {
                         arg = b.Call(base_fn_sym, arg);
                     }

src/tint/transform/preserve_padding.cc

@@ -151,7 +151,7 @@ struct PreservePadding::State {
                 return call_helper([&]() {
                     utils::Vector<const ast::Statement*, 8> body;
                     for (auto member : str->Members()) {
-                        auto name = sym.NameFor(member->Declaration()->symbol);
+                        auto name = sym.NameFor(member->Declaration()->name->symbol);
                         body.Push(MakeAssignment(member->Type(),
                                                  b.MemberAccessor(b.Deref(kDestParamName), name),
                                                  b.MemberAccessor(kValueParamName, name)));

src/tint/transform/spirv_atomic.cc

@@ -132,7 +132,7 @@ struct SpirvAtomic::State {
                         auto* member = str->members[i];
                         if (forked.atomic_members.count(i)) {
                             auto* type = AtomicTypeFor(ctx.src->Sem().Get(member)->Type());
-                            auto name = ctx.src->Symbols().NameFor(member->symbol);
+                            auto name = ctx.src->Symbols().NameFor(member->name->symbol);
                             members.Push(b.Member(name, type, ctx.Clone(member->attributes)));
                         } else {
                             members.Push(ctx.Clone(member));

src/tint/transform/std140.cc

@@ -384,7 +384,7 @@ struct Std140::State {
             bool unique = true;
             for (auto* member : str->members) {
                 // The member name must be unique over the entire set of `count` suffixed names.
-                if (strings.Contains(sym.NameFor(member->symbol))) {
+                if (strings.Contains(sym.NameFor(member->name->symbol))) {
                     unique = false;
                     break;
                 }
@@ -420,7 +420,8 @@ struct Std140::State {
                         b.Structure(name, members);
                         return Std140Matrix{
                             name,
-                            utils::Transform(members, [&](auto* member) { return member->symbol; }),
+                            utils::Transform(members,
+                                             [&](auto* member) { return member->name->symbol; }),
                         };
                     });
                     return b.ty(std140_mat.name);
@@ -704,7 +705,7 @@ struct Std140::State {
                             auto* mat_ty = CreateASTTypeFor(ctx, member->Type());
                             auto mat_args =
                                 utils::Transform(*col_members, [&](const ast::StructMember* m) {
-                                    return b.MemberAccessor(param, m->symbol);
+                                    return b.MemberAccessor(param, m->name->symbol);
                                 });
                             args.Push(b.Call(mat_ty, std::move(mat_args)));
                         } else {
@@ -838,7 +839,7 @@ struct Std140::State {
             auto mat_member_idx = std::get<u32>(chain.indices[std140_mat_idx]);
             auto* mat_member = str->Members()[mat_member_idx];
             auto mat_columns = *std140_mat_members.Get(mat_member);
-            expr = b.MemberAccessor(expr, mat_columns[column_idx]->symbol);
+            expr = b.MemberAccessor(expr, mat_columns[column_idx]->name->symbol);
             ty = mat_member->Type()->As<type::Matrix>()->ColumnType();
         } else {
             // Non-structure-member matrix. The columns are decomposed into a new, bespoke std140
@@ -921,7 +922,7 @@ struct Std140::State {
                             std::to_string(column_param_idx);
                 }
                 auto mat_columns = *std140_mat_members.Get(mat_member);
-                expr = b.MemberAccessor(expr, mat_columns[column_idx]->symbol);
+                expr = b.MemberAccessor(expr, mat_columns[column_idx]->name->symbol);
                 ty = mat_member->Type()->As<type::Matrix>()->ColumnType();
             } else {
                 // Non-structure-member matrix. The columns are decomposed into a new, bespoke
@@ -1015,7 +1016,7 @@ struct Std140::State {
             auto* mat_member = str->Members()[mat_member_idx];
             auto mat_columns = *std140_mat_members.Get(mat_member);
             columns = utils::Transform(mat_columns, [&](auto* column_member) {
-                return b.MemberAccessor(b.Deref(let), column_member->symbol);
+                return b.MemberAccessor(b.Deref(let), column_member->name->symbol);
             });
             ty = mat_member->Type();
             name += "_" + sym.NameFor(mat_member->Name());

src/tint/transform/vertex_pulling.cc

@@ -805,7 +805,7 @@ struct VertexPulling::State {
         bool has_locations = false;
         utils::Vector<const ast::StructMember*, 8> members_to_clone;
         for (auto* member : struct_ty->members) {
-            auto member_sym = ctx.Clone(member->symbol);
+            auto member_sym = ctx.Clone(member->name->symbol);
             std::function<const ast::Expression*()> member_expr = [this, param_sym, member_sym]() {
                 return b.MemberAccessor(param_sym, member_sym);
             };
@@ -851,10 +851,10 @@ struct VertexPulling::State {
             // Create a new struct without the location attributes.
             utils::Vector<const ast::StructMember*, 8> new_members;
             for (auto* member : members_to_clone) {
-                auto member_sym = ctx.Clone(member->symbol);
+                auto member_name = ctx.Clone(member->name);
                 auto* member_type = ctx.Clone(member->type);
                 auto member_attrs = ctx.Clone(member->attributes);
-                new_members.Push(b.Member(member_sym, member_type, std::move(member_attrs)));
+                new_members.Push(b.Member(member_name, member_type, std::move(member_attrs)));
             }
             auto* new_struct = b.Structure(b.Sym(), new_members);
 
@@ -864,7 +864,7 @@ struct VertexPulling::State {
 
             // Copy values from the new parameter to the function-scope variable.
             for (auto* member : members_to_clone) {
-                auto member_name = ctx.Clone(member->symbol);
+                auto member_name = ctx.Clone(member->name->symbol);
                 ctx.InsertFront(func->body->statements,
                                 b.Assign(b.MemberAccessor(func_var, member_name),
                                          b.MemberAccessor(new_param, member_name)));

src/tint/transform/zero_init_workgroup_memory.cc

@@ -172,7 +172,7 @@ struct ZeroInitWorkgroupMemory::State {
                         if (builtin->builtin == ast::BuiltinValue::kLocalInvocationIndex) {
                             local_index = [=] {
                                 auto* param_expr = b.Expr(ctx.Clone(param->symbol));
-                                auto member_name = ctx.Clone(member->Declaration()->symbol);
+                                auto* member_name = ctx.Clone(member->Declaration()->name);
                                 return b.MemberAccessor(param_expr, member_name);
                             };
                             break;
@@ -317,7 +317,7 @@ struct ZeroInitWorkgroupMemory::State {
 
         if (auto* str = ty->As<sem::Struct>()) {
             for (auto* member : str->Members()) {
-                auto name = ctx.Clone(member->Declaration()->symbol);
+                auto name = ctx.Clone(member->Declaration()->name->symbol);
                 auto get_member = [&](uint32_t num_values) {
                     auto s = get_expr(num_values);
                     if (!s) {

src/tint/writer/msl/generator_impl.cc

@@ -843,7 +843,7 @@ bool GeneratorImpl::EmitTypeInitializer(std::ostream& out,
         if (auto* struct_ty = type->As<sem::Struct>()) {
             // Emit field designators for structures to account for padding members.
             auto* member = struct_ty->Members()[i]->Declaration();
-            auto name = program_->Symbols().NameFor(member->symbol);
+            auto name = program_->Symbols().NameFor(member->name->symbol);
             out << "." << name << "=";
         }
 

src/tint/writer/wgsl/generator_impl.cc

@@ -655,7 +655,7 @@ bool GeneratorImpl::EmitStructType(const ast::Struct* str) {
         }
 
         auto out = line();
-        out << program_->Symbols().NameFor(mem->symbol) << " : ";
+        out << program_->Symbols().NameFor(mem->name->symbol) << " : ";
         if (!EmitType(out, mem->type)) {
             return false;
         }