From 6d60c046e80acbe10c002d4322e0a20a58b9a416 Mon Sep 17 00:00:00 2001
From: Ben Clayton <bclayton@google.com>
Date: Mon, 23 Aug 2021 19:01:19 +0000
Subject: [PATCH] resolver: Validate that parameter names are unique

Fixed: chromium:1242330
Change-Id: I7a5c32f55b5a4d5898c1fe4efeee084dfeb15346
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/62444
Auto-Submit: Ben Clayton <bclayton@google.com>
Commit-Queue: James Price <jrprice@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
---
 src/resolver/function_validation_test.cc | 34 +++++++++++++++++++++---
 src/resolver/resolver.cc                 |  5 ++++
 2 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/src/resolver/function_validation_test.cc b/src/resolver/function_validation_test.cc
index 5aa7bac5e6..3cd7d01ff4 100644
--- a/src/resolver/function_validation_test.cc
+++ b/src/resolver/function_validation_test.cc
@@ -28,13 +28,13 @@ class ResolverFunctionValidationTest : public resolver::TestHelper,
 TEST_F(ResolverFunctionValidationTest, FunctionNamesMustBeUnique_fail) {
   // fn func -> i32 { return 2; }
   // fn func -> i32 { return 2; }
-  Func("func", ast::VariableList{}, ty.i32(),
+  Func(Source{{56, 78}}, "func", ast::VariableList{}, ty.i32(),
        ast::StatementList{
            Return(2),
        },
        ast::DecorationList{});
 
-  Func(Source{Source::Location{12, 34}}, "func", ast::VariableList{}, ty.i32(),
+  Func(Source{{12, 34}}, "func", ast::VariableList{}, ty.i32(),
        ast::StatementList{
            Return(2),
        },
@@ -42,8 +42,34 @@ TEST_F(ResolverFunctionValidationTest, FunctionNamesMustBeUnique_fail) {
 
   EXPECT_FALSE(r()->Resolve());
   EXPECT_EQ(r()->error(),
-            "12:34 error: redefinition of 'func'\nnote: previous definition "
-            "is here");
+            R"(12:34 error: redefinition of 'func'
+56:78 note: previous definition is here)");
+}
+
+TEST_F(ResolverFunctionValidationTest, ParameterNamesMustBeUnique_fail) {
+  // fn func(common_name : f32, x : i32, common_name : u32) { }
+  Func("func",
+       {
+           Param(Source{{56, 78}}, "common_name", ty.f32()),
+           Param("x", ty.i32()),
+           Param(Source{{12, 34}}, "common_name", ty.u32()),
+       },
+       ty.void_(), {});
+
+  EXPECT_FALSE(r()->Resolve());
+  EXPECT_EQ(r()->error(),
+            R"(12:34 error: redefinition of 'common_name'
+56:78 note: previous definition is here)");
+}
+
+TEST_F(ResolverFunctionValidationTest, ParameterNamesMustBeUnique_pass) {
+  // fn func_a(common_name : f32) { }
+  // fn func_b(common_name : f32) { }
+  Func("func_a", {Param("common_name", ty.f32())}, ty.void_(), {});
+  Func("func_b", {Param("common_name", ty.f32())}, ty.void_(), {});
+
+  EXPECT_TRUE(r()->Resolve());
+  EXPECT_EQ(r()->error(), "");
 }
 
 TEST_F(ResolverFunctionValidationTest,
diff --git a/src/resolver/resolver.cc b/src/resolver/resolver.cc
index 8f3206b54f..3115afb16a 100644
--- a/src/resolver/resolver.cc
+++ b/src/resolver/resolver.cc
@@ -1698,6 +1698,11 @@ bool Resolver::Function(ast::Function* func) {
   uint32_t parameter_index = 0;
   for (auto* param : func->params()) {
     Mark(param);
+
+    if (!ValidateNoDuplicateDefinition(param->symbol(), param->source())) {
+      return false;
+    }
+
     auto* param_info =
         Variable(param, VariableKind::kParameter, parameter_index++);
     if (!param_info) {