From 18bb1f5af22b4d6bc309b96d844f856077eb6b60 Mon Sep 17 00:00:00 2001
From: Alon Zakai <azakai@google.com>
Date: Thu, 30 May 2024 13:44:18 -0700
Subject: [PATCH] Avoid duplicate type names (#6633)

If we replace a type with another, use the original name for the new type,
and give the old a unique name (for the rare cases in which it has uses).
---
 src/ir/type-updating.cpp               | 23 ++++++++++++++++++++---
 src/passes/RemoveUnusedTypes.cpp       |  2 +-
 test/lit/passes/signature-pruning.wast |  3 ++-
 3 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/src/ir/type-updating.cpp b/src/ir/type-updating.cpp
index 760d71b9c20..520408d449f 100644
--- a/src/ir/type-updating.cpp
+++ b/src/ir/type-updating.cpp
@@ -18,6 +18,7 @@
 #include "find_all.h"
 #include "ir/local-structural-dominance.h"
 #include "ir/module-utils.h"
+#include "ir/names.h"
 #include "ir/utils.h"
 #include "support/topological_sort.h"
 #include "wasm-type-ordering.h"
@@ -152,11 +153,27 @@ GlobalTypeRewriter::TypeMap GlobalTypeRewriter::rebuildTypes(
     oldToNewTypes[type] = newTypes[index];
   }
 
-  // Update type names (doing it before mapTypes can help debugging there, but
-  // has no other effect; mapTypes does not look at type names).
+  // Update type names to avoid duplicates.
+  std::unordered_set<Name> typeNames;
+  for (auto& [type, info] : wasm.typeNames) {
+    typeNames.insert(info.name);
+  }
   for (auto& [old, new_] : oldToNewTypes) {
+    if (old == new_) {
+      // The type is being mapped to itself; no need to rename anything.
+      continue;
+    }
+
     if (auto it = wasm.typeNames.find(old); it != wasm.typeNames.end()) {
-      wasm.typeNames[new_] = it->second;
+      wasm.typeNames[new_] = wasm.typeNames[old];
+      // Use the existing name in the new type, as usually it completely
+      // replaces the old. Rename the old name in a unique way to avoid
+      // confusion in the case that it remains used.
+      auto deduped =
+        Names::getValidName(wasm.typeNames[old].name,
+                            [&](Name test) { return !typeNames.count(test); });
+      wasm.typeNames[old].name = deduped;
+      typeNames.insert(deduped);
     }
   }
 
diff --git a/src/passes/RemoveUnusedTypes.cpp b/src/passes/RemoveUnusedTypes.cpp
index 1ea65fb0f04..45222708a81 100644
--- a/src/passes/RemoveUnusedTypes.cpp
+++ b/src/passes/RemoveUnusedTypes.cpp
@@ -45,7 +45,7 @@ struct RemoveUnusedTypes : Pass {
     }
 
     // We're not changing the contents of any of the types, so we just round
-    // trip them throgh GlobalTypeRewriter, which will put all the private types
+    // trip them through GlobalTypeRewriter which will put all the private types
     // in a single new rec group and leave out all the unused types.
     GlobalTypeRewriter(*module).update();
   }
diff --git a/test/lit/passes/signature-pruning.wast b/test/lit/passes/signature-pruning.wast
index c273d1588ba..8754c8189f8 100644
--- a/test/lit/passes/signature-pruning.wast
+++ b/test/lit/passes/signature-pruning.wast
@@ -1168,10 +1168,11 @@
    ;; CHECK:      (rec
    ;; CHECK-NEXT:  (type $none (func))
    (type $none (func))
-   ;; CHECK:       (type $much (func (param i32)))
    (type $much (func (param i32)))
   )
 
+  ;; CHECK:       (type $much_0 (func (param i32)))
+
   ;; CHECK:      (export "exported" (func $exported))
 
   ;; CHECK:      (func $exported (type $none)