cleanup to allow binaryen to be built in more strict environments (WebAssembly#3566)

walkingeyerobot · web-flow · commit a74bc72dbe6f · 2021-02-16T10:37:16.000-08:00
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -233,6 +233,9 @@ else()
   add_compile_flag("-Wno-implicit-int-float-conversion")
   add_compile_flag("-Wno-unknown-warning-option")
   add_compile_flag("-Wswitch") # we explicitly expect this in the code
+  add_compile_flag("-Wimplicit-fallthrough")
+  add_compile_flag("-Wnon-virtual-dtor")
+
   if(WIN32)
     add_compile_flag("-D_GNU_SOURCE")
     add_compile_flag("-D__STDC_FORMAT_MACROS")
diff --git a/README.md b/README.md
@@ -484,8 +484,9 @@ Emscripten's WebAssembly processing library (`wasm-emscripten`).
 * Does it compile under Windows and/or Visual Studio?
 
 Yes, it does. Here's a step-by-step [tutorial][win32]  on how to compile it
-under **Windows 10 x64** with with **CMake** and **Visual Studio 2015**. Help
-would be appreciated on Windows and OS X as most of the core devs are on Linux.
+under **Windows 10 x64** with with **CMake** and **Visual Studio 2015**. 
+However, Visual Studio 2017 may now be required. Help would be appreciated on
+Windows and OS X as most of the core devs are on Linux.
 
 [compiling to WebAssembly]: https://github.com/WebAssembly/binaryen/wiki/Compiling-to-WebAssembly-with-Binaryen
 [win32]: https://github.com/brakmic/bazaar/blob/master/webassembly/COMPILING_WIN32.md
diff --git a/src/dataflow/node.h b/src/dataflow/node.h
@@ -183,10 +183,12 @@ struct Node {
         }
         break;
       }
