tint: Have ast::MemberAccessorExpression use ast::Identifier

Instead of ast::IdentifierExpression
The member is not an expression, but a name.

Fixed: tint:1257
Change-Id: I879ddf09c3e521a18cef85422bb2f8fe78cddf5b
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/118343
Reviewed-by: Dan Sinclair <dsinclair@chromium.org>
Kokoro: Ben Clayton <bclayton@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>

src/tint/ast/member_accessor_expression.cc

@@ -24,7 +24,7 @@ MemberAccessorExpression::MemberAccessorExpression(ProgramID pid,
                                                    NodeID nid,
                                                    const Source& src,
                                                    const Expression* str,
-                                                   const IdentifierExpression* mem)
+                                                   const Identifier* mem)
     : Base(pid, nid, src), structure(str), member(mem) {
     TINT_ASSERT(AST, structure);
     TINT_ASSERT_PROGRAM_IDS_EQUAL_IF_VALID(AST, structure, program_id);

src/tint/ast/member_accessor_expression.h

@@ -32,7 +32,7 @@ class MemberAccessorExpression final : public Castable<MemberAccessorExpression,
                              NodeID nid,
                              const Source& source,
                              const Expression* structure,
-                             const IdentifierExpression* member);
+                             const Identifier* member);
     /// Move constructor
     MemberAccessorExpression(MemberAccessorExpression&&);
     ~MemberAccessorExpression() override;
@@ -47,7 +47,7 @@ class MemberAccessorExpression final : public Castable<MemberAccessorExpression,
     const Expression* const structure;
 
     /// The member expression
-    const IdentifierExpression* const member;
+    const Identifier* const member;
 };
 
 }  // namespace tint::ast

src/tint/ast/member_accessor_expression_test.cc

@@ -22,7 +22,7 @@ using MemberAccessorExpressionTest = TestHelper;
 
 TEST_F(MemberAccessorExpressionTest, Creation) {
     auto* str = Expr("structure");
-    auto* mem = Expr("member");
+    auto* mem = Ident("member");
 
     auto* stmt = create<MemberAccessorExpression>(str, mem);
     EXPECT_EQ(stmt->structure, str);
@@ -31,14 +31,14 @@ TEST_F(MemberAccessorExpressionTest, Creation) {
 
 TEST_F(MemberAccessorExpressionTest, Creation_WithSource) {
     auto* stmt = create<MemberAccessorExpression>(Source{Source::Location{20, 2}},
-                                                  Expr("structure"), Expr("member"));
+                                                  Expr("structure"), Ident("member"));
     auto src = stmt->source;
     EXPECT_EQ(src.range.begin.line, 20u);
     EXPECT_EQ(src.range.begin.column, 2u);
 }
 
 TEST_F(MemberAccessorExpressionTest, IsMemberAccessor) {
-    auto* stmt = create<MemberAccessorExpression>(Expr("structure"), Expr("member"));
+    auto* stmt = create<MemberAccessorExpression>(Expr("structure"), Ident("member"));
     EXPECT_TRUE(stmt->Is<MemberAccessorExpression>());
 }
 
@@ -46,7 +46,7 @@ TEST_F(MemberAccessorExpressionTest, Assert_Null_Struct) {
     EXPECT_FATAL_FAILURE(
         {
             ProgramBuilder b;
-            b.create<MemberAccessorExpression>(nullptr, b.Expr("member"));
+            b.create<MemberAccessorExpression>(nullptr, b.Ident("member"));
         },
         "internal compiler error");
 }
@@ -65,7 +65,7 @@ TEST_F(MemberAccessorExpressionTest, Assert_DifferentProgramID_Struct) {
         {
             ProgramBuilder b1;
             ProgramBuilder b2;
-            b1.create<MemberAccessorExpression>(b2.Expr("structure"), b1.Expr("member"));
+            b1.create<MemberAccessorExpression>(b2.Expr("structure"), b1.Ident("member"));
         },
         "internal compiler error");
 }
@@ -75,7 +75,7 @@ TEST_F(MemberAccessorExpressionTest, Assert_DifferentProgramID_Member) {
         {
             ProgramBuilder b1;
             ProgramBuilder b2;
-            b1.create<MemberAccessorExpression>(b1.Expr("structure"), b2.Expr("member"));
+            b1.create<MemberAccessorExpression>(b1.Expr("structure"), b2.Ident("member"));
         },
         "internal compiler error");
 }

src/tint/ast/traverse_expressions.h

@@ -132,14 +132,10 @@ bool TraverseExpressions(const ast::Expression* root, diag::List& diags, CALLBAC
                 return true;
             },
             [&](const CallExpression* call) {
-                // TODO(crbug.com/tint/1257): Resolver breaks if we actually include
-                // the function name in the traversal. push_single(call->func);
                 push_list(call->args, p.depth + 1);
                 return true;
             },
             [&](const MemberAccessorExpression* member) {
-                // TODO(crbug.com/tint/1257): Resolver breaks if we actually include
-                // the member name in the traversal. push_pair(member->member, p.depth + 1);
                 push_single(member->structure, p.depth + 1);
                 return true;
             },

