From eec1a6e628e3585c48d5e08208cc22e81b7b9ab0 Mon Sep 17 00:00:00 2001
From: Sarah Mashayekhi <sarahmashay@google.com>
Date: Mon, 17 Aug 2020 15:32:38 +0000
Subject: [PATCH] [validation] Validates function name uniqueness

This CL implements and add a test for the following validation rule:
v-0016: Function names must be unique

Bug: tint: 6
Change-Id: I9f135dd577863e41f03a2d02adebe4347a9922eb
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/26782
Commit-Queue: Sarah Mashayekhi <sarahmashay@google.com>
Commit-Queue: dan sinclair <dsinclair@chromium.org>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
---
 src/validator_function_test.cc | 35 ++++++++++++++++++++++++++++++++++
 src/validator_impl.cc          | 13 ++++++++++++-
 src/validator_impl.h           |  3 +++
 3 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/src/validator_function_test.cc b/src/validator_function_test.cc
index d42b76bfcf..b220e45e60 100644
--- a/src/validator_function_test.cc
+++ b/src/validator_function_test.cc
@@ -163,5 +163,40 @@ TEST_F(ValidateFunctionTest,
       "12:34: v-000y: function type must match its return statement type");
 }
 
+TEST_F(ValidateFunctionTest, FunctionNamesMustBeUnique_fail) {
+  // fn func -> i32 { return 2; }
+  // fn func -> i32 { return 2; }
+  ast::type::VoidType void_type;
+  ast::type::I32Type i32;
+
+  ast::VariableList params;
+  auto func = std::make_unique<ast::Function>("func", std::move(params), &i32);
+  auto body = std::make_unique<ast::BlockStatement>();
+  auto return_expr = std::make_unique<ast::ScalarConstructorExpression>(
+      std::make_unique<ast::SintLiteral>(&i32, 2));
+
+  body->append(std::make_unique<ast::ReturnStatement>(std::move(return_expr)));
+  func->set_body(std::move(body));
+
+  ast::VariableList params_copy;
+  auto func_copy = std::make_unique<ast::Function>(
+      Source{12, 34}, "func", std::move(params_copy), &i32);
+  auto body_copy = std::make_unique<ast::BlockStatement>();
+  auto return_expr_copy = std::make_unique<ast::ScalarConstructorExpression>(
+      std::make_unique<ast::SintLiteral>(&i32, 2));
+
+  body_copy->append(
+      std::make_unique<ast::ReturnStatement>(std::move(return_expr_copy)));
+  func_copy->set_body(std::move(body_copy));
+
+  mod()->AddFunction(std::move(func));
+  mod()->AddFunction(std::move(func_copy));
+
+  EXPECT_TRUE(td()->Determine()) << td()->error();
+  tint::ValidatorImpl v;
+  EXPECT_FALSE(v.Validate(mod()));
+  EXPECT_EQ(v.error(), "12:34: v-0016: function names must be unique 'func'");
+}
+
 }  // namespace
 }  // namespace tint
diff --git a/src/validator_impl.cc b/src/validator_impl.cc
index b597b2c608..0bbda6c177 100644
--- a/src/validator_impl.cc
+++ b/src/validator_impl.cc
@@ -31,6 +31,7 @@ bool ValidatorImpl::Validate(const ast::Module* module) {
   if (!module) {
     return false;
   }
+  function_stack_.push_scope();
   for (const auto& var : module->global_variables()) {
     if (variable_stack_.has(var->name())) {
       set_error(var->source(),
@@ -45,12 +46,22 @@ bool ValidatorImpl::Validate(const ast::Module* module) {
   if (!ValidateFunctions(module->functions())) {
     return false;
   }
+  function_stack_.pop_scope();
   return true;
 }
 
 bool ValidatorImpl::ValidateFunctions(const ast::FunctionList& funcs) {
   for (const auto& func : funcs) {
-    if (!ValidateFunction(func.get())) {
+    auto* func_ptr = func.get();
+    if (function_stack_.has(func_ptr->name())) {
+      set_error(func_ptr->source(), "v-0016: function names must be unique '" +
+                                        func_ptr->name() + "'");
+      return false;
+    }
+
+    function_stack_.set(func_ptr->name(), func_ptr);
+
+    if (!ValidateFunction(func_ptr)) {
       return false;
     }
   }
diff --git a/src/validator_impl.h b/src/validator_impl.h
index 0019d7da87..8cf7b96265 100644
--- a/src/validator_impl.h
+++ b/src/validator_impl.h
@@ -16,12 +16,14 @@
 #define SRC_VALIDATOR_IMPL_H_
 
 #include <string>
+#include <unordered_map>
 
 #include "src/ast/assignment_statement.h"
 #include "src/ast/expression.h"
 #include "src/ast/identifier_expression.h"
 #include "src/ast/module.h"
 #include "src/ast/statement.h"
+#include "src/ast/type/type.h"
 #include "src/ast/variable.h"
 #include "src/scope_stack.h"
 
@@ -98,6 +100,7 @@ class ValidatorImpl {
  private:
   std::string error_;
   ScopeStack<ast::Variable*> variable_stack_;
+  ScopeStack<ast::Function*> function_stack_;
 };
 
 }  // namespace tint