Remove more uses of AST to_str() and type_name()

These methods are going to be removed as they provide little benefit over the WGSL form, are a maintainance burden and they massively bloat our codebase.

This change introduces sem::CallTargetSignature, which can be used as a std::unordered_map key.
This is used in writer/spirv to replace a map that was keyed off ast::Function::type_name().

Bug: tint:1225
Change-Id: Ic220b3155011f21b14d49eecc8042001148e4ca5
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/66443
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@google.com>
Reviewed-by: James Price <jrprice@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>

src/inspector/inspector.cc

@@ -806,13 +806,13 @@ void Inspector::GenerateSamplerTargets() {
       continue;
     }
 
-    const auto& params = i->Parameters();
+    const auto& signature = i->Signature();
-    int sampler_index = sem::IndexOf(params, sem::ParameterUsage::kSampler);
+    int sampler_index = signature.IndexOf(sem::ParameterUsage::kSampler);
     if (sampler_index == -1) {
       continue;
     }
 
-    int texture_index = sem::IndexOf(params, sem::ParameterUsage::kTexture);
+    int texture_index = signature.IndexOf(sem::ParameterUsage::kTexture);
     if (texture_index == -1) {
       continue;
     }

src/inspector/inspector_test.cc

@@ -1495,7 +1495,7 @@ TEST_F(InspectorGetUniformBufferResourceBindingsTest, ContainingArray) {
   // with elem stride of 16, and that is 16-byte aligned within the struct)
   ast::Struct* foo_struct_type = Structure(
       "foo_type",
-      {Member("0__i32", ty.i32()),
+      {Member("0i32", ty.i32()),
        Member("b", ty.array(ty.u32(), 4, /*stride*/ 16), {MemberAlign(16)})},
       {create<ast::StructBlockDecoration>()});
 

src/inspector/test_inspector_builder.cc

@@ -84,7 +84,7 @@ bool InspectorBuilder::ContainsName(const std::vector<StageVariable>& vec,
 }
 
 std::string InspectorBuilder::StructMemberName(size_t idx, ast::Type* type) {
-  return std::to_string(idx) + type->type_name();
+  return std::to_string(idx) + type->FriendlyName(Symbols());
 }
 
 ast::Struct* InspectorBuilder::MakeStructType(

src/resolver/resolver.cc

@@ -2538,8 +2538,8 @@ bool Resolver::ValidateTextureIntrinsicFunction(
     return false;
   }
   std::string func_name = intrinsic->str();
-  auto index =
+  auto& signature = intrinsic->Signature();
-      sem::IndexOf(intrinsic->Parameters(), sem::ParameterUsage::kOffset);
+  auto index = signature.IndexOf(sem::ParameterUsage::kOffset);
   if (index > -1) {
     auto* param = ast_call->args()[index];
     if (param->Is<ast::TypeConstructorExpression>()) {

src/sem/call_target.cc

@@ -15,6 +15,7 @@
 #include "src/sem/call_target.h"
 
 #include "src/symbol_table.h"
+#include "src/utils/hash.h"
 
 TINT_INSTANTIATE_TYPEINFO(tint::sem::CallTarget);
 
@@ -22,15 +23,20 @@ namespace tint {
 namespace sem {
 
 CallTarget::CallTarget(sem::Type* return_type, const ParameterList& parameters)
-    : return_type_(return_type), parameters_(parameters) {
+    : signature_{return_type, parameters} {
   TINT_ASSERT(Semantic, return_type);
 }
 
 CallTarget::CallTarget(const CallTarget&) = default;
-
 CallTarget::~CallTarget() = default;
 
-int IndexOf(const ParameterList& parameters, ParameterUsage usage) {
+CallTargetSignature::CallTargetSignature(sem::Type* ret_ty,
+                                         const ParameterList& params)
+    : return_type(ret_ty), parameters(params) {}
+CallTargetSignature::CallTargetSignature(const CallTargetSignature&) = default;
+CallTargetSignature::~CallTargetSignature() = default;
+
+int CallTargetSignature::IndexOf(ParameterUsage usage) const {
   for (size_t i = 0; i < parameters.size(); i++) {
     if (parameters[i]->Usage() == usage) {
       return static_cast<int>(i);
@@ -39,5 +45,33 @@ int IndexOf(const ParameterList& parameters, ParameterUsage usage) {
   return -1;
 }
 
+bool CallTargetSignature::operator==(const CallTargetSignature& other) const {
+  if (return_type != other.return_type ||
+      parameters.size() != other.parameters.size()) {
+    return false;
+  }
+  for (size_t i = 0; i < parameters.size(); i++) {
+    auto* a = parameters[i];
+    auto* b = other.parameters[i];
+    if (a->Type() != b->Type() || a->Usage() != b->Usage()) {
+      return false;
+    }
+  }
+  return true;
+}
+
 }  // namespace sem
 }  // namespace tint
+
+namespace std {
+
+std::size_t hash<tint::sem::CallTargetSignature>::operator()(
+    const tint::sem::CallTargetSignature& sig) const {
+  size_t hash = tint::utils::Hash(sig.parameters.size());
+  for (auto* p : sig.parameters) {
+    tint::utils::HashCombine(&hash, p->Type(), p->Usage());
+  }
+  return tint::utils::Hash(hash, sig.return_type);
+}
+
+}  // namespace std

src/sem/call_target.h

@@ -27,11 +27,34 @@ namespace sem {
 // Forward declarations
 class Type;
 
-/// @param parameters the list of parameters
+/// CallTargetSignature holds the return type and parameters for a call target
-/// @param usage the parameter usage to find
+struct CallTargetSignature {
-/// @returns the index of the parameter with the given usage, or -1 if no
+  /// Constructor
-/// parameter with the given usage exists.
+  /// @param ret_ty the call target return type
-int IndexOf(const ParameterList& parameters, ParameterUsage usage);
+  /// @param params the call target parameters
+  CallTargetSignature(sem::Type* ret_ty, const ParameterList& params);
+
+  /// Copy constructor
+  CallTargetSignature(const CallTargetSignature&);
+
+  /// Destructor
+  ~CallTargetSignature();
+
+  /// The type of the call target return value
+  sem::Type* const return_type = nullptr;
+  /// The parameters of the call target
+  ParameterList const parameters;
+
+  /// Equality operator
+  /// @param other the signature to compare this to
+  /// @returns true if this signature is equal to other
+  bool operator==(const CallTargetSignature& other) const;
+
+  /// @param usage the parameter usage to find
+  /// @returns the index of the parameter with the given usage, or -1 if no
+  /// parameter with the given usage exists.
+  int IndexOf(ParameterUsage usage) const;
+};
 
 /// CallTarget is the base for callable functions
 class CallTarget : public Castable<CallTarget, Node> {
@@ -44,21 +67,38 @@ class CallTarget : public Castable<CallTarget, Node> {
   /// Copy constructor
   CallTarget(const CallTarget&);
 
-  /// @return the return type of the call target
-  sem::Type* ReturnType() const { return return_type_; }
-
   /// Destructor
   ~CallTarget() override;
 
+  /// @return the return type of the call target
+  sem::Type* ReturnType() const { return signature_.return_type; }
+
   /// @return the parameters of the call target
-  const ParameterList& Parameters() const { return parameters_; }
+  const ParameterList& Parameters() const { return signature_.parameters; }
+
+  /// @return the signature of the call target
+  const CallTargetSignature& Signature() const { return signature_; }
 
  private:
-  sem::Type* const return_type_;
+  CallTargetSignature signature_;
-  ParameterList const parameters_;
 };
 
 }  // namespace sem
 }  // namespace tint
 
+namespace std {
+
+/// Custom std::hash specialization for tint::sem::CallTargetSignature so
+/// CallTargetSignature can be used as keys for std::unordered_map and
+/// std::unordered_set.
+template <>
+class hash<tint::sem::CallTargetSignature> {
+ public:
+  /// @param sig the CallTargetSignature to hash
+  /// @return the hash value
+  std::size_t operator()(const tint::sem::CallTargetSignature& sig) const;
+};
+
+}  // namespace std
+
 #endif  // SRC_SEM_CALL_TARGET_H_

src/transform/robustness.cc

@@ -208,14 +208,11 @@ struct Robustness::State {
 
     // Indices of the mandatory texture and coords parameters, and the optional
     // array and level parameters.
-    auto texture_idx =
+    auto& signature = intrinsic->Signature();
-        sem::IndexOf(intrinsic->Parameters(), sem::ParameterUsage::kTexture);
+    auto texture_idx = signature.IndexOf(sem::ParameterUsage::kTexture);
-    auto coords_idx =
+    auto coords_idx = signature.IndexOf(sem::ParameterUsage::kCoords);
-        sem::IndexOf(intrinsic->Parameters(), sem::ParameterUsage::kCoords);
+    auto array_idx = signature.IndexOf(sem::ParameterUsage::kArrayIndex);
-    auto array_idx =
+    auto level_idx = signature.IndexOf(sem::ParameterUsage::kLevel);
-        sem::IndexOf(intrinsic->Parameters(), sem::ParameterUsage::kArrayIndex);
-    auto level_idx =
-        sem::IndexOf(intrinsic->Parameters(), sem::ParameterUsage::kLevel);
 
     auto* texture_arg = expr->args()[texture_idx];
     auto* coords_arg = expr->args()[coords_idx];

src/writer/glsl/generator_impl.cc

@@ -960,12 +960,12 @@ bool GeneratorImpl::EmitTextureCall(std::ostream& out,
                                     const sem::Intrinsic* intrinsic) {
   using Usage = sem::ParameterUsage;
 
-  auto parameters = intrinsic->Parameters();
+  auto& signature = intrinsic->Signature();
   auto arguments = expr->args();
 
   // Returns the argument with the given usage
   auto arg = [&](Usage usage) {
-    int idx = sem::IndexOf(parameters, usage);
+    int idx = signature.IndexOf(usage);
     return (idx >= 0) ? arguments[idx] : nullptr;
   };
 
@@ -1395,8 +1395,9 @@ bool GeneratorImpl::EmitExpression(std::ostream& out, ast::Expression* expr) {
     return EmitUnaryOp(out, u);
   }
 
-  diagnostics_.add_error(diag::System::Writer,
+  diagnostics_.add_error(
-                         "unknown expression type: " + builder_.str(expr));
+      diag::System::Writer,
+      "unknown expression type: " + std::string(expr->TypeInfo().name));
   return false;
 }
 
@@ -2235,8 +2236,9 @@ bool GeneratorImpl::EmitStatement(ast::Statement* stmt) {
     return EmitVariable(v->variable());
   }
 
-  diagnostics_.add_error(diag::System::Writer,
+  diagnostics_.add_error(
-                         "unknown statement type: " + builder_.str(stmt));
+      diag::System::Writer,
+      "unknown statement type: " + std::string(stmt->TypeInfo().name));
   return false;
 }
 

src/writer/hlsl/generator_impl.cc

@@ -1620,12 +1620,12 @@ bool GeneratorImpl::EmitTextureCall(std::ostream& out,
                                     const sem::Intrinsic* intrinsic) {
   using Usage = sem::ParameterUsage;
 
-  auto parameters = intrinsic->Parameters();
+  auto& signature = intrinsic->Signature();
   auto arguments = expr->args();
 
   // Returns the argument with the given usage
   auto arg = [&](Usage usage) {
-    int idx = sem::IndexOf(parameters, usage);
+    int idx = signature.IndexOf(usage);
     return (idx >= 0) ? arguments[idx] : nullptr;
   };
 
@@ -2215,8 +2215,9 @@ bool GeneratorImpl::EmitExpression(std::ostream& out, ast::Expression* expr) {
     return EmitUnaryOp(out, u);
   }
 
-  diagnostics_.add_error(diag::System::Writer,
+  diagnostics_.add_error(
-                         "unknown expression type: " + builder_.str(expr));
+      diag::System::Writer,
+      "unknown expression type: " + std::string(expr->TypeInfo().name));
   return false;
 }
 
@@ -2934,8 +2935,9 @@ bool GeneratorImpl::EmitStatement(ast::Statement* stmt) {
     return EmitVariable(v->variable());
   }
 
-  diagnostics_.add_error(diag::System::Writer,
+  diagnostics_.add_error(
-                         "unknown statement type: " + builder_.str(stmt));
+      diag::System::Writer,
+      "unknown statement type: " + std::string(stmt->TypeInfo().name));
   return false;
 }
 

src/writer/msl/generator_impl.cc

@@ -764,12 +764,12 @@ bool GeneratorImpl::EmitTextureCall(std::ostream& out,
                                     const sem::Intrinsic* intrinsic) {
   using Usage = sem::ParameterUsage;
 
-  auto parameters = intrinsic->Parameters();
+  auto& signature = intrinsic->Signature();
   auto arguments = expr->args();
 
   // Returns the argument with the given usage
   auto arg = [&](Usage usage) {
-    int idx = sem::IndexOf(parameters, usage);
+    int idx = signature.IndexOf(usage);
     return (idx >= 0) ? arguments[idx] : nullptr;
   };
 
@@ -1427,8 +1427,9 @@ bool GeneratorImpl::EmitExpression(std::ostream& out, ast::Expression* expr) {
     return EmitUnaryOp(out, u);
   }
 
-  diagnostics_.add_error(diag::System::Writer,
+  diagnostics_.add_error(
-                         "unknown expression type: " + program_->str(expr));
+      diag::System::Writer,
+      "unknown expression type: " + std::string(expr->TypeInfo().name));
   return false;
 }
 
@@ -2052,8 +2053,9 @@ bool GeneratorImpl::EmitStatement(ast::Statement* stmt) {
     return EmitVariable(var);
   }
 
-  diagnostics_.add_error(diag::System::Writer,
+  diagnostics_.add_error(
-                         "unknown statement type: " + program_->str(stmt));
+      diag::System::Writer,
+      "unknown statement type: " + std::string(stmt->TypeInfo().name));
   return false;
 }
 

src/writer/spirv/builder.cc

@@ -584,7 +584,7 @@ uint32_t Builder::GenerateExpression(ast::Expression* expr) {
     return GenerateUnaryOpExpression(u);
   }
 
-  error_ = "unknown expression type: " + builder_.str(expr);
+  error_ = "unknown expression type: " + std::string(expr->TypeInfo().name);
   return 0;
 }
 
@@ -659,32 +659,28 @@ bool Builder::GenerateFunction(ast::Function* func_ast) {
 }
 
 uint32_t Builder::GenerateFunctionTypeIfNeeded(const sem::Function* func) {
-  auto val = type_name_to_id_.find(func->Declaration()->type_name());
+  return utils::GetOrCreate(
-  if (val != type_name_to_id_.end()) {
+      func_sig_to_id_, func->Signature(), [&]() -> uint32_t {
-    return val->second;
+        auto func_op = result_op();
-  }
+        auto func_type_id = func_op.to_i();
 
-  auto func_op = result_op();
+        auto ret_id = GenerateTypeIfNeeded(func->ReturnType());
-  auto func_type_id = func_op.to_i();
+        if (ret_id == 0) {
+          return 0;
+        }
 
-  auto ret_id = GenerateTypeIfNeeded(func->ReturnType());
+        OperandList ops = {func_op, Operand::Int(ret_id)};
-  if (ret_id == 0) {
+        for (auto* param : func->Parameters()) {
-    return 0;
+          auto param_type_id = GenerateTypeIfNeeded(param->Type());
-  }
+          if (param_type_id == 0) {
+            return 0;
+          }
+          ops.push_back(Operand::Int(param_type_id));
+        }
 
-  OperandList ops = {func_op, Operand::Int(ret_id)};
+        push_type(spv::Op::OpTypeFunction, std::move(ops));
-  for (auto* param : func->Parameters()) {
+        return func_type_id;
-    auto param_type_id = GenerateTypeIfNeeded(param->Type());
+      });
-    if (param_type_id == 0) {
-      return 0;
-    }
-    ops.push_back(Operand::Int(param_type_id));
-  }
-
-  push_type(spv::Op::OpTypeFunction, std::move(ops));
-
-  type_name_to_id_[func->Declaration()->type_name()] = func_type_id;
-  return func_type_id;
 }
 
 bool Builder::GenerateFunctionVariable(ast::Variable* var) {
@@ -1139,7 +1135,8 @@ uint32_t Builder::GenerateAccessorExpression(ast::Expression* expr) {
       }
 
     } else {
-      error_ = "invalid accessor in list: " + builder_.str(accessor);
+      error_ =
+          "invalid accessor in list: " + std::string(accessor->TypeInfo().name);
       return 0;
     }
   }
@@ -2662,7 +2659,7 @@ bool Builder::GenerateTextureIntrinsic(ast::CallExpression* call,
                                        Operand result_id) {
   using Usage = sem::ParameterUsage;
 
-  auto parameters = intrinsic->Parameters();
+  auto& signature = intrinsic->Signature();
   auto arguments = call->args();
 
   // Generates the given expression, returning the operand ID
@@ -2678,7 +2675,7 @@ bool Builder::GenerateTextureIntrinsic(ast::CallExpression* call,
 
   // Returns the argument with the given usage
   auto arg = [&](Usage usage) {
-    int idx = sem::IndexOf(parameters, usage);
+    int idx = signature.IndexOf(usage);
     return (idx >= 0) ? arguments[idx] : nullptr;
   };
 
@@ -3785,7 +3782,7 @@ bool Builder::GenerateStatement(ast::Statement* stmt) {
     return GenerateVariableDeclStatement(v);
   }
 
-  error_ = "Unknown statement: " + builder_.str(stmt);
+  error_ = "Unknown statement: " + std::string(stmt->TypeInfo().name);
   return false;
 }
 

src/writer/spirv/builder.h

@@ -585,6 +585,7 @@ class Builder {
 
   std::unordered_map<std::string, uint32_t> import_name_to_id_;
   std::unordered_map<Symbol, uint32_t> func_symbol_to_id_;
+  std::unordered_map<sem::CallTargetSignature, uint32_t> func_sig_to_id_;
   std::unordered_map<std::string, uint32_t> type_name_to_id_;
   std::unordered_map<ScalarConstant, uint32_t> const_to_id_;
   std::unordered_map<std::string, uint32_t> type_constructor_to_id_;

src/writer/wgsl/generator_impl.cc

@@ -524,8 +524,9 @@ bool GeneratorImpl::EmitType(std::ostream& out, const ast::Type* ty) {
   } else if (auto* tn = ty->As<ast::TypeName>()) {
     out << program_->Symbols().NameFor(tn->name());
   } else {
-    diagnostics_.add_error(diag::System::Writer,
+    diagnostics_.add_error(
-                           "unknown type in EmitType: " + ty->type_name());
+        diag::System::Writer,
+        "unknown type in EmitType: " + std::string(ty->TypeInfo().name));
     return false;
   }
   return true;
@@ -869,8 +870,9 @@ bool GeneratorImpl::EmitStatement(ast::Statement* stmt) {
     return EmitVariable(line(), v->variable());
   }
 
-  diagnostics_.add_error(diag::System::Writer,
+  diagnostics_.add_error(
-                         "unknown statement type: " + program_->str(stmt));
+      diag::System::Writer,
+      "unknown statement type: " + std::string(stmt->TypeInfo().name));
   return false;
 }