src/tint/ast/traverse_expressions_test.cc

@@ -140,12 +140,10 @@ TEST_F(TraverseExpressionsTest, DescendCallExpression) {
     }
 }
 
-// TODO(crbug.com/tint/1257): Test ignores member accessor 'member' field.
+TEST_F(TraverseExpressionsTest, DescendMemberAccessorExpression) {
-// Replace with the test below when fixed.
-TEST_F(TraverseExpressionsTest, DescendMemberIndexExpression) {
     auto* e = Expr(1_i);
-    auto* m = MemberAccessor(e, Expr("a"));
+    auto* m = MemberAccessor(e, "a");
-    auto* root = MemberAccessor(m, Expr("b"));
+    auto* root = MemberAccessor(m, "b");
     {
         std::vector<const ast::Expression*> l2r;
         TraverseExpressions<TraverseOrder::LeftToRight>(root, Diagnostics(),
@@ -166,12 +164,14 @@ TEST_F(TraverseExpressionsTest, DescendMemberIndexExpression) {
     }
 }
 
-// TODO(crbug.com/tint/1257): The correct test for DescendMemberIndexExpression.
+TEST_F(TraverseExpressionsTest, DescendMemberIndexExpression) {
-TEST_F(TraverseExpressionsTest, DISABLED_DescendMemberIndexExpression) {
+    auto* a = Expr("a");
-    auto* e = Expr(1_i);
+    auto* b = Expr("b");
-    std::vector<const ast::IdentifierExpression*> i = {Expr("a"), Expr("b")};
+    auto* c = IndexAccessor(a, b);
-    auto* m = MemberAccessor(e, i[0]);
+    auto* d = Expr("d");
-    auto* root = MemberAccessor(m, i[1]);
+    auto* e = Expr("e");
+    auto* f = IndexAccessor(d, e);
+    auto* root = IndexAccessor(c, f);
     {
         std::vector<const ast::Expression*> l2r;
         TraverseExpressions<TraverseOrder::LeftToRight>(root, Diagnostics(),
@@ -179,7 +179,7 @@ TEST_F(TraverseExpressionsTest, DISABLED_DescendMemberIndexExpression) {
                                                             l2r.push_back(expr);
                                                             return ast::TraverseAction::Descend;
                                                         });
-        EXPECT_THAT(l2r, ElementsAre(root, m, e, i[0], i[1]));
+        EXPECT_THAT(l2r, ElementsAre(root, c, a, b, f, d, e));
     }
     {
         std::vector<const ast::Expression*> r2l;
@@ -188,7 +188,7 @@ TEST_F(TraverseExpressionsTest, DISABLED_DescendMemberIndexExpression) {
                                                             r2l.push_back(expr);
                                                             return ast::TraverseAction::Descend;
                                                         });
-        EXPECT_THAT(r2l, ElementsAre(root, i[1], m, i[0], e));
+        EXPECT_THAT(r2l, ElementsAre(root, f, e, d, c, b, a));
     }
 }
 

src/tint/program_builder.h

@@ -2374,7 +2374,7 @@ class ProgramBuilder {
                                                         OBJ&& obj,
                                                         IDX&& idx) {
         return create<ast::MemberAccessorExpression>(source, Expr(std::forward<OBJ>(obj)),
-                                                     Expr(std::forward<IDX>(idx)));
+                                                     Ident(std::forward<IDX>(idx)));
     }
 
     /// @param obj the object for the member accessor expression
@@ -2383,7 +2383,7 @@ class ProgramBuilder {
     template <typename OBJ, typename IDX>
     const ast::MemberAccessorExpression* MemberAccessor(OBJ&& obj, IDX&& idx) {
         return create<ast::MemberAccessorExpression>(Expr(std::forward<OBJ>(obj)),
-                                                     Expr(std::forward<IDX>(idx)));
+                                                     Ident(std::forward<IDX>(idx)));
     }
 
     /// Creates a ast::StructMemberOffsetAttribute

src/tint/reader/spirv/function.cc

@@ -1096,8 +1096,7 @@ bool FunctionEmitter::EmitPipelineInput(std::string var_name,
                     },
                     [&](const Struct* struct_type) {
                         store_dest = builder_.MemberAccessor(
-                            store_dest,
+                            store_dest, parser_impl_.GetMemberName(*struct_type, index));
-                            builder_.Expr(parser_impl_.GetMemberName(*struct_type, index)));
                         current_type = struct_type->members[static_cast<size_t>(index)];
                     });
             }
@@ -1224,19 +1223,16 @@ bool FunctionEmitter::EmitPipelineOutput(std::string var_name,
                 Switch(
                     current_type,
                     [&](const Matrix* matrix_type) {
-                        load_source =
+                        load_source = builder_.IndexAccessor(load_source, i32(index));
-                            builder_.IndexAccessor(load_source, builder_.Expr(i32(index)));
                         current_type = ty_.Vector(matrix_type->type, matrix_type->rows);
                     },
                     [&](const Array* array_type) {
-                        load_source =
+                        load_source = builder_.IndexAccessor(load_source, i32(index));
-                            builder_.IndexAccessor(load_source, builder_.Expr(i32(index)));
                         current_type = array_type->type->UnwrapAlias();
                     },
                     [&](const Struct* struct_type) {
                         load_source = builder_.MemberAccessor(
-                            load_source,
+                            load_source, parser_impl_.GetMemberName(*struct_type, index));
-                            builder_.Expr(parser_impl_.GetMemberName(*struct_type, index)));
                         current_type = struct_type->members[static_cast<size_t>(index)];
                     });
             }
@@ -4290,23 +4286,23 @@ TypedExpression FunctionEmitter::EmitGlslStd450MatrixInverse(
     return {};
 }
 
-const ast::IdentifierExpression* FunctionEmitter::Swizzle(uint32_t i) {
+const ast::Identifier* FunctionEmitter::Swizzle(uint32_t i) {
     if (i >= kMaxVectorLen) {
         Fail() << "vector component index is larger than " << kMaxVectorLen - 1 << ": " << i;
         return nullptr;
     }
     const char* names[] = {"x", "y", "z", "w"};
-    return builder_.Expr(names[i & 3]);
+    return builder_.Ident(names[i & 3]);
 }
 
-const ast::IdentifierExpression* FunctionEmitter::PrefixSwizzle(uint32_t n) {
+const ast::Identifier* FunctionEmitter::PrefixSwizzle(uint32_t n) {
     switch (n) {
         case 1:
-            return builder_.Expr("x");
+            return builder_.Ident("x");
         case 2:
-            return builder_.Expr("xy");
+            return builder_.Ident("xy");
         case 3:
-            return builder_.Expr("xyz");
+            return builder_.Ident("xyz");
         default:
             break;
     }
@@ -4510,10 +4506,8 @@ TypedExpression FunctionEmitter::MakeAccessChain(const spvtools::opt::Instructio
                     return {};
                 }
                 auto name = namer_.GetMemberName(pointee_type_id, uint32_t(index_const_val));
-                auto* member_access = builder_.Expr(name);
 
-                next_expr = create<ast::MemberAccessorExpression>(Source{}, current_expr.expr,
+                next_expr = builder_.MemberAccessor(Source{}, current_expr.expr, name);
-                                                                  member_access);
                 pointee_type_id = pointee_type_inst->GetSingleWordInOperand(
                     static_cast<uint32_t>(index_const_val));
                 break;
@@ -4673,10 +4667,8 @@ TypedExpression FunctionEmitter::MakeCompositeValueDecomposition(
                     return {};
                 }
                 auto name = namer_.GetMemberName(current_type_id, uint32_t(index_val));
-                auto* member_access = builder_.Expr(name);
 
-                next_expr = create<ast::MemberAccessorExpression>(Source{}, current_expr.expr,
+                next_expr = builder_.MemberAccessor(Source{}, current_expr.expr, name);
-                                                                  member_access);
                 current_type_id = current_type_inst->GetSingleWordInOperand(index_val);
                 break;
             }
@@ -6122,9 +6114,7 @@ TypedExpression FunctionEmitter::MakeArrayLength(const spvtools::opt::Instructio
     if (member_expr.type->Is<Pointer>()) {
         member_expr = Dereference(member_expr);
     }
-    auto* member_ident = builder_.Expr(field_name);
+    auto* member_access = builder_.MemberAccessor(Source{}, member_expr.expr, field_name);
-    auto* member_access =
-        create<ast::MemberAccessorExpression>(Source{}, member_expr.expr, member_ident);
 
     // Generate the builtin function call.
     auto* call_expr = builder_.Call(Source{}, "arrayLength", builder_.AddressOf(member_access));

src/tint/reader/spirv/function.h

@@ -916,14 +916,14 @@ class FunctionEmitter {
     /// index is out of range, i.e. 4 or higher.
     /// @param i index of the subcomponent
     /// @returns the identifier expression for the `i`'th component
-    const ast::IdentifierExpression* Swizzle(uint32_t i);
+    const ast::Identifier* Swizzle(uint32_t i);
 
     /// Returns an identifier expression for the swizzle name of the first
     /// `n` elements of a vector.  Emits an error and returns nullptr if `n`
     /// is out of range, i.e. 4 or higher.
     /// @param n the number of components in the swizzle
     /// @returns the swizzle identifier for the first n elements of a vector
-    const ast::IdentifierExpression* PrefixSwizzle(uint32_t n);
+    const ast::Identifier* PrefixSwizzle(uint32_t n);
 
     /// Converts SPIR-V image coordinates from an image access instruction
     /// (e.g. OpImageSampledImplicitLod) into an expression list consisting of

src/tint/reader/spirv/parser_impl_test_helper.cc

@@ -51,26 +51,32 @@ std::string ToString(const Program& program, utils::VectorRef<const ast::Stateme
 
 std::string ToString(const Program& program, const ast::Node* node) {
     writer::wgsl::GeneratorImpl writer(&program);
-    if (auto* expr = node->As<ast::Expression>()) {
+    return Switch(
-        std::stringstream out;
+        node,
-        if (!writer.EmitExpression(out, expr)) {
+        [&](const ast::Expression* expr) {
-            return "WGSL writer error: " + writer.error();
+            std::stringstream out;
-        }
+            if (!writer.EmitExpression(out, expr)) {
-        return out.str();
+                return "WGSL writer error: " + writer.error();
-    } else if (auto* stmt = node->As<ast::Statement>()) {
+            }
-        if (!writer.EmitStatement(stmt)) {
+            return out.str();
-            return "WGSL writer error: " + writer.error();
+        },
-        }
+        [&](const ast::Statement* stmt) {
-    } else if (auto* ty = node->As<ast::Type>()) {
+            if (!writer.EmitStatement(stmt)) {
-        std::stringstream out;
+                return "WGSL writer error: " + writer.error();
-        if (!writer.EmitType(out, ty)) {
+            }
-            return "WGSL writer error: " + writer.error();
+            return writer.result();
-        }
+        },
-        return out.str();
+        [&](const ast::Type* ty) {
-    } else {
+            std::stringstream out;
-        return "<unhandled AST node type " + std::string(node->TypeInfo().name) + ">";
+            if (!writer.EmitType(out, ty)) {
-    }
+                return "WGSL writer error: " + writer.error();
-    return writer.result();
+            }
+            return out.str();
+        },
+        [&](const ast::Identifier* ident) { return program.Symbols().NameFor(ident->symbol); },
+        [&](Default) {
+            return "<unhandled AST node type " + std::string(node->TypeInfo().name) + ">";
+        });
 }
 
 }  // namespace tint::reader::spirv::test

src/tint/reader/wgsl/parser_impl.cc

@@ -2707,7 +2707,7 @@ Maybe<const ast::Expression*> ParserImpl::component_or_swizzle_specifier(
             }
 
             prefix = builder_.MemberAccessor(ident.source, prefix,
-                                             builder_.Expr(ident.source, ident.value));
+                                             builder_.Ident(ident.source, ident.value));
             continue;
         }
 

src/tint/reader/wgsl/parser_impl_assignment_stmt_test.cc

@@ -71,9 +71,7 @@ TEST_F(ParserImplTest, AssignmentStmt_Parses_ToMember) {
     ASSERT_TRUE(a->lhs->Is<ast::MemberAccessorExpression>());
     auto* mem = a->lhs->As<ast::MemberAccessorExpression>();
 
-    ASSERT_TRUE(mem->member->Is<ast::IdentifierExpression>());
+    EXPECT_EQ(mem->member->symbol, p->builder().Symbols().Get("d"));
-    auto* ident_expr = mem->member->As<ast::IdentifierExpression>();
-    EXPECT_EQ(ident_expr->identifier->symbol, p->builder().Symbols().Get("d"));
 
     ASSERT_TRUE(mem->structure->Is<ast::IndexAccessorExpression>());
     auto* idx = mem->structure->As<ast::IndexAccessorExpression>();
@@ -84,20 +82,15 @@ TEST_F(ParserImplTest, AssignmentStmt_Parses_ToMember) {
 
     ASSERT_TRUE(idx->object->Is<ast::MemberAccessorExpression>());
     mem = idx->object->As<ast::MemberAccessorExpression>();
-    ASSERT_TRUE(mem->member->Is<ast::IdentifierExpression>());
+    EXPECT_EQ(mem->member->symbol, p->builder().Symbols().Get("c"));
-    ident_expr = mem->member->As<ast::IdentifierExpression>();
-    EXPECT_EQ(ident_expr->identifier->symbol, p->builder().Symbols().Get("c"));
 
     ASSERT_TRUE(mem->structure->Is<ast::MemberAccessorExpression>());
+
     mem = mem->structure->As<ast::MemberAccessorExpression>();
-
     ASSERT_TRUE(mem->structure->Is<ast::IdentifierExpression>());
-    ident_expr = mem->structure->As<ast::IdentifierExpression>();
+    auto* ident_expr = mem->structure->As<ast::IdentifierExpression>();
     EXPECT_EQ(ident_expr->identifier->symbol, p->builder().Symbols().Get("a"));
-
+    EXPECT_EQ(mem->member->symbol, p->builder().Symbols().Get("b"));
-    ASSERT_TRUE(mem->member->Is<ast::IdentifierExpression>());
-    ident_expr = mem->member->As<ast::IdentifierExpression>();
-    EXPECT_EQ(ident_expr->identifier->symbol, p->builder().Symbols().Get("b"));
 }
 
 TEST_F(ParserImplTest, AssignmentStmt_Parses_ToPhony) {

src/tint/reader/wgsl/parser_impl_increment_decrement_stmt_test.cc

@@ -90,9 +90,7 @@ TEST_F(ParserImplTest, IncrementDecrementStmt_ToMember) {
     ASSERT_TRUE(i->lhs->Is<ast::MemberAccessorExpression>());
     auto* mem = i->lhs->As<ast::MemberAccessorExpression>();
 
-    ASSERT_TRUE(mem->member->Is<ast::IdentifierExpression>());
+    EXPECT_EQ(mem->member->symbol, p->builder().Symbols().Get("d"));
-    auto* ident_expr = mem->member->As<ast::IdentifierExpression>();
-    EXPECT_EQ(ident_expr->identifier->symbol, p->builder().Symbols().Get("d"));
 
     ASSERT_TRUE(mem->structure->Is<ast::IndexAccessorExpression>());
     auto* idx = mem->structure->As<ast::IndexAccessorExpression>();
@@ -103,20 +101,15 @@ TEST_F(ParserImplTest, IncrementDecrementStmt_ToMember) {
 
     ASSERT_TRUE(idx->object->Is<ast::MemberAccessorExpression>());
     mem = idx->object->As<ast::MemberAccessorExpression>();
-    ASSERT_TRUE(mem->member->Is<ast::IdentifierExpression>());
+    EXPECT_EQ(mem->member->symbol, p->builder().Symbols().Get("c"));
-    ident_expr = mem->member->As<ast::IdentifierExpression>();
-    EXPECT_EQ(ident_expr->identifier->symbol, p->builder().Symbols().Get("c"));
 
     ASSERT_TRUE(mem->structure->Is<ast::MemberAccessorExpression>());
     mem = mem->structure->As<ast::MemberAccessorExpression>();
 
     ASSERT_TRUE(mem->structure->Is<ast::IdentifierExpression>());
-    ident_expr = mem->structure->As<ast::IdentifierExpression>();
+    auto* ident_expr = mem->structure->As<ast::IdentifierExpression>();
     EXPECT_EQ(ident_expr->identifier->symbol, p->builder().Symbols().Get("a"));
-
+    EXPECT_EQ(mem->member->symbol, p->builder().Symbols().Get("b"));
-    ASSERT_TRUE(mem->member->Is<ast::IdentifierExpression>());
-    ident_expr = mem->member->As<ast::IdentifierExpression>();
-    EXPECT_EQ(ident_expr->identifier->symbol, p->builder().Symbols().Get("b"));
 }
 
 TEST_F(ParserImplTest, IncrementDecrementStmt_InvalidLHS) {

src/tint/reader/wgsl/parser_impl_lhs_expression_test.cc

@@ -147,10 +147,7 @@ TEST_F(ParserImplTest, LHSExpression_PostfixExpression) {
 
     auto* struct_ident = access->structure->As<ast::IdentifierExpression>();
     EXPECT_EQ(struct_ident->identifier->symbol, p->builder().Symbols().Get("a"));
-
+    EXPECT_EQ(access->member->symbol, p->builder().Symbols().Get("foo"));
-    ASSERT_TRUE(access->member->Is<ast::IdentifierExpression>());
-    auto* member_ident = access->member->As<ast::IdentifierExpression>();
-    EXPECT_EQ(member_ident->identifier->symbol, p->builder().Symbols().Get("foo"));
 }
 
 TEST_F(ParserImplTest, LHSExpression_InvalidPostfixExpression) {

src/tint/reader/wgsl/parser_impl_singular_expression_test.cc

@@ -167,7 +167,7 @@ TEST_F(ParserImplTest, SingularExpression_MemberAccessor) {
     EXPECT_EQ(m->structure->As<ast::IdentifierExpression>()->identifier->symbol,
               p->builder().Symbols().Get("a"));
 
-    EXPECT_EQ(m->member->identifier->symbol, p->builder().Symbols().Get("b"));
+    EXPECT_EQ(m->member->symbol, p->builder().Symbols().Get("b"));
 }
 
 TEST_F(ParserImplTest, SingularExpression_MemberAccesssor_InvalidIdent) {

src/tint/resolver/const_eval_binary_op_test.cc

@@ -2151,7 +2151,7 @@ TEST_F(ResolverConstEvalTest, ShortCircuit_And_Error_MemberAccess) {
     GlobalConst("s", Construct(ty.type_name("S"), Expr(1_a), Expr(2.0_a)));
     GlobalConst("one", Expr(1_a));
     auto* lhs = Equal("one", 0_a);
-    auto* rhs = Equal(MemberAccessor(Source{{12, 34}}, "s", Expr("c")), 0_a);
+    auto* rhs = Equal(MemberAccessor(Source{{12, 34}}, "s", "c"), 0_a);
     GlobalConst("result", LogicalAnd(lhs, rhs));
 
     EXPECT_FALSE(r()->Resolve());
@@ -2170,7 +2170,7 @@ TEST_F(ResolverConstEvalTest, ShortCircuit_Or_Error_MemberAccess) {
     GlobalConst("s", Construct(ty.type_name("S"), Expr(1_a), Expr(2.0_a)));
     GlobalConst("one", Expr(1_a));
     auto* lhs = Equal("one", 1_a);
-    auto* rhs = Equal(MemberAccessor(Source{{12, 34}}, "s", Expr("c")), 0_a);
+    auto* rhs = Equal(MemberAccessor(Source{{12, 34}}, "s", "c"), 0_a);
     GlobalConst("result", LogicalOr(lhs, rhs));
 
     EXPECT_FALSE(r()->Resolve());
@@ -2189,7 +2189,7 @@ TEST_F(ResolverConstEvalTest, ShortCircuit_And_Error_Swizzle) {
     // const result = (one == 0) && (vec2(1, 2).z == 0);
     GlobalConst("one", Expr(1_a));
     auto* lhs = Equal("one", 0_a);
-    auto* rhs = Equal(MemberAccessor(vec2<AInt>(1_a, 2_a), Expr(Source{{12, 34}}, "z")), 0_a);
+    auto* rhs = Equal(MemberAccessor(vec2<AInt>(1_a, 2_a), Ident(Source{{12, 34}}, "z")), 0_a);
     GlobalConst("result", LogicalAnd(lhs, rhs));
 
     EXPECT_FALSE(r()->Resolve());
@@ -2201,7 +2201,7 @@ TEST_F(ResolverConstEvalTest, ShortCircuit_Or_Error_Swizzle) {
     // const result = (one == 1) || (vec2(1, 2).z == 0);
     GlobalConst("one", Expr(1_a));
     auto* lhs = Equal("one", 1_a);
-    auto* rhs = Equal(MemberAccessor(vec2<AInt>(1_a, 2_a), Expr(Source{{12, 34}}, "z")), 0_a);
+    auto* rhs = Equal(MemberAccessor(vec2<AInt>(1_a, 2_a), Ident(Source{{12, 34}}, "z")), 0_a);
     GlobalConst("result", LogicalOr(lhs, rhs));
 
     EXPECT_FALSE(r()->Resolve());

src/tint/resolver/resolver.cc

@@ -2767,12 +2767,11 @@ sem::Expression* Resolver::MemberAccessor(const ast::MemberAccessorExpression* e
     bool has_side_effects = object && object->HasSideEffects();
 
     Mark(expr->member);
-    Mark(expr->member->identifier);
 
     return Switch(
         storage_ty,  //
         [&](const sem::Struct* str) -> sem::Expression* {
-            auto symbol = expr->member->identifier->symbol;
+            auto symbol = expr->member->symbol;
 
             const sem::StructMember* member = nullptr;
             for (auto* m : str->Members()) {
@@ -2805,7 +2804,7 @@ sem::Expression* Resolver::MemberAccessor(const ast::MemberAccessorExpression* e
         },
 
         [&](const type::Vector* vec) -> sem::Expression* {
-            std::string s = builder_->Symbols().NameFor(expr->member->identifier->symbol);
+            std::string s = builder_->Symbols().NameFor(expr->member->symbol);
             auto size = s.size();
             utils::Vector<uint32_t, 4> swizzle;
             swizzle.Reserve(s.size());

src/tint/resolver/validation_test.cc

@@ -358,9 +358,7 @@ TEST_F(ResolverValidationTest, AddressSpace_TextureExplicitAddressSpace) {
 TEST_F(ResolverValidationTest, Expr_MemberAccessor_VectorSwizzle_BadChar) {
     GlobalVar("my_vec", ty.vec3<f32>(), type::AddressSpace::kPrivate);
 
-    auto* ident = Expr(Source{{{3, 3}, {3, 7}}}, "xyqz");
+    auto* mem = MemberAccessor("my_vec", Ident(Source{{{3, 3}, {3, 7}}}, "xyqz"));
-
-    auto* mem = MemberAccessor("my_vec", ident);
     WrapInFunction(mem);
 
     EXPECT_FALSE(r()->Resolve());
@@ -370,9 +368,7 @@ TEST_F(ResolverValidationTest, Expr_MemberAccessor_VectorSwizzle_BadChar) {
 TEST_F(ResolverValidationTest, Expr_MemberAccessor_VectorSwizzle_MixedChars) {
     GlobalVar("my_vec", ty.vec4<f32>(), type::AddressSpace::kPrivate);
 
-    auto* ident = Expr(Source{{{3, 3}, {3, 7}}}, "rgyw");
+    auto* mem = MemberAccessor("my_vec", Ident(Source{{{3, 3}, {3, 7}}}, "rgyw"));
-
-    auto* mem = MemberAccessor("my_vec", ident);
     WrapInFunction(mem);
 
     EXPECT_FALSE(r()->Resolve());
@@ -383,8 +379,7 @@ TEST_F(ResolverValidationTest, Expr_MemberAccessor_VectorSwizzle_MixedChars) {
 TEST_F(ResolverValidationTest, Expr_MemberAccessor_VectorSwizzle_BadLength) {
     GlobalVar("my_vec", ty.vec3<f32>(), type::AddressSpace::kPrivate);
 
-    auto* ident = Expr(Source{{{3, 3}, {3, 8}}}, "zzzzz");
+    auto* mem = MemberAccessor("my_vec", Ident(Source{{{3, 3}, {3, 8}}}, "zzzzz"));
-    auto* mem = MemberAccessor("my_vec", ident);
     WrapInFunction(mem);
 
     EXPECT_FALSE(r()->Resolve());
@@ -394,8 +389,7 @@ TEST_F(ResolverValidationTest, Expr_MemberAccessor_VectorSwizzle_BadLength) {
 TEST_F(ResolverValidationTest, Expr_MemberAccessor_VectorSwizzle_BadIndex) {
     GlobalVar("my_vec", ty.vec2<f32>(), type::AddressSpace::kPrivate);
 
-    auto* ident = Expr(Source{{3, 3}}, "z");
+    auto* mem = MemberAccessor("my_vec", Ident(Source{{3, 3}}, "z"));
-    auto* mem = MemberAccessor("my_vec", ident);
     WrapInFunction(mem);
 
     EXPECT_FALSE(r()->Resolve());
@@ -406,10 +400,9 @@ TEST_F(ResolverValidationTest, Expr_MemberAccessor_BadParent) {
     // var param: vec4<f32>
     // let ret: f32 = *(&param).x;
     auto* param = Var("param", ty.vec4<f32>());
-    auto* x = Expr(Source{{12, 34}}, "x");
 
     auto* addressOf_expr = AddressOf(param);
-    auto* accessor_expr = MemberAccessor(addressOf_expr, x);
+    auto* accessor_expr = MemberAccessor(addressOf_expr, Ident(Source{{12, 34}}, "x"));
     auto* star_p = Deref(accessor_expr);
     auto* ret = Var("r", ty.f32(), star_p);
     WrapInFunction(Decl(param), Decl(ret));
@@ -427,8 +420,7 @@ TEST_F(ResolverValidationTest, EXpr_MemberAccessor_FuncGoodParent) {
     // }
     auto* p = Param("p", ty.pointer(ty.vec4<f32>(), type::AddressSpace::kFunction));
     auto* star_p = Deref(p);
-    auto* z = Expr("z");
+    auto* accessor_expr = MemberAccessor(star_p, "z");
-    auto* accessor_expr = MemberAccessor(star_p, z);
     auto* x = Var("x", ty.f32(), accessor_expr);
     Func("func", utils::Vector{p}, ty.f32(),
          utils::Vector{
@@ -444,8 +436,7 @@ TEST_F(ResolverValidationTest, EXpr_MemberAccessor_FuncBadParent) {
     //     return x;
     // }
     auto* p = Param("p", ty.pointer(ty.vec4<f32>(), type::AddressSpace::kFunction));
-    auto* z = Expr(Source{{12, 34}}, "z");
+    auto* accessor_expr = MemberAccessor(p, Ident(Source{{12, 34}}, "z"));
-    auto* accessor_expr = MemberAccessor(p, z);
     auto* star_p = Deref(accessor_expr);
     auto* x = Var("x", ty.f32(), star_p);
     Func("func", utils::Vector{p}, ty.f32(),

src/tint/transform/canonicalize_entry_point_io.cc

@@ -439,7 +439,7 @@ struct CanonicalizeEntryPointIO::State {
     const ast::Expression* GLPosition(const char* component) {
         Symbol pos = ctx.dst->Symbols().Register("gl_Position");
         Symbol c = ctx.dst->Symbols().Register(component);
-        return ctx.dst->MemberAccessor(ctx.dst->Expr(pos), ctx.dst->Expr(c));
+        return ctx.dst->MemberAccessor(ctx.dst->Expr(pos), c);
     }
 
     /// Create the wrapper function's struct parameter and type objects.

src/tint/transform/decompose_memory_access.cc

@@ -915,7 +915,7 @@ Transform::ApplyResult DecomposeMemoryAccess::Apply(const Program* src,
             } else {
                 if (auto access = state.TakeAccess(accessor->structure)) {
                     auto* str_ty = access.type->As<sem::Struct>();
-                    auto* member = str_ty->FindMember(accessor->member->identifier->symbol);
+                    auto* member = str_ty->FindMember(accessor->member->symbol);
                     auto offset = member->Offset();
                     state.AddAccess(accessor, {
                                                   access.var,

src/tint/transform/num_workgroups_from_uniform.cc

@@ -177,7 +177,7 @@ Transform::ApplyResult NumWorkgroupsFromUniform::Apply(const Program* src,
             continue;
         }
 
-        if (to_replace.count({ident->identifier->symbol, accessor->member->identifier->symbol})) {
+        if (to_replace.count({ident->identifier->symbol, accessor->member->symbol})) {
             ctx.Replace(accessor, b.MemberAccessor(get_ubo()->symbol, kNumWorkgroupsMemberName));
         }
     }

src/tint/transform/promote_side_effects_to_decl.cc

@@ -134,7 +134,7 @@ class DecomposeSideEffects::CollectHoistsState : public StateBase {
                 return false;
             },
             [&](const ast::MemberAccessorExpression* e) {
-                if (HasSideEffects(e->structure) || HasSideEffects(e->member)) {
+                if (HasSideEffects(e->structure)) {
                     return true;
                 }
                 no_side_effects.insert(e);
@@ -216,7 +216,7 @@ class DecomposeSideEffects::CollectHoistsState : public StateBase {
             return false;
         };
 
-        auto binary_process = [&](auto* lhs, auto* rhs) {
+        auto binary_process = [&](const ast::Expression* lhs, const ast::Expression* rhs) {
             // If neither side causes side-effects, but at least one receives them,
             // let parent node hoist. This avoids over-hoisting side-effect receivers
             // of compound binary expressions (e.g. for "((a && b) && c) && f()", we
@@ -236,7 +236,8 @@ class DecomposeSideEffects::CollectHoistsState : public StateBase {
             return false;
         };
 
-        auto accessor_process = [&](auto* lhs, auto* rhs) {
+        auto accessor_process = [&](const ast::Expression* lhs,
+                                    const ast::Expression* rhs = nullptr) {
             auto maybe = process(lhs);
             // If lhs is a variable, let parent node hoist otherwise flush it right
             // away. This is to avoid over-hoisting the lhs of accessor chains (e.g.
@@ -247,7 +248,9 @@ class DecomposeSideEffects::CollectHoistsState : public StateBase {
                 Flush(maybe_hoist);
                 maybe = false;
             }
-            default_process(rhs);
+            if (rhs) {
+                default_process(rhs);
+            }
             return maybe;
         };
 
@@ -320,9 +323,7 @@ class DecomposeSideEffects::CollectHoistsState : public StateBase {
             [&](const ast::IndexAccessorExpression* e) {
                 return accessor_process(e->object, e->index);
             },
-            [&](const ast::MemberAccessorExpression* e) {
+            [&](const ast::MemberAccessorExpression* e) { return accessor_process(e->structure); },
-                return accessor_process(e->structure, e->member);
-            },
             [&](const ast::LiteralExpression*) {
                 // Leaf
                 return false;
@@ -519,7 +520,6 @@ class DecomposeSideEffects::DecomposeState : public StateBase {
             },
             [&](const ast::MemberAccessorExpression* member) {
                 ctx.Replace(member->structure, decompose(member->structure));
-                ctx.Replace(member->member, decompose(member->member));
                 return clone_maybe_hoisted(member);
             },
             [&](const ast::UnaryOpExpression* unary) {

src/tint/transform/renamer.cc

@@ -1287,11 +1287,11 @@ Transform::ApplyResult Renamer::Apply(const Program* src,
             [&](const ast::MemberAccessorExpression* accessor) {
                 auto* sem = src->Sem().Get(accessor)->UnwrapLoad();
                 if (sem->Is<sem::Swizzle>()) {
-                    preserved_identifiers.Add(accessor->member->identifier);
+                    preserved_identifiers.Add(accessor->member);
                 } else if (auto* str_expr = src->Sem().Get(accessor->structure)) {
                     if (auto* ty = str_expr->Type()->UnwrapRef()->As<sem::Struct>()) {
                         if (ty->Declaration() == nullptr) {  // Builtin structure
-                            preserved_identifiers.Add(accessor->member->identifier);
+                            preserved_identifiers.Add(accessor->member);
                         }
                     }
                 }

src/tint/transform/spirv_atomic.cc

@@ -90,7 +90,7 @@ struct SpirvAtomic::State {
                         auto old_value_decl = b.Decl(b.Let(
                             old_value,
                             b.MemberAccessor(b.Call(sem::str(stub->builtin), std::move(out_args)),
-                                             b.Expr("old_value"))));
+                                             "old_value")));
                         ctx.InsertBefore(block->statements, call->Stmt()->Declaration(),
                                          old_value_decl);
                         ctx.Replace(call->Declaration(), b.Expr(old_value));

src/tint/writer/glsl/generator_impl.cc

@@ -2731,7 +2731,7 @@ bool GeneratorImpl::EmitMemberAccessor(std::ostream& out,
         sem,
         [&](const sem::Swizzle*) {
             // Swizzles output the name directly
-            out << builder_.Symbols().NameFor(expr->member->identifier->symbol);
+            out << builder_.Symbols().NameFor(expr->member->symbol);
             return true;
         },
         [&](const sem::StructMemberAccess* member_access) {

src/tint/writer/hlsl/generator_impl.cc

@@ -3753,7 +3753,7 @@ bool GeneratorImpl::EmitMemberAccessor(std::ostream& out,
         sem,
         [&](const sem::Swizzle*) {
             // Swizzles output the name directly
-            out << builder_.Symbols().NameFor(expr->member->identifier->symbol);
+            out << builder_.Symbols().NameFor(expr->member->symbol);
             return true;
         },
         [&](const sem::StructMemberAccess* member_access) {

src/tint/writer/msl/generator_impl.cc

@@ -2369,7 +2369,7 @@ bool GeneratorImpl::EmitMemberAccessor(std::ostream& out,
                 if (!write_lhs()) {
                     return false;
                 }
-                out << ")." << program_->Symbols().NameFor(expr->member->identifier->symbol);
+                out << ")." << program_->Symbols().NameFor(expr->member->symbol);
             }
             return true;
         },

src/tint/writer/wgsl/generator_impl.cc

@@ -217,9 +217,8 @@ bool GeneratorImpl::EmitMemberAccessor(std::ostream& out,
         out << ")";
     }
 
-    out << ".";
+    out << "." << program_->Symbols().NameFor(expr->member->symbol);
-
+    return true;
-    return EmitExpression(out, expr->member);
 }
 
 bool GeneratorImpl::EmitBitcast(std::ostream& out, const ast::BitcastExpression* expr) {