[Wasm Exceptions] Fix RemoveUnusedNames on Try (#3583)

The delegate field of Try was not being scanned, so we could
remove a name that was used only by a delegate.

The bug was because visitTry overrides the generic visitor
visitExpression. So we need to call it manually. Sadly the code here
was pretty old (I probably wrote it back in 2015 or so) and it was
misleading, as it had unnecessary calls from the generic visitor
to visitBlock, visitLoop, which are not needed. This PR removes
them which is shorter and cleaner.

Also, we must handle the case of the delegate field being unset,
so check name.is().

3 files changed, 66 insertions, 11 deletions

diff --git a/src/passes/RemoveUnusedNames.cpp b/src/passes/RemoveUnusedNames.cpp
index 53da00733..addd4367b 100644
--- a/src/passes/RemoveUnusedNames.cpp
+++ b/src/passes/RemoveUnusedNames.cpp
@@ -37,16 +37,11 @@ struct RemoveUnusedNames
   std::map<Name, std::set<Expression*>> branchesSeen;
 
   void visitExpression(Expression* curr) {
-    if (auto* block = curr->dynCast<Block>()) {
-      visitBlock(block);
-      return;
-    }
-    if (auto* loop = curr->dynCast<Loop>()) {
-      visitLoop(loop);
-      return;
-    }
-    BranchUtils::operateOnScopeNameUses(
-      curr, [&](Name& name) { branchesSeen[name].insert(curr); });
+    BranchUtils::operateOnScopeNameUses(curr, [&](Name& name) {
+      if (name.is()) {
+        branchesSeen[name].insert(curr);
+      }
+    });
   }
 
   void handleBreakTarget(Name& name) {
@@ -83,7 +78,12 @@ struct RemoveUnusedNames
     }
   }
 
-  void visitTry(Try* curr) { handleBreakTarget(curr->name); }
+  void visitTry(Try* curr) {
+    handleBreakTarget(curr->name);
+    // Try has not just a break target but also an optional delegate with a
+    // target name, so call the generic visitor as well to handle that.
+    visitExpression(curr);
+  }
 
   void visitFunction(Function* curr) { assert(branchesSeen.empty()); }
 };
diff --git a/test/passes/remove-unused-names_all-features.txt b/test/passes/remove-unused-names_all-features.txt
new file mode 100644
index 000000000..6e3bba584
--- /dev/null
+++ b/test/passes/remove-unused-names_all-features.txt
@@ -0,0 +1,29 @@
+(module
+ (type $none_=>_none (func))
+ (type $i32_=>_none (func (param i32)))
+ (event $event$0 (attr 0) (param i32))
+ (func $0
+  (try $label$9
+   (do
+    (nop)
+   )
+   (catch $event$0
+    (try $label$8
+     (do
+      (try
+       (do
+        (rethrow $label$9)
+       )
+       (delegate $label$8)
+      )
+     )
+     (catch $event$0
+      (drop
+       (pop i32)
+      )
+     )
+    )
+   )
+  )
+ )
+)
diff --git a/test/passes/remove-unused-names_all-features.wast b/test/passes/remove-unused-names_all-features.wast
new file mode 100644
index 000000000..1c1bd43e2
--- /dev/null
+++ b/test/passes/remove-unused-names_all-features.wast
@@ -0,0 +1,26 @@
+(module
+ (event $event$0 (attr 0) (param i32))
+ (func $0
+  (try $label$9 ;; needed due to a rethrow
+   (do
+   )
+   (catch $event$0
+    (try $label$8 ;; needed due to a delegate
+     (do
+      (try $label$6 ;; this one is not needed
+       (do
+        (rethrow $label$9)
+       )
+       (delegate $label$8)
+      )
+     )
+     (catch $event$0
+      (drop
+       (pop i32)
+      )
+     )
+    )
+   )
+  )
+ )
+)