Some cleanup.

12 files changed, 51 insertions, 70 deletions

diff --git a/src/asm2wasm.h b/src/asm2wasm.h
index b968b8596..7fa252c8c 100644
--- a/src/asm2wasm.h
+++ b/src/asm2wasm.h
@@ -1489,7 +1489,7 @@ Function* Asm2WasmBuilder::processFunction(Ref ast) {
 void Asm2WasmBuilder::optimize() {
   // Optimization passes. Note: no effort is made to free nodes that are no longer held on to.
 
-  struct BlockBreakOptimizer : public WasmWalker<BlockBreakOptimizer, void> {
+  struct BlockBreakOptimizer : public WasmWalker<BlockBreakOptimizer> {
     void visitBlock(Block *curr) {
       // if the block ends in a break on this very block, then just put the value there
       Break *last = curr->list[curr->list.size()-1]->dyn_cast<Break>();
@@ -1504,7 +1504,7 @@ void Asm2WasmBuilder::optimize() {
       }
       // we might be broken to, but maybe there isn't a break (and we may have removed it, leading to this)
 
-      struct BreakSeeker : public WasmWalker<BreakSeeker, void> {
+      struct BreakSeeker : public WasmWalker<BreakSeeker> {
         IString target; // look for this one
         size_t found;
 
diff --git a/src/pass.h b/src/pass.h
index 82b3e3c40..f8b56aff6 100644
--- a/src/pass.h
+++ b/src/pass.h
@@ -118,7 +118,7 @@ public:
 // e.g. through PassRunner::getLast
 
 // Handles names in a module, in particular adding names without duplicates
-class NameManager : public WalkerPass<WasmWalker<NameManager, void> > {
+class NameManager : public WalkerPass<WasmWalker<NameManager>> {
  public:
   Name getUnique(std::string prefix);
   // TODO: getUniqueInFunction
diff --git a/src/passes/LowerInt64.cpp b/src/passes/LowerInt64.cpp
index 09daa9934..58e56cba6 100644
--- a/src/passes/LowerInt64.cpp
+++ b/src/passes/LowerInt64.cpp
@@ -178,27 +178,6 @@ struct LowerInt64 : public Pass {
       replaceCurrent(ret);
     }
   }
-  void visitConst(Const *curr) {
-  }
-  void visitUnary(Unary *curr) {
-  }
-  void visitBinary(Binary *curr) {
-  }
-  void visitSelect(Select *curr) {
-  }
-  void visitHost(Host *curr) {
-  }
-  void visitNop(Nop *curr) {
-  }
-  void visitUnreachable(Unreachable *curr) {
-  }
-
-  void visitFunctionType(FunctionType *curr) {
-  }
-  void visitImport(Import *curr) {
-  }
-  void visitExport(Export *curr) {
-  }
   void visitFunction(Function *curr) {
     // TODO: new params
     for (auto localPair : locals) { // TODO: ignore params
@@ -207,12 +186,6 @@ struct LowerInt64 : public Pass {
     fixes.clear();
     locals.clear();
   }
-  void visitTable(Table *curr) {
-  }
-  void visitMemory(Memory *curr) {
-  }
-  void visitModule(Module *curr) {
-  }
 };
 
 static RegisterPass<LowerInt64> registerPass("lower-i64", "lowers i64 into pairs of i32s");
diff --git a/src/passes/MergeBlocks.cpp b/src/passes/MergeBlocks.cpp
index 60b865777..c85f3d600 100644
--- a/src/passes/MergeBlocks.cpp
+++ b/src/passes/MergeBlocks.cpp
@@ -23,7 +23,7 @@
 
 namespace wasm {
 
-struct MergeBlocks : public WalkerPass<WasmWalker<MergeBlocks, void> > {
+struct MergeBlocks : public WalkerPass<WasmWalker<MergeBlocks>> {
   void visitBlock(Block *curr) {
     bool more = true;
     while (more) {
diff --git a/src/passes/RemoveImports.cpp b/src/passes/RemoveImports.cpp
index edcd7b580..cd741180f 100644
--- a/src/passes/RemoveImports.cpp
+++ b/src/passes/RemoveImports.cpp
@@ -27,7 +27,7 @@
 
 namespace wasm {
 
-struct RemoveImports : public WalkerPass<WasmWalker<RemoveImports, void> > {
+struct RemoveImports : public WalkerPass<WasmWalker<RemoveImports>> {
   MixedArena* allocator;
   std::map<Name, Import*> importsMap;
 
diff --git a/src/passes/RemoveUnusedBrs.cpp b/src/passes/RemoveUnusedBrs.cpp
index 4bfc368f8..245642055 100644
--- a/src/passes/RemoveUnusedBrs.cpp
+++ b/src/passes/RemoveUnusedBrs.cpp
@@ -23,7 +23,7 @@
 
 namespace wasm {
 
-struct RemoveUnusedBrs : public WalkerPass<WasmWalker<RemoveUnusedBrs, void> > {
+struct RemoveUnusedBrs : public WalkerPass<WasmWalker<RemoveUnusedBrs>> {
   // preparation: try to unify branches, as the fewer there are, the higher a chance we can remove them
   // specifically for if-else, turn an if-else with branches to the same target at the end of each
   // child, and with a value, to a branch to that target containing the if-else
diff --git a/src/passes/RemoveUnusedNames.cpp b/src/passes/RemoveUnusedNames.cpp
index 8ebcdd1d3..6929aa671 100644
--- a/src/passes/RemoveUnusedNames.cpp
+++ b/src/passes/RemoveUnusedNames.cpp
@@ -23,7 +23,7 @@
 
 namespace wasm {
 
-struct RemoveUnusedNames : public WalkerPass<WasmWalker<RemoveUnusedNames, void> > {
+struct RemoveUnusedNames : public WalkerPass<WasmWalker<RemoveUnusedNames>> {
   // We maintain a list of branches that we saw in children, then when we reach
   // a parent block, we know if it was branched to
   std::set<Name> branchesSeen;
diff --git a/src/passes/SimplifyLocals.cpp b/src/passes/SimplifyLocals.cpp
index 6d16dfafe..cbfc0dd66 100644
--- a/src/passes/SimplifyLocals.cpp
+++ b/src/passes/SimplifyLocals.cpp
@@ -23,7 +23,7 @@
 
 namespace wasm {
 
-struct SimplifyLocals : public WalkerPass<WasmWalker<SimplifyLocals, void> > {
+struct SimplifyLocals : public WalkerPass<WasmWalker<SimplifyLocals>> {
   void visitBlock(Block *curr) {
     // look for pairs of setlocal-getlocal, which can be just a setlocal (since it returns a value)
     if (curr->list.size() == 0) return;
diff --git a/src/s2wasm.h b/src/s2wasm.h
index e22e76289..bb4ca78d8 100644
--- a/src/s2wasm.h
+++ b/src/s2wasm.h
@@ -1102,7 +1102,7 @@ public:
 
     o << ";; METADATA: { ";
     // find asmConst calls, and emit their metadata
-    struct AsmConstWalker : public WasmWalker<AsmConstWalker, void> {
+    struct AsmConstWalker : public WasmWalker<AsmConstWalker> {
       S2WasmBuilder* parent;
 
       std::map<std::string, std::set<std::string>> sigsForCode;
diff --git a/src/wasm-validator.h b/src/wasm-validator.h
index 237942047..ba4e5fe08 100644
--- a/src/wasm-validator.h
+++ b/src/wasm-validator.h
@@ -25,7 +25,7 @@
 
 namespace wasm {
 
-struct WasmValidator : public WasmWalker<WasmValidator, void> {
+struct WasmValidator : public WasmWalker<WasmValidator> {
   bool valid;
 
 public:
@@ -98,7 +98,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 WasmWalker<LoopChildChecker, void> {
+  struct LoopChildChecker : public WasmWalker<LoopChildChecker> {
     Name in;
     bool valid = true;
 
diff --git a/src/wasm.h b/src/wasm.h
index a5ad3fe77..72ebe98ee 100644
--- a/src/wasm.h
+++ b/src/wasm.h
@@ -1132,9 +1132,13 @@ class AllocatingModule : public Module {
 };
 
 //
-// Simple WebAssembly AST visiting. Useful for anything that wants to do
-// something different for each AST node type, like printing, interpreting,
-// etc.
+// WebAssembly AST visitor. Useful for anything that wants to do something
+// different for each AST node type, like printing, interpreting, etc.
+//
+// This class is specifically designed as a template to avoid virtual function
+// call overhead. To write a visitor, derive from this class as follows:
+//
+//   struct MyVisitor : public WasmVisitor<MyVisitor> { .. }
 //
 
 template<typename SubType, typename ReturnType>
@@ -1170,33 +1174,33 @@ struct WasmVisitor {
   ReturnType visitMemory(Memory *curr) { abort(); }
   ReturnType visitModule(Module *curr) { abort(); }
 
+#define DELEGATE(CLASS_TO_VISIT) \
+  return static_cast<SubType*>(this)-> \
+      visit##CLASS_TO_VISIT(static_cast<CLASS_TO_VISIT*>(curr))
+
   ReturnType visit(Expression *curr) {
     assert(curr);
-    SubType* self = static_cast<SubType*>(this);
     switch (curr->_id) {
-      case Expression::Id::BlockId: return self->visitBlock((Block*)curr);
-      case Expression::Id::IfId: return self->visitIf((If*)curr);
-      case Expression::Id::LoopId: return self->visitLoop((Loop*)curr);
-      case Expression::Id::BreakId: return self->visitBreak((Break*)curr);
-      case Expression::Id::SwitchId: return self->visitSwitch((Switch*)curr);
-      case Expression::Id::CallId: return self->visitCall((Call*)curr);
-      case Expression::Id::CallImportId: return self->visitCallImport((CallImport*)curr);
-      case Expression::Id::CallIndirectId: return self->visitCallIndirect((CallIndirect*)curr);
-      case Expression::Id::GetLocalId: return self->visitGetLocal((GetLocal*)curr);
-      case Expression::Id::SetLocalId: return self->visitSetLocal((SetLocal*)curr);
-      case Expression::Id::LoadId: return self->visitLoad((Load*)curr);
-      case Expression::Id::StoreId: return self->visitStore((Store*)curr);
-      case Expression::Id::ConstId: return self->visitConst((Const*)curr);
-      case Expression::Id::UnaryId: return self->visitUnary((Unary*)curr);
-      case Expression::Id::BinaryId: return self->visitBinary((Binary*)curr);
-      case Expression::Id::SelectId: return self->visitSelect((Select*)curr);
-      case Expression::Id::HostId: return self->visitHost((Host*)curr);
-      case Expression::Id::NopId: return self->visitNop((Nop*)curr);
-      case Expression::Id::UnreachableId: return self->visitUnreachable((Unreachable*)curr);
-      default: {
-        std::cerr << "visiting unknown expression " << curr->_id << '\n';
-        abort();
-      }
+      case Expression::Id::InvalidId: abort();
+      case Expression::Id::BlockId: DELEGATE(Block);
+      case Expression::Id::IfId: DELEGATE(If);
+      case Expression::Id::LoopId: DELEGATE(Loop);
+      case Expression::Id::BreakId: DELEGATE(Break);
+      case Expression::Id::SwitchId: DELEGATE(Switch);
+      case Expression::Id::CallId: DELEGATE(Call);
+      case Expression::Id::CallImportId: DELEGATE(CallImport);
+      case Expression::Id::CallIndirectId: DELEGATE(CallIndirect);
+      case Expression::Id::GetLocalId: DELEGATE(GetLocal);
+      case Expression::Id::SetLocalId: DELEGATE(SetLocal);
+      case Expression::Id::LoadId: DELEGATE(Load);
+      case Expression::Id::StoreId: DELEGATE(Store);
+      case Expression::Id::ConstId: DELEGATE(Const);
+      case Expression::Id::UnaryId: DELEGATE(Unary);
+      case Expression::Id::BinaryId: DELEGATE(Binary);
+      case Expression::Id::SelectId: DELEGATE(Select);
+      case Expression::Id::HostId: DELEGATE(Host);
+      case Expression::Id::NopId: DELEGATE(Nop);
+      case Expression::Id::UnreachableId: DELEGATE(Unreachable);
     }
   }
 };
@@ -1234,7 +1238,10 @@ std::ostream& Expression::print(std::ostream &o, unsigned indent) {
   return o;
 }
 
-template<typename SubType, typename ReturnType>
+//
+// Base class for all WasmWalkers
+//
+template<typename SubType, typename ReturnType = void>
 struct WasmWalkerBase : public WasmVisitor<SubType, ReturnType> {
   virtual void walk(Expression*& curr) { abort(); }
   virtual void startWalk(Function *func) { abort(); }
@@ -1242,7 +1249,7 @@ struct WasmWalkerBase : public WasmVisitor<SubType, ReturnType> {
 };
 
 template<typename ParentType>
-struct ChildWalker : public WasmWalkerBase<ChildWalker<ParentType>, void> {
+struct ChildWalker : public WasmWalkerBase<ChildWalker<ParentType>> {
   ParentType& parent;
 
   ChildWalker(ParentType& parent) : parent(parent) {}
@@ -1330,7 +1337,7 @@ struct ChildWalker : public WasmWalkerBase<ChildWalker<ParentType>, void> {
 // the current expression node. Useful for writing optimization passes.
 //
 
-template<typename SubType, typename ReturnType>
+template<typename SubType, typename ReturnType = void>
 struct WasmWalker : public WasmWalkerBase<SubType, ReturnType> {
   Expression* replace;
 
@@ -1374,7 +1381,7 @@ struct WasmWalker : public WasmWalkerBase<SubType, ReturnType> {
   void walk(Expression*& curr) override {
     if (!curr) return;
 
-    ChildWalker<WasmWalker<SubType, ReturnType> >(*this).visit(curr);
+    ChildWalker<WasmWalker<SubType, ReturnType>>(*this).visit(curr);
 
     this->visit(curr);
 
@@ -1389,6 +1396,7 @@ struct WasmWalker : public WasmWalkerBase<SubType, ReturnType> {
   }
 
   void startWalk(Module *module) override {
+    // Dispatch statically through the SubType.
     SubType* self = static_cast<SubType*>(this);
     for (auto curr : module->functionTypes) {
       self->visitFunctionType(curr);
diff --git a/src/wasm2asm.h b/src/wasm2asm.h
index 4d831f273..f1a58e170 100644
--- a/src/wasm2asm.h
+++ b/src/wasm2asm.h
@@ -387,7 +387,7 @@ Ref Wasm2AsmBuilder::processFunction(Function* func) {
 }
 
 void Wasm2AsmBuilder::scanFunctionBody(Expression* curr) {
-  struct ExpressionScanner : public WasmWalker<ExpressionScanner, void> {
+  struct ExpressionScanner : public WasmWalker<ExpressionScanner> {
     Wasm2AsmBuilder* parent;
 
     ExpressionScanner(Wasm2AsmBuilder* parent) : parent(parent) {}