Remove Function::name().

This CL removes the function name accessor and changes all usages to use
the symbol.

Change-Id: I19b92bf1bc557ba14e68ef8cb381487a4ad1f7ee
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/36821
Reviewed-by: Ben Clayton <bclayton@google.com>

src/ast/function.h

@@ -74,7 +74,7 @@ class Function : public Castable<Function, Node> {
   /// @returns the function symbol
   Symbol symbol() const { return symbol_; }
   /// @returns the function name
-  const std::string& name() { return name_; }
+  const std::string& name_for_clone() { return name_; }
   /// @returns the function params
   const VariableList& params() const { return params_; }
 

src/ast/function_test.cc

@@ -36,7 +36,6 @@ TEST_F(FunctionTest, Creation) {
   auto* f =
       Func("func", params, ty.void_, StatementList{}, FunctionDecorationList{});
   EXPECT_EQ(f->symbol(), mod->RegisterSymbol("func"));
-  EXPECT_EQ(f->name(), "func");
   ASSERT_EQ(f->params().size(), 1u);
   EXPECT_EQ(f->return_type(), ty.void_);
   EXPECT_EQ(f->params()[0], var);

src/inspector/inspector.cc

@@ -57,8 +57,8 @@ std::vector<EntryPoint> Inspector::GetEntryPoints() {
     }
 
     EntryPoint entry_point;
-    entry_point.name = func->name();
+    entry_point.name = module_.SymbolToName(func->symbol());
-    entry_point.remapped_name = func->name();
+    entry_point.remapped_name = namer_->NameFor(func->symbol());
     entry_point.stage = func->pipeline_stage();
     std::tie(entry_point.workgroup_size_x, entry_point.workgroup_size_y,
              entry_point.workgroup_size_z) = func->workgroup_size();

src/reader/wgsl/parser_impl_function_decl_test.cc

@@ -37,7 +37,7 @@ TEST_F(ParserImplTest, FunctionDecl) {
   EXPECT_TRUE(f.matched);
   ASSERT_NE(f.value, nullptr);
 
-  EXPECT_EQ(f->name(), "main");
+  EXPECT_EQ(f->symbol(), p->get_module().RegisterSymbol("main"));
   ASSERT_NE(f->return_type(), nullptr);
   EXPECT_TRUE(f->return_type()->Is<ast::type::Void>());
 
@@ -65,7 +65,7 @@ TEST_F(ParserImplTest, FunctionDecl_DecorationList) {
   EXPECT_TRUE(f.matched);
   ASSERT_NE(f.value, nullptr);
 
-  EXPECT_EQ(f->name(), "main");
+  EXPECT_EQ(f->symbol(), p->get_module().RegisterSymbol("main"));
   ASSERT_NE(f->return_type(), nullptr);
   EXPECT_TRUE(f->return_type()->Is<ast::type::Void>());
   ASSERT_EQ(f->params().size(), 0u);
@@ -103,7 +103,7 @@ fn main() -> void { return; })");
   EXPECT_TRUE(f.matched);
   ASSERT_NE(f.value, nullptr);
 
-  EXPECT_EQ(f->name(), "main");
+  EXPECT_EQ(f->symbol(), p->get_module().RegisterSymbol("main"));
   ASSERT_NE(f->return_type(), nullptr);
   EXPECT_TRUE(f->return_type()->Is<ast::type::Void>());
   ASSERT_EQ(f->params().size(), 0u);
@@ -148,7 +148,7 @@ fn main() -> void { return; })");
   EXPECT_TRUE(f.matched);
   ASSERT_NE(f.value, nullptr);
 
-  EXPECT_EQ(f->name(), "main");
+  EXPECT_EQ(f->symbol(), p->get_module().RegisterSymbol("main"));
   ASSERT_NE(f->return_type(), nullptr);
   EXPECT_TRUE(f->return_type()->Is<ast::type::Void>());
   ASSERT_EQ(f->params().size(), 0u);

src/transform/transform.cc

@@ -32,7 +32,7 @@ ast::Function* Transform::CloneWithStatementsAtStart(
     statements.emplace_back(ctx->Clone(s));
   }
   return ctx->mod->create<ast::Function>(
-      ctx->Clone(in->source()), ctx->Clone(in->symbol()), in->name(),
+      ctx->Clone(in->source()), ctx->Clone(in->symbol()), in->name_for_clone(),
       ctx->Clone(in->params()), ctx->Clone(in->return_type()),
       ctx->mod->create<ast::BlockStatement>(ctx->Clone(in->body()->source()),
                                             statements),

src/type_determiner.cc

@@ -121,7 +121,7 @@ bool TypeDeterminer::Determine() {
     if (!func->IsEntryPoint()) {
       continue;
     }
-    for (const auto& callee : caller_to_callee_[func->name()]) {
+    for (const auto& callee : caller_to_callee_[func->symbol().value()]) {
       set_entry_points(callee, func->symbol());
     }
   }
@@ -129,11 +129,10 @@ bool TypeDeterminer::Determine() {
   return true;
 }
 
-void TypeDeterminer::set_entry_points(const std::string& fn_name,
+void TypeDeterminer::set_entry_points(const Symbol& fn_sym, Symbol ep_sym) {
-                                      Symbol ep_sym) {
+  symbol_to_function_[fn_sym.value()]->add_ancestor_entry_point(ep_sym);
-  name_to_function_[fn_name]->add_ancestor_entry_point(ep_sym);
 
-  for (const auto& callee : caller_to_callee_[fn_name]) {
+  for (const auto& callee : caller_to_callee_[fn_sym.value()]) {
     set_entry_points(callee, ep_sym);
   }
 }
@@ -148,7 +147,7 @@ bool TypeDeterminer::DetermineFunctions(const ast::FunctionList& funcs) {
 }
 
 bool TypeDeterminer::DetermineFunction(ast::Function* func) {
-  name_to_function_[func->name()] = func;
+  symbol_to_function_[func->symbol().value()] = func;
 
   current_function_ = func;
 
@@ -387,13 +386,13 @@ bool TypeDeterminer::DetermineCall(ast::CallExpression* expr) {
       }
     } else {
       if (current_function_) {
-        caller_to_callee_[current_function_->name()].push_back(ident->name());
+        caller_to_callee_[current_function_->symbol().value()].push_back(
+            ident->symbol());
 
-        auto* callee_func =
+        auto* callee_func = mod_->FindFunctionBySymbol(ident->symbol());
-            mod_->FindFunctionBySymbol(mod_->GetSymbol(ident->name()));
         if (callee_func == nullptr) {
-          set_error(expr->source(),
+          set_error(expr->source(), "unable to find called function: " +
-                    "unable to find called function: " + ident->name());
+                                        mod_->SymbolToName(ident->symbol()));
           return false;
         }
 
@@ -416,10 +415,10 @@ bool TypeDeterminer::DetermineCall(ast::CallExpression* expr) {
   }
 
   if (!expr->func()->result_type()) {
-    auto func_name = expr->func()->As<ast::IdentifierExpression>()->name();
+    auto func_sym = expr->func()->As<ast::IdentifierExpression>()->symbol();
-    set_error(
+    set_error(expr->source(),
-        expr->source(),
+              "v-0005: function must be declared before use: '" +
-        "v-0005: function must be declared before use: '" + func_name + "'");
+                  mod_->SymbolToName(func_sym) + "'");
     return false;
   }
 
@@ -858,7 +857,6 @@ bool TypeDeterminer::DetermineConstructor(ast::ConstructorExpression* expr) {
 }
 
 bool TypeDeterminer::DetermineIdentifier(ast::IdentifierExpression* expr) {
-  auto name = expr->name();
   auto symbol = expr->symbol();
   ast::Variable* var;
   if (variable_stack_.get(symbol, &var)) {
@@ -877,15 +875,16 @@ bool TypeDeterminer::DetermineIdentifier(ast::IdentifierExpression* expr) {
     return true;
   }
 
-  auto iter = name_to_function_.find(name);
+  auto iter = symbol_to_function_.find(symbol.value());
-  if (iter != name_to_function_.end()) {
+  if (iter != symbol_to_function_.end()) {
     expr->set_result_type(iter->second->return_type());
     return true;
   }
 
   if (!SetIntrinsicIfNeeded(expr)) {
     set_error(expr->source(),
-              "v-0006: identifier must be declared before use: " + name);
+              "v-0006: identifier must be declared before use: " +
+                  mod_->SymbolToName(symbol));
     return false;
   }
   return true;

src/type_determiner.h

@@ -113,7 +113,7 @@ class TypeDeterminer {
  private:
   void set_error(const Source& src, const std::string& msg);
   void set_referenced_from_function_if_needed(ast::Variable* var, bool local);
-  void set_entry_points(const std::string& fn_name, Symbol ep_sym);
+  void set_entry_points(const Symbol& fn_sym, Symbol ep_sym);
 
   bool DetermineArrayAccessor(ast::ArrayAccessorExpression* expr);
   bool DetermineBinary(ast::BinaryExpression* expr);
@@ -129,11 +129,11 @@ class TypeDeterminer {
   ast::Module* mod_;
   std::string error_;
   ScopeStack<ast::Variable*> variable_stack_;
-  std::unordered_map<std::string, ast::Function*> name_to_function_;
+  std::unordered_map<uint32_t, ast::Function*> symbol_to_function_;
   ast::Function* current_function_ = nullptr;
 
   // Map from caller functions to callee functions.
-  std::unordered_map<std::string, std::vector<std::string>> caller_to_callee_;
+  std::unordered_map<uint32_t, std::vector<Symbol>> caller_to_callee_;
 };
 
 }  // namespace tint

src/validator/validator_impl.cc

@@ -160,14 +160,14 @@ bool ValidatorImpl::ValidateEntryPoint(const ast::FunctionList& funcs) {
       if (!func->params().empty()) {
         add_error(func->source(), "v-0023",
                   "Entry point function must accept no parameters: '" +
-                      func->name() + "'");
+                      module_.SymbolToName(func->symbol()) + "'");
         return false;
       }
 
       if (!func->return_type()->Is<ast::type::Void>()) {
-        add_error(
+        add_error(func->source(), "v-0024",
-            func->source(), "v-0024",
+                  "Entry point function must return void: '" +
-            "Entry point function must return void: '" + func->name() + "'");
+                      module_.SymbolToName(func->symbol()) + "'");
         return false;
       }
       auto stage_deco_count = 0;

src/writer/spirv/builder.cc

@@ -448,7 +448,7 @@ bool Builder::GenerateEntryPoint(ast::Function* func, uint32_t id) {
   // OperandList operands = {Operand::Int(stage), Operand::Int(id),
   //                         Operand::String(func->name())};
   OperandList operands = {Operand::Int(stage), Operand::Int(id),
-                          Operand::String(func->name())};
+                          Operand::String(namer_->NameFor(func->symbol()))};
 
   for (const auto* var : func->referenced_module_variables()) {
     // For SPIR-V 1.3 we only output Input/output variables. If we update to
@@ -593,8 +593,7 @@ bool Builder::GenerateFunction(ast::Function* func) {
 
   scope_stack_.pop_scope();
 
-  func_name_to_id_[func->name()] = func_id;
+  func_symbol_to_id_[func->symbol().value()] = func_id;
-  func_name_to_func_[func->name()] = func;
 
   return true;
 }
@@ -1820,7 +1819,7 @@ uint32_t Builder::GenerateCallExpression(ast::CallExpression* expr) {
 
   OperandList ops = {Operand::Int(type_id), result};
 
-  auto func_id = func_name_to_id_[ident->name()];
+  auto func_id = func_symbol_to_id_[ident->symbol().value()];
   if (func_id == 0) {
     error_ = "unable to find called function: " + ident->name();
     return 0;

src/writer/spirv/builder.h

@@ -490,16 +490,6 @@ class Builder {
   /// automatically.
   Operand result_op();
 
-  /// Retrives the id for the given function name
-  /// @param name the function name to search for
-  /// @returns the id for the given name or 0 on failure
-  uint32_t id_for_func_name(const std::string& name) {
-    if (func_name_to_id_.count(name) == 0) {
-      return 0;
-    }
-    return func_name_to_id_[name];
-  }
-
   ast::Module* mod_;
   std::unique_ptr<Namer> namer_;
   std::string error_;
@@ -517,8 +507,7 @@ class Builder {
   std::vector<Function> functions_;
 
   std::unordered_map<std::string, uint32_t> import_name_to_id_;
-  std::unordered_map<std::string, uint32_t> func_name_to_id_;
+  std::unordered_map<uint32_t, uint32_t> func_symbol_to_id_;
-  std::unordered_map<std::string, ast::Function*> func_name_to_func_;
   std::unordered_map<std::string, uint32_t> type_name_to_id_;
   std::unordered_map<std::string, uint32_t> const_to_id_;
   std::unordered_map<std::string, uint32_t>

src/writer/wgsl/generator_impl.cc

@@ -361,7 +361,7 @@ bool GeneratorImpl::EmitFunction(ast::Function* func) {
   }
 
   make_indent();
-  out_ << "fn " << func->name() << "(";
+  out_ << "fn " << module_.SymbolToName(func->symbol()) << "(";
 
   bool first = true;
   for (auto* v : func->params()) {