-      case Cond:
+      case Cond: {
         if (index != other.index) {
           return false;
         }
+        break;
+      }
       default: {}
     }
     if (values.size() != other.values.size()) {
diff --git a/src/dataflow/utils.h b/src/dataflow/utils.h
@@ -25,6 +25,7 @@
 #ifndef wasm_dataflow_utils_h
 #define wasm_dataflow_utils_h
 
+#include "dataflow/graph.h"
 #include "dataflow/node.h"
 #include "wasm.h"
 
diff --git a/src/emscripten-optimizer/istring.h b/src/emscripten-optimizer/istring.h
@@ -48,11 +48,11 @@ struct IString {
     return (size_t)hash;
   }
 
-  class CStringHash : public std::hash<const char*> {
+  class CStringHash {
   public:
     size_t operator()(const char* str) const { return IString::hash_c(str); }
   };
-  class CStringEqual : public std::equal_to<const char*> {
+  class CStringEqual {
   public:
     bool operator()(const char* x, const char* y) const {
       return strcmp(x, y) == 0;
diff --git a/src/emscripten-optimizer/optimizer-shared.cpp b/src/emscripten-optimizer/optimizer-shared.cpp
@@ -205,7 +205,8 @@ AsmSign detectSign(Ref node, IString minifiedFround) {
         if (op == TRSHIFT) {
           return ASM_UNSIGNED;
         }
-      } // fallthrough
+        [[fallthrough]];
+      }
       case '|':
       case '&':
       case '^':
diff --git a/src/ir/parents.h b/src/ir/parents.h
@@ -17,6 +17,8 @@
 #ifndef wasm_ir_parents_h
 #define wasm_ir_parents_h
 
+#include "parsing.h"
+
 namespace wasm {
 
 struct Parents {
diff --git a/src/passes/FuncCastEmulation.cpp b/src/passes/FuncCastEmulation.cpp
@@ -117,6 +117,7 @@ static Expression* fromABI(Expression* value, Type type, Module* module) {
     }
     case Type::none: {
       value = builder.makeDrop(value);
+      break;
     }
     case Type::unreachable: {
       // can leave it, the call isn't taken anyhow
diff --git a/src/passes/OptimizeInstructions.cpp b/src/passes/OptimizeInstructions.cpp
@@ -2204,6 +2204,7 @@ struct OptimizeInstructions
                 Type::v128);
             }
           }
+          break;
         }
         default: {
         }
diff --git a/src/passes/Print.cpp b/src/passes/Print.cpp
@@ -3415,7 +3415,7 @@ printStackIR(StackIR* ir, std::ostream& o, Function* func) {
       }
       case StackInst::TryBegin:
         catchIndexStack.push_back(0);
-        // fallthrough
+        [[fallthrough]];
       case StackInst::BlockBegin:
       case StackInst::IfBegin:
       case StackInst::LoopBegin: {
@@ -3427,7 +3427,7 @@ printStackIR(StackIR* ir, std::ostream& o, Function* func) {
       }
       case StackInst::TryEnd:
         catchIndexStack.pop_back();
-        // fallthrough
+        [[fallthrough]];
       case StackInst::BlockEnd:
       case StackInst::IfEnd:
       case StackInst::LoopEnd: {
diff --git a/src/passes/ReReloop.cpp b/src/passes/ReReloop.cpp
@@ -94,6 +94,7 @@ struct ReReloop final : public Pass {
   // we work using a stack of control flow tasks
 
   struct Task {
+    virtual ~Task() = default;
     ReReloop& parent;
     Task(ReReloop& parent) : parent(parent) {}
     virtual void run() { WASM_UNREACHABLE("unimpl"); }
diff --git a/src/support/command-line.cpp b/src/support/command-line.cpp
@@ -146,7 +146,7 @@ void Options::parse(int argc, const char* argv[]) {
                       << currentOption << "' for " << positionalName << '\n';
             exit(EXIT_FAILURE);
           }
-        // Fallthrough.
+          [[fallthrough]];
         case Arguments::N:
           positionalAction(this, currentOption);
           ++positionalsSeen;
@@ -186,7 +186,7 @@ void Options::parse(int argc, const char* argv[]) {
                     << currentOption << "'\n";
           exit(EXIT_FAILURE);
         }
-      // Fallthrough.
+        [[fallthrough]];
       case Arguments::N:
         if (!argument.size()) {
           if (i + 1 == e) {
diff --git a/src/support/sorted_vector.h b/src/support/sorted_vector.h
@@ -21,6 +21,7 @@
 #ifndef wasm_support_sorted_vector_h
 #define wasm_support_sorted_vector_h
 
+#include "wasm.h"
 #include <vector>
 
 namespace wasm {
diff --git a/src/support/string.h b/src/support/string.h
@@ -21,6 +21,7 @@
 #ifndef wasm_support_string_h
 #define wasm_support_string_h
 
+#include "support/utilities.h"
 #include <cctype>
 #include <string>
 #include <vector>
diff --git a/src/support/timing.h b/src/support/timing.h
@@ -22,6 +22,8 @@
 #define wasm_support_timing_h
 
 #include <chrono>
+#include <iostream>
+#include <string>
 
 namespace wasm {
 
diff --git a/src/tools/js-wrapper.h b/src/tools/js-wrapper.h
@@ -19,6 +19,10 @@
 // values, useful for fuzzing.
 //
 
+#include "wasm-type.h"
+#include "wasm.h"
+#include <string>
+
 namespace wasm {
 
 static std::string generateJSWrapper(Module& wasm) {
diff --git a/src/tools/spec-wrapper.h b/src/tools/spec-wrapper.h
@@ -19,6 +19,9 @@
 // values, useful for fuzzing.
 //
 
+#include "wasm-type.h"
+#include "wasm.h"
+
 namespace wasm {
 
 static std::string generateSpecWrapper(Module& wasm) {
diff --git a/src/tools/wasm2c-wrapper.h b/src/tools/wasm2c-wrapper.h
@@ -19,6 +19,7 @@
 // wasm2c, useful for fuzzing.
 //
 
+#include <sstream>
 #include <string>
 
 #include "wasm.h"
diff --git a/src/wasm-interpreter.h b/src/wasm-interpreter.h
@@ -192,6 +192,7 @@ class ExpressionRunner : public OverriddenVisitor<SubType, Flow> {
                    Index maxLoopIterations = NO_LIMIT)
     : module(module), maxDepth(maxDepth), maxLoopIterations(maxLoopIterations) {
   }
+  virtual ~ExpressionRunner() = default;
 
   Flow visit(Expression* curr) {
     depth++;
@@ -2066,6 +2067,7 @@ template<typename GlobalManager, typename SubType> class ModuleInstanceBase {
   // an imported function or accessing memory.
   //
   struct ExternalInterface {
+    virtual ~ExternalInterface() = default;
     virtual void init(Module& wasm, SubType& instance) {}
     virtual void importGlobals(GlobalManager& globals, Module& wasm) = 0;
     virtual Literals callImport(Function* import, LiteralList& arguments) = 0;
diff --git a/src/wasm-module-building.h b/src/wasm-module-building.h
@@ -17,6 +17,7 @@
 #ifndef wasm_wasm_module_building_h
 #define wasm_wasm_module_building_h
 
+#include "pass.h"
 #include <support/threads.h>
 #include <wasm.h>
 
diff --git a/src/wasm/wasm-stack.cpp b/src/wasm/wasm-stack.cpp
@@ -2302,7 +2302,7 @@ void StackIRToBinaryWriter::write() {
     switch (inst->op) {
       case StackInst::TryBegin:
         catchIndexStack.push_back(0);
-        // fallthrough
+        [[fallthrough]];
       case StackInst::Basic:
       case StackInst::BlockBegin:
       case StackInst::IfBegin:
@@ -2312,7 +2312,7 @@ void StackIRToBinaryWriter::write() {
       }
       case StackInst::TryEnd:
         catchIndexStack.pop_back();
-        // fallthrough
+        [[fallthrough]];
       case StackInst::BlockEnd:
       case StackInst::IfEnd:
       case StackInst::LoopEnd: {
diff --git a/src/wasm2js.h b/src/wasm2js.h
@@ -293,7 +293,7 @@ class Wasm2JSBuilder {
 
   // Mangled names cache by interned names.
   // Utilizes the usually reused underlying cstring's pointer as the key.
-  std::unordered_map<const char*, IString>
+  std::unordered_map<const void*, IString>
     wasmNameToMangledName[(int)NameScope::Max];
   // Set of all mangled names in each scope.
   std::unordered_set<IString> mangledNames[(int)NameScope::Max];

Original file line number	Diff line number	Diff line change
`@@ -183,10 +183,12 @@ struct Node {`
`183`	`183`	`}`
`184`	`184`	`break;`
`185`	`185`	`}`
`186`		`- case Cond:`
	`186`	`+ case Cond: {`
`187`	`187`	`if (index != other.index) {`
`188`	`188`	`return false;`
`189`	`189`	`}`
	`190`	`+ break;`
	`191`	`+ }`
`190`	`192`	`default: {}`
`191`	`193`	`}`
`192`	`194`	`if (values.size() != other.values.size()) {`
Original file line number	Diff line number	Diff line change
`@@ -117,6 +117,7 @@ static Expression* fromABI(Expression* value, Type type, Module* module) {`
`117`	`117`	`}`
`118`	`118`	`case Type::none: {`
`119`	`119`	`value = builder.makeDrop(value);`
	`120`	`+ break;`
`120`	`121`	`}`
`121`	`122`	`case Type::unreachable: {`
`122`	`123`	`// can leave it, the call isn't taken anyhow`
Original file line number	Diff line number	Diff line change
`@@ -2204,6 +2204,7 @@ struct OptimizeInstructions`
`2204`	`2204`	`Type::v128);`
`2205`	`2205`	`}`
`2206`	`2206`	`}`
	`2207`	`+ break;`
`2207`	`2208`	`}`
`2208`	`2209`	`default: {`
`2209`	`2210`	`}`