commit eec1a6e628e3585c48d5e08208cc22e81b7b9ab0
author Sarah Mashayekhi <sarahmashay@google.com> Mon Aug 17 15:32:38 2020 +0000
committer Sarah Mashayekhi <sarahmashay@google.com> Mon Aug 17 15:32:38 2020 +0000
tree c4b9386138d133ccbfc82f75d18cf82585b53bb7
parent 23c3fee35436cb4a2541722b9ef3628382401026

[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

diff --git a/src/validator_function_test.cc b/src/validator_function_test.cc
index d42b76b..b220e45 100644
--- a/src/validator_function_test.cc
+++ b/src/validator_function_test.cc
@@ -163,5 +163,40 @@
       "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

src/validator_impl.cc

diff --git a/src/validator_impl.cc b/src/validator_impl.cc
index b597b2c..0bbda6c 100644
--- a/src/validator_impl.cc
+++ b/src/validator_impl.cc
@@ -31,6 +31,7 @@
   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 @@
   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;
     }
   }

src/validator_impl.h

diff --git a/src/validator_impl.h b/src/validator_impl.h
index 0019d7d..8cf7b96 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 @@
  private:
   std::string error_;
   ScopeStack<ast::Variable*> variable_stack_;
+  ScopeStack<ast::Function*> function_stack_;
 };
 
 }  // namespace tint