From 0bbe24d4ce9050f0e5ffcd3598d04e5d25761da4 Mon Sep 17 00:00:00 2001
From: Sarah <sarahmashay@google.com>
Date: Mon, 19 Jul 2021 17:38:20 +0000
Subject: [PATCH] validation: struct member name must be unique

Bug: tint:964
Change-Id: I45e324f65fb6e7c20488a154510daca6ae347e47
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/58260
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: Ben Clayton <bclayton@chromium.org>
Commit-Queue: Sarah Mashayekhi <sarahmashay@google.com>
Auto-Submit: Sarah Mashayekhi <sarahmashay@google.com>
---
 src/resolver/resolver.cc                      |  9 ++++++++
 .../type_constructor_validation_test.cc       |  6 ++---
 src/resolver/validation_test.cc               | 22 +++++++++++++++++++
 3 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/src/resolver/resolver.cc b/src/resolver/resolver.cc
index 6a4d7b30f0..7c3ae1fce6 100644
--- a/src/resolver/resolver.cc
+++ b/src/resolver/resolver.cc
@@ -4003,9 +4003,18 @@ sem::Struct* Resolver::Structure(const ast::Struct* str) {
   // validation.
   uint32_t struct_size = 0;
   uint32_t struct_align = 1;
+  std::unordered_map<Symbol, ast::StructMember*> member_map;
 
   for (auto* member : str->members()) {
     Mark(member);
+    auto result = member_map.emplace(member->symbol(), member);
+    if (!result.second) {
+      AddError("redefinition of '" +
+                   builder_->Symbols().NameFor(member->symbol()) + "'",
+               member->source());
+      AddNote("previous definition is here", result.first->second->source());
+      return nullptr;
+    }
 
     // Resolve member type
     auto* type = Type(member->type());
diff --git a/src/resolver/type_constructor_validation_test.cc b/src/resolver/type_constructor_validation_test.cc
index 2f67ce0ec5..0c275b88fb 100644
--- a/src/resolver/type_constructor_validation_test.cc
+++ b/src/resolver/type_constructor_validation_test.cc
@@ -2113,9 +2113,9 @@ TEST_F(ResolverTypeConstructorValidationTest, Expr_Constructor_Struct_Nested) {
   auto* inner_m = Member("m", ty.i32());
   auto* inner_s = Structure("inner_s", {inner_m});
 
-  auto* m0 = Member("m", ty.i32());
-  auto* m1 = Member("m", ty.Of(inner_s));
-  auto* m2 = Member("m", ty.i32());
+  auto* m0 = Member("m0", ty.i32());
+  auto* m1 = Member("m1", ty.Of(inner_s));
+  auto* m2 = Member("m2", ty.i32());
   auto* s = Structure("s", {m0, m1, m2});
 
   auto* tc = create<ast::TypeConstructorExpression>(Source{{12, 34}}, ty.Of(s),
diff --git a/src/resolver/validation_test.cc b/src/resolver/validation_test.cc
index 6bb426fd74..92bd17c8fb 100644
--- a/src/resolver/validation_test.cc
+++ b/src/resolver/validation_test.cc
@@ -810,6 +810,28 @@ TEST_F(ResolverValidationTest, Stmt_BreakNotInLoopOrSwitch) {
             "12:34 error: break statement must be in a loop or switch case");
 }
 
+TEST_F(ResolverValidationTest, StructMemberDuplicateName) {
+  Structure("S",
+            {Member("a", ty.i32()), Member(Source{{12, 34}}, "a", ty.i32())});
+  EXPECT_FALSE(r()->Resolve());
+  EXPECT_EQ(
+      r()->error(),
+      "12:34 error: redefinition of 'a'\nnote: previous definition is here");
+}
+TEST_F(ResolverValidationTest, StructMemberDuplicateNameDifferentTypes) {
+  Structure("S", {Member("a", ty.bool_()),
+                  Member(Source{{12, 34}}, "a", ty.vec3<f32>())});
+  EXPECT_FALSE(r()->Resolve());
+  EXPECT_EQ(
+      r()->error(),
+      "12:34 error: redefinition of 'a'\nnote: previous definition is here");
+}
+TEST_F(ResolverValidationTest, StructMemberDuplicateNamePass) {
+  Structure("S", {Member("a", ty.i32()), Member("b", ty.f32())});
+  Structure("S1", {Member("a", ty.i32()), Member("b", ty.f32())});
+  EXPECT_TRUE(r()->Resolve());
+}
+
 TEST_F(ResolverValidationTest, NonPOTStructMemberAlignDecoration) {
   Structure("S", {
                      Member("a", ty.f32(), {MemberAlign(Source{{12, 34}}, 3)}),