create a UnifiedExpressionVisitor for passes that want a single visitor function, to avoid confusion with having both visit* and visitExpression in a single pass (#357)

17 files changed, 79 insertions, 52 deletions

diff --git a/src/ast_utils.h b/src/ast_utils.h
index 3fe4ee17a..f91cfab24 100644
--- a/src/ast_utils.h
+++ b/src/ast_utils.h
@@ -22,7 +22,7 @@
 
 namespace wasm {
 
-struct BreakSeeker : public PostWalker<BreakSeeker> {
+struct BreakSeeker : public PostWalker<BreakSeeker, Visitor<BreakSeeker>> {
   Name target; // look for this one
   size_t found;
 
@@ -42,7 +42,7 @@ struct BreakSeeker : public PostWalker<BreakSeeker> {
 // Look for side effects, including control flow
 // TODO: optimize
 
-struct EffectAnalyzer : public PostWalker<EffectAnalyzer> {
+struct EffectAnalyzer : public PostWalker<EffectAnalyzer, Visitor<EffectAnalyzer>> {
   bool branches = false;
   bool calls = false;
   std::set<Index> localsRead;
diff --git a/src/pass.h b/src/pass.h
index 2c3a09ae6..85aec624f 100644
--- a/src/pass.h
+++ b/src/pass.h
@@ -143,7 +143,7 @@ public:
 // e.g. through PassRunner::getLast
 
 // Handles names in a module, in particular adding names without duplicates
-class NameManager : public WalkerPass<PostWalker<NameManager>> {
+class NameManager : public WalkerPass<PostWalker<NameManager, Visitor<NameManager>>> {
  public:
   Name getUnique(std::string prefix);
   // TODO: getUniqueInFunction
diff --git a/src/passes/LowerIfElse.cpp b/src/passes/LowerIfElse.cpp
index 48c1c0f9c..c0df094fc 100644
--- a/src/passes/LowerIfElse.cpp
+++ b/src/passes/LowerIfElse.cpp
@@ -32,7 +32,7 @@
 
 namespace wasm {
 
-struct LowerIfElse : public WalkerPass<PostWalker<LowerIfElse>> {
+struct LowerIfElse : public WalkerPass<PostWalker<LowerIfElse, Visitor<LowerIfElse>>> {
   MixedArena* allocator;
   std::unique_ptr<NameManager> namer;
 
diff --git a/src/passes/MergeBlocks.cpp b/src/passes/MergeBlocks.cpp
index 5033d4016..f3b85ab3d 100644
--- a/src/passes/MergeBlocks.cpp
+++ b/src/passes/MergeBlocks.cpp
@@ -23,7 +23,7 @@
 
 namespace wasm {
 
-struct MergeBlocks : public WalkerPass<PostWalker<MergeBlocks>> {
+struct MergeBlocks : public WalkerPass<PostWalker<MergeBlocks, Visitor<MergeBlocks>>> {
   bool isFunctionParallel() { return true; }
 
   void visitBlock(Block *curr) {
diff --git a/src/passes/Metrics.cpp b/src/passes/Metrics.cpp
index 19df981b8..8890c3cc3 100644
--- a/src/passes/Metrics.cpp
+++ b/src/passes/Metrics.cpp
@@ -24,7 +24,7 @@ namespace wasm {
 using namespace std;
 
 // Prints metrics between optimization passes.
-struct Metrics : public WalkerPass<PostWalker<Metrics>> {
+struct Metrics : public WalkerPass<PostWalker<Metrics, UnifiedExpressionVisitor<Metrics>>> {
   static Metrics *lastMetricsPass;
 
   map<const char *, int> counts;
diff --git a/src/passes/OptimizeInstructions.cpp b/src/passes/OptimizeInstructions.cpp
index 28cac726a..368b483ad 100644
--- a/src/passes/OptimizeInstructions.cpp
+++ b/src/passes/OptimizeInstructions.cpp
@@ -25,7 +25,7 @@
 
 namespace wasm {
 
-struct OptimizeInstructions : public WalkerPass<PostWalker<OptimizeInstructions>> {
+struct OptimizeInstructions : public WalkerPass<PostWalker<OptimizeInstructions, Visitor<OptimizeInstructions>>> {
   bool isFunctionParallel() { return true; }
 
   void visitIf(If* curr) {
diff --git a/src/passes/PostEmscripten.cpp b/src/passes/PostEmscripten.cpp
index ef01b11fe..43fd1b2bc 100644
--- a/src/passes/PostEmscripten.cpp
+++ b/src/passes/PostEmscripten.cpp
@@ -24,7 +24,7 @@
 
 namespace wasm {
 
-struct PostEmscripten : public WalkerPass<PostWalker<PostEmscripten>> {
+struct PostEmscripten : public WalkerPass<PostWalker<PostEmscripten, Visitor<PostEmscripten>>> {
   bool isFunctionParallel() { return true; }
 
   // When we have a Load from a local value (typically a GetLocal) plus a constant offset,
diff --git a/src/passes/RemoveImports.cpp b/src/passes/RemoveImports.cpp
index 4f42c526f..0adb9a448 100644
--- a/src/passes/RemoveImports.cpp
+++ b/src/passes/RemoveImports.cpp
@@ -27,7 +27,7 @@
 
 namespace wasm {
 
-struct RemoveImports : public WalkerPass<PostWalker<RemoveImports>> {
+struct RemoveImports : public WalkerPass<PostWalker<RemoveImports, Visitor<RemoveImports>>> {
   MixedArena* allocator;
   Module* module;
 
diff --git a/src/passes/RemoveUnusedBrs.cpp b/src/passes/RemoveUnusedBrs.cpp
index 6718fc14b..cfd9ea7ce 100644
--- a/src/passes/RemoveUnusedBrs.cpp
+++ b/src/passes/RemoveUnusedBrs.cpp
@@ -23,7 +23,7 @@
 
 namespace wasm {
 
-struct RemoveUnusedBrs : public WalkerPass<PostWalker<RemoveUnusedBrs>> {
+struct RemoveUnusedBrs : public WalkerPass<PostWalker<RemoveUnusedBrs, Visitor<RemoveUnusedBrs>>> {
   bool isFunctionParallel() { return true; }
 
   // preparation: try to unify branches, as the fewer there are, the higher a chance we can remove them
diff --git a/src/passes/RemoveUnusedNames.cpp b/src/passes/RemoveUnusedNames.cpp
index d33b5081a..4a37270e6 100644
--- a/src/passes/RemoveUnusedNames.cpp
+++ b/src/passes/RemoveUnusedNames.cpp
@@ -23,7 +23,7 @@
 
 namespace wasm {
 
-struct RemoveUnusedNames : public WalkerPass<PostWalker<RemoveUnusedNames>> {
+struct RemoveUnusedNames : public WalkerPass<PostWalker<RemoveUnusedNames, Visitor<RemoveUnusedNames>>> {
   bool isFunctionParallel() { return true; }
 
   // We maintain a list of branches that we saw in children, then when we reach
diff --git a/src/passes/ReorderLocals.cpp b/src/passes/ReorderLocals.cpp
index 22295dc1a..1a3fbf09a 100644
--- a/src/passes/ReorderLocals.cpp
+++ b/src/passes/ReorderLocals.cpp
@@ -26,7 +26,7 @@
 
 namespace wasm {
 
-struct ReorderLocals : public WalkerPass<PostWalker<ReorderLocals>> {
+struct ReorderLocals : public WalkerPass<PostWalker<ReorderLocals, Visitor<ReorderLocals>>> {
   bool isFunctionParallel() { return true; }
 
   std::map<Index, uint32_t> counts;
@@ -79,7 +79,7 @@ struct ReorderLocals : public WalkerPass<PostWalker<ReorderLocals>> {
       }
     }
     // apply the renaming to AST nodes
-    struct ReIndexer : public PostWalker<ReIndexer> {
+    struct ReIndexer : public PostWalker<ReIndexer, Visitor<ReIndexer>> {
       Function* func;
       std::vector<Index>& oldToNew;
 
diff --git a/src/passes/SimplifyLocals.cpp b/src/passes/SimplifyLocals.cpp
index bb5fdd3a5..fbdc248e3 100644
--- a/src/passes/SimplifyLocals.cpp
+++ b/src/passes/SimplifyLocals.cpp
@@ -32,7 +32,7 @@
 
 namespace wasm {
 
-struct SimplifyLocals : public WalkerPass<LinearExecutionWalker<SimplifyLocals>> {
+struct SimplifyLocals : public WalkerPass<LinearExecutionWalker<SimplifyLocals, Visitor<SimplifyLocals>>> {
   bool isFunctionParallel() { return true; }
 
   struct SinkableInfo {
@@ -153,7 +153,7 @@ struct SimplifyLocals : public WalkerPass<LinearExecutionWalker<SimplifyLocals>>
         self->pushTask(scan, &list[i]);
       }
     } else {
-      WalkerPass<LinearExecutionWalker<SimplifyLocals>>::scan(self, currp);
+      WalkerPass<LinearExecutionWalker<SimplifyLocals, Visitor<SimplifyLocals>>>::scan(self, currp);
     }
 
     self->pushTask(visitPre, currp);
@@ -169,7 +169,7 @@ struct SimplifyLocals : public WalkerPass<LinearExecutionWalker<SimplifyLocals>>
       numGetLocals.resize(getFunction()->getNumLocals());
       sunk = false;
       // main operation
-      WalkerPass<LinearExecutionWalker<SimplifyLocals>>::walk(root);
+      WalkerPass<LinearExecutionWalker<SimplifyLocals, Visitor<SimplifyLocals>>>::walk(root);
       // after optimizing a function, we can see if we have set_locals
       // for a local with no remaining gets, in which case, we can
       // remove the set.
diff --git a/src/passes/Vacuum.cpp b/src/passes/Vacuum.cpp
index 060dcd9dc..dfede6d25 100644
--- a/src/passes/Vacuum.cpp
+++ b/src/passes/Vacuum.cpp
@@ -23,7 +23,7 @@
 
 namespace wasm {
 
-struct Vacuum : public WalkerPass<PostWalker<Vacuum>> {
+struct Vacuum : public WalkerPass<PostWalker<Vacuum, Visitor<Vacuum>>> {
   bool isFunctionParallel() { return true; }
 
   void visitBlock(Block *curr) {
diff --git a/src/s2wasm.h b/src/s2wasm.h
index 51d2a0e2f..ff3c1f29f 100644
--- a/src/s2wasm.h
+++ b/src/s2wasm.h
@@ -1396,7 +1396,7 @@ public:
 
     o << ";; METADATA: { ";
     // find asmConst calls, and emit their metadata
-    struct AsmConstWalker : public PostWalker<AsmConstWalker> {
+    struct AsmConstWalker : public PostWalker<AsmConstWalker, Visitor<AsmConstWalker>> {
       S2WasmBuilder* parent;
 
       std::map<std::string, std::set<std::string>> sigsForCode;
diff --git a/src/wasm-traversal.h b/src/wasm-traversal.h
index 21f9b3256..555243621 100644
--- a/src/wasm-traversal.h
+++ b/src/wasm-traversal.h
@@ -100,6 +100,37 @@ struct Visitor {
   }
 };
 
+// Visit with a single unified visitor, called on every node, instead of
+// separate visit* per node
+
+template<typename SubType, typename ReturnType = void>
+struct UnifiedExpressionVisitor : public Visitor<SubType> {
+  // called on each node
+  ReturnType visitExpression(Expression* curr) {}
+
+  // redirects
+  ReturnType visitBlock(Block *curr) { return static_cast<SubType*>(this)->visitExpression(curr); }
+  ReturnType visitIf(If *curr) { return static_cast<SubType*>(this)->visitExpression(curr); }
+  ReturnType visitLoop(Loop *curr) { return static_cast<SubType*>(this)->visitExpression(curr); }
+  ReturnType visitBreak(Break *curr) { return static_cast<SubType*>(this)->visitExpression(curr); }
+  ReturnType visitSwitch(Switch *curr) { return static_cast<SubType*>(this)->visitExpression(curr); }
+  ReturnType visitCall(Call *curr) { return static_cast<SubType*>(this)->visitExpression(curr); }
+  ReturnType visitCallImport(CallImport *curr) { return static_cast<SubType*>(this)->visitExpression(curr); }
+  ReturnType visitCallIndirect(CallIndirect *curr) { return static_cast<SubType*>(this)->visitExpression(curr); }
+  ReturnType visitGetLocal(GetLocal *curr) { return static_cast<SubType*>(this)->visitExpression(curr); }
+  ReturnType visitSetLocal(SetLocal *curr) { return static_cast<SubType*>(this)->visitExpression(curr); }
+  ReturnType visitLoad(Load *curr) { return static_cast<SubType*>(this)->visitExpression(curr); }
+  ReturnType visitStore(Store *curr) { return static_cast<SubType*>(this)->visitExpression(curr); }
+  ReturnType visitConst(Const *curr) { return static_cast<SubType*>(this)->visitExpression(curr); }
+  ReturnType visitUnary(Unary *curr) { return static_cast<SubType*>(this)->visitExpression(curr); }
+  ReturnType visitBinary(Binary *curr) { return static_cast<SubType*>(this)->visitExpression(curr); }
+  ReturnType visitSelect(Select *curr) { return static_cast<SubType*>(this)->visitExpression(curr); }
+  ReturnType visitReturn(Return *curr) { return static_cast<SubType*>(this)->visitExpression(curr); }
+  ReturnType visitHost(Host *curr) { return static_cast<SubType*>(this)->visitExpression(curr); }
+  ReturnType visitNop(Nop *curr) { return static_cast<SubType*>(this)->visitExpression(curr); }
+  ReturnType visitUnreachable(Unreachable *curr) { return static_cast<SubType*>(this)->visitExpression(curr); }
+};
+
 //
 // Base class for all WasmWalkers, which can traverse an AST
 // and provide the option to replace nodes while doing so.
@@ -107,12 +138,8 @@ struct Visitor {
 // Subclass and implement the visit*()
 // calls to run code on different node types.
 //
-template<typename SubType>
-struct Walker : public Visitor<SubType> {
-  // Extra generic visitor, called before each node's specific visitor. Useful for
-  // passes that need to do the same thing for every node type.
-  void visitExpression(Expression* curr) {}
-
+template<typename SubType, typename VisitorType>
+struct Walker : public VisitorType {
   // Function parallelism. By default, walks are not run in parallel, but you
   // can override this method to say that functions are parallelizable. This
   // should always be safe *unless* you do something in the pass that makes it
@@ -245,26 +272,26 @@ struct Walker : public Visitor<SubType> {
 
   // task hooks to call visitors
 
-  static void doVisitBlock(SubType* self, Expression** currp)        { self->visitExpression(*currp); self->visitBlock((*currp)->cast<Block>()); }
-  static void doVisitIf(SubType* self, Expression** currp)           { self->visitExpression(*currp); self->visitIf((*currp)->cast<If>()); }
-  static void doVisitLoop(SubType* self, Expression** currp)         { self->visitExpression(*currp); self->visitLoop((*currp)->cast<Loop>()); }
-  static void doVisitBreak(SubType* self, Expression** currp)        { self->visitExpression(*currp); self->visitBreak((*currp)->cast<Break>()); }
-  static void doVisitSwitch(SubType* self, Expression** currp)       { self->visitExpression(*currp); self->visitSwitch((*currp)->cast<Switch>()); }
-  static void doVisitCall(SubType* self, Expression** currp)         { self->visitExpression(*currp); self->visitCall((*currp)->cast<Call>()); }
-  static void doVisitCallImport(SubType* self, Expression** currp)   { self->visitExpression(*currp); self->visitCallImport((*currp)->cast<CallImport>()); }
-  static void doVisitCallIndirect(SubType* self, Expression** currp) { self->visitExpression(*currp); self->visitCallIndirect((*currp)->cast<CallIndirect>()); }
-  static void doVisitGetLocal(SubType* self, Expression** currp)     { self->visitExpression(*currp); self->visitGetLocal((*currp)->cast<GetLocal>()); }
-  static void doVisitSetLocal(SubType* self, Expression** currp)     { self->visitExpression(*currp); self->visitSetLocal((*currp)->cast<SetLocal>()); }
-  static void doVisitLoad(SubType* self, Expression** currp)         { self->visitExpression(*currp); self->visitLoad((*currp)->cast<Load>()); }
-  static void doVisitStore(SubType* self, Expression** currp)        { self->visitExpression(*currp); self->visitStore((*currp)->cast<Store>()); }
-  static void doVisitConst(SubType* self, Expression** currp)        { self->visitExpression(*currp); self->visitConst((*currp)->cast<Const>()); }
-  static void doVisitUnary(SubType* self, Expression** currp)        { self->visitExpression(*currp); self->visitUnary((*currp)->cast<Unary>()); }
-  static void doVisitBinary(SubType* self, Expression** currp)       { self->visitExpression(*currp); self->visitBinary((*currp)->cast<Binary>()); }
-  static void doVisitSelect(SubType* self, Expression** currp)       { self->visitExpression(*currp); self->visitSelect((*currp)->cast<Select>()); }
-  static void doVisitReturn(SubType* self, Expression** currp)       { self->visitExpression(*currp); self->visitReturn((*currp)->cast<Return>()); }
-  static void doVisitHost(SubType* self, Expression** currp)         { self->visitExpression(*currp); self->visitHost((*currp)->cast<Host>()); }
-  static void doVisitNop(SubType* self, Expression** currp)          { self->visitExpression(*currp); self->visitNop((*currp)->cast<Nop>()); }
-  static void doVisitUnreachable(SubType* self, Expression** currp)  { self->visitExpression(*currp); self->visitUnreachable((*currp)->cast<Unreachable>()); }
+  static void doVisitBlock(SubType* self, Expression** currp)        { self->visitBlock((*currp)->cast<Block>()); }
+  static void doVisitIf(SubType* self, Expression** currp)           { self->visitIf((*currp)->cast<If>()); }
+  static void doVisitLoop(SubType* self, Expression** currp)         { self->visitLoop((*currp)->cast<Loop>()); }
+  static void doVisitBreak(SubType* self, Expression** currp)        { self->visitBreak((*currp)->cast<Break>()); }
+  static void doVisitSwitch(SubType* self, Expression** currp)       { self->visitSwitch((*currp)->cast<Switch>()); }
+  static void doVisitCall(SubType* self, Expression** currp)         { self->visitCall((*currp)->cast<Call>()); }
+  static void doVisitCallImport(SubType* self, Expression** currp)   { self->visitCallImport((*currp)->cast<CallImport>()); }
+  static void doVisitCallIndirect(SubType* self, Expression** currp) { self->visitCallIndirect((*currp)->cast<CallIndirect>()); }
+  static void doVisitGetLocal(SubType* self, Expression** currp)     { self->visitGetLocal((*currp)->cast<GetLocal>()); }
+  static void doVisitSetLocal(SubType* self, Expression** currp)     { self->visitSetLocal((*currp)->cast<SetLocal>()); }
+  static void doVisitLoad(SubType* self, Expression** currp)         { self->visitLoad((*currp)->cast<Load>()); }
+  static void doVisitStore(SubType* self, Expression** currp)        { self->visitStore((*currp)->cast<Store>()); }
+  static void doVisitConst(SubType* self, Expression** currp)        { self->visitConst((*currp)->cast<Const>()); }
+  static void doVisitUnary(SubType* self, Expression** currp)        { self->visitUnary((*currp)->cast<Unary>()); }
+  static void doVisitBinary(SubType* self, Expression** currp)       { self->visitBinary((*currp)->cast<Binary>()); }
+  static void doVisitSelect(SubType* self, Expression** currp)       { self->visitSelect((*currp)->cast<Select>()); }
+  static void doVisitReturn(SubType* self, Expression** currp)       { self->visitReturn((*currp)->cast<Return>()); }
+  static void doVisitHost(SubType* self, Expression** currp)         { self->visitHost((*currp)->cast<Host>()); }
+  static void doVisitNop(SubType* self, Expression** currp)          { self->visitNop((*currp)->cast<Nop>()); }
+  static void doVisitUnreachable(SubType* self, Expression** currp)  { self->visitUnreachable((*currp)->cast<Unreachable>()); }
 
   void setFunction(Function *func) {
     currFunction = func;
@@ -279,8 +306,8 @@ private:
 // Walks in post-order, i.e., children first. When there isn't an obvious
 // order to operands, we follow them in order of execution.
 
-template<typename SubType>
-struct PostWalker : public Walker<SubType> {
+template<typename SubType, typename VisitorType>
+struct PostWalker : public Walker<SubType, VisitorType> {
 
   static void scan(SubType* self, Expression** currp) {
 
@@ -422,8 +449,8 @@ struct PostWalker : public Walker<SubType> {
 // When execution is no longer linear, this notifies via a call
 // to noteNonLinear().
 
-template<typename SubType>
-struct LinearExecutionWalker : public PostWalker<SubType> {
+template<typename SubType, typename VisitorType>
+struct LinearExecutionWalker : public PostWalker<SubType, VisitorType> {
   LinearExecutionWalker() {}
 
   // subclasses should implement this
@@ -486,7 +513,7 @@ struct LinearExecutionWalker : public PostWalker<SubType> {
       }
       default: {
         // other node types do not have control flow, use regular post-order
-        PostWalker<SubType>::scan(self, currp);
+        PostWalker<SubType, VisitorType>::scan(self, currp);
       }
     }
   }
diff --git a/src/wasm-validator.h b/src/wasm-validator.h
index 7b2987dd9..73aa7fecf 100644
--- a/src/wasm-validator.h
+++ b/src/wasm-validator.h
@@ -25,7 +25,7 @@
 
 namespace wasm {
 
-struct WasmValidator : public PostWalker<WasmValidator> {
+struct WasmValidator : public PostWalker<WasmValidator, Visitor<WasmValidator>> {
   bool valid;
 
   std::map<Name, WasmType> breakTypes; // breaks to a label must all have the same type, and the right type
@@ -118,7 +118,7 @@ public:
 private:
 
   // the "in" label has a none type, since no one can receive its value. make sure no one breaks to it with a value.
-  struct LoopChildChecker : public PostWalker<LoopChildChecker> {
+  struct LoopChildChecker : public PostWalker<LoopChildChecker, Visitor<LoopChildChecker>> {
     Name in;
     bool valid = true;
 
diff --git a/src/wasm2asm.h b/src/wasm2asm.h
index 9ccbb4f19..1dbe4f09a 100644
--- a/src/wasm2asm.h
+++ b/src/wasm2asm.h
@@ -392,7 +392,7 @@ Ref Wasm2AsmBuilder::processFunction(Function* func) {
 }
 
 void Wasm2AsmBuilder::scanFunctionBody(Expression* curr) {
-  struct ExpressionScanner : public PostWalker<ExpressionScanner> {
+  struct ExpressionScanner : public PostWalker<ExpressionScanner, Visitor<ExpressionScanner>> {
     Wasm2AsmBuilder* parent;
 
     ExpressionScanner(Wasm2AsmBuilder* parent) : parent(parent) {}