Fix warnings found by GCC

My previous patch addressed all LLVM warnings, this one addresses all the GCC ones as well (mostly signed / unsigned mix).

The patch also turns on -Wall -Werror.

9 files changed, 41 insertions, 33 deletions

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 72ec22970..95c303c94 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -7,7 +7,7 @@ SET(CMAKE_RUNTIME_OUTPUT_DIRECTORY "${PROJECT_BINARY_DIR}/bin")
 
 SET(CMAKE_CXX_FLAGS "-std=c++11 ${CMAKE_CXX_FLAGS}")
 SET(CMAKE_CXX_FLAGS "-msse2 -mfpmath=sse ${CMAKE_CXX_FLAGS}")
-SET(CMAKE_CXX_FLAGS "-O2 ${CMAKE_CXX_FLAGS}")
+SET(CMAKE_CXX_FLAGS "-O2 -Wall -Werror ${CMAKE_CXX_FLAGS}")
 
 # clang doesn't print colored diagnostics when invoked from Ninja
 IF (UNIX AND
diff --git a/src/asm2wasm.h b/src/asm2wasm.h
index 2d92efbfd..0c7f58a04 100644
--- a/src/asm2wasm.h
+++ b/src/asm2wasm.h
@@ -358,7 +358,7 @@ private:
     abort(); // avoid warning
   }
 
-  unsigned bytesToShift(unsigned bytes) {
+  int64_t bytesToShift(unsigned bytes) {
     switch (bytes) {
       case 1: return 0;
       case 2: return 1;
@@ -1402,11 +1402,12 @@ Function* Asm2WasmBuilder::processFunction(Ref ast) {
           assert(index >= min);
           index -= min;
           assert(index >= 0);
+          size_t index_s = index;
           case_.name = getNextId("switch-case");
-          if (ret->targets.size() <= index) {
-            ret->targets.resize(index+1);
+          if (ret->targets.size() <= index_s) {
+            ret->targets.resize(index_s+1);
           }
-          ret->targets[index] = case_.name;
+          ret->targets[index_s] = case_.name;
         }
         ret->cases.push_back(case_);
       }
@@ -1431,7 +1432,7 @@ Function* Asm2WasmBuilder::processFunction(Ref ast) {
   // given HEAP32[addr >> 2], we need an absolute address, and would like to remove that shift.
   // if there is a shift, we can just look through it, etc.
   processUnshifted = [&](Ref ptr, unsigned bytes) {
-    unsigned shifts = bytesToShift(bytes);
+    auto shifts = bytesToShift(bytes);
     if (ptr[0] == BINARY && ptr[1] == RSHIFT && ptr[3][0] == NUM && ptr[3][1]->getInteger() == shifts) {
       return process(ptr[2]); // look through it
     } else if (ptr[0] == NUM) {
diff --git a/src/binaryen-shell.cpp b/src/binaryen-shell.cpp
index 75a138c99..4c4e21f6f 100644
--- a/src/binaryen-shell.cpp
+++ b/src/binaryen-shell.cpp
@@ -181,7 +181,8 @@ int main(int argc, char **argv) {
   bool print_after = false;
   std::vector<std::string> passes;
 
-  for (size_t i = 1; i < argc; i++) {
+  assert(argc > 0 && "expect at least program name as an argument");
+  for (size_t i = 1, e = argc; i != e; i++) {
     char* curr = argv[i];
     if (curr[0] == '-') {
       std::string arg = curr;
diff --git a/src/command-line.h b/src/command-line.h
index 77c10d3e7..28a1aa5b8 100644
--- a/src/command-line.h
+++ b/src/command-line.h
@@ -35,7 +35,8 @@ bool optionIs(const char *arg, const char *LongOpt, const char *ShortOpt) {
 
 // TODO(jfb) Make this configurable: callers should pass in option handlers.
 void processCommandLine(int argc, const char *argv[], Options *options) {
-  for (size_t i = 1; i != argc; ++i) {
+  assert(argc > 0 && "expect at least program name as an argument");
+  for (size_t i = 1, e = argc; i != e; ++i) {
     if (optionIs(argv[i], "--help", "-h")) {
       std::cerr << "s2wasm INFILE\n\n"
                    "Link .s file into .wast\n\n"
@@ -48,7 +49,7 @@ void processCommandLine(int argc, const char *argv[], Options *options) {
     } else if (optionIs(argv[i], "--debug", "-d")) {
       options->debug = true;
     } else if (optionIs(argv[i], "--output", "-o")) {
-      if (i + 1 == argc) {
+      if (i + 1 == e) {
         std::cerr << "No output file" << std::endl;
         exit(EXIT_FAILURE);
       }
diff --git a/src/emscripten-optimizer/simple_ast.h b/src/emscripten-optimizer/simple_ast.h
index cc07d8a12..a6073d9af 100644
--- a/src/emscripten-optimizer/simple_ast.h
+++ b/src/emscripten-optimizer/simple_ast.h
@@ -170,7 +170,7 @@ struct Value {
     *arr = a;
     return *this;
   }
-  Value& setArray(int size_hint=0) {
+  Value& setArray(size_t size_hint=0) {
     free();
     type = Array;
     arr = arena.allocArray();
@@ -395,7 +395,7 @@ struct Value {
           os << std::endl;
           indent++;
         }
-        for (unsigned i = 0; i < arr->size(); i++) {
+        for (size_t i = 0; i < arr->size(); i++) {
           if (i > 0) {
             if (pretty) os << "," << std::endl;
             else os << ", ";
@@ -450,17 +450,17 @@ struct Value {
 
   // Array operations
 
-  unsigned size() {
+  size_t size() {
     assert(isArray());
     return arr->size();
   }
 
-  void setSize(unsigned size) {
+  void setSize(size_t size) {
     assert(isArray());
-    unsigned old = arr->size();
+    auto old = arr->size();
     if (old != size) arr->resize(size);
     if (old < size) {
-      for (unsigned i = old; i < size; i++) {
+      for (auto i = old; i < size; i++) {
         (*arr)[i] = arena.alloc();
       }
     }
@@ -503,7 +503,7 @@ struct Value {
 
   int indexOf(Ref other) {
     assert(isArray());
-    for (unsigned i = 0; i < arr->size(); i++) {
+    for (size_t i = 0; i < arr->size(); i++) {
       if (other == (*arr)[i]) return i;
     }
     return -1;
@@ -513,7 +513,7 @@ struct Value {
     assert(isArray());
     Ref ret = arena.alloc();
     ret->setArray();
-    for (unsigned i = 0; i < arr->size(); i++) {
+    for (size_t i = 0; i < arr->size(); i++) {
       ret->push_back(func((*arr)[i]));
     }
     return ret;
@@ -523,7 +523,7 @@ struct Value {
     assert(isArray());
     Ref ret = arena.alloc();
     ret->setArray();
-    for (unsigned i = 0; i < arr->size(); i++) {
+    for (size_t i = 0; i < arr->size(); i++) {
       Ref curr = (*arr)[i];
       if (func(curr)) ret->push_back(curr);
     }
@@ -532,7 +532,7 @@ struct Value {
 
   /*
   void forEach(std::function<void (Ref)> func) {
-    for (unsigned i = 0; i < arr->size(); i++) {
+    for (size_t i = 0; i < arr->size(); i++) {
       func((*arr)[i]);
     }
   }
@@ -575,7 +575,7 @@ struct JSPrinter {
   bool pretty, finalize;
 
   char *buffer;
-  int size, used;
+  size_t size, used;
 
   int indent;
   bool possibleSpace; // add a space to separate identifiers
@@ -593,18 +593,18 @@ struct JSPrinter {
 
   void ensure(int safety=100) {
     if (size < used + safety) {
-      size = std::max(1024, size*2) + safety;
+      size = std::max((size_t)1024, size * 2) + safety;
       if (!buffer) {
         buffer = (char*)malloc(size);
         if (!buffer) {
-          printf("Out of memory allocating %d bytes for output buffer!", size);
+          printf("Out of memory allocating %zd bytes for output buffer!", size);
           abort();
         }
       } else {
         char *buf = (char*)realloc(buffer, size);
         if (!buf) {
           free(buffer);
-          printf("Out of memory allocating %d bytes for output buffer!", size);
+          printf("Out of memory allocating %zd bytes for output buffer!", size);
           abort();
         }
         buffer = buf;
@@ -745,7 +745,7 @@ struct JSPrinter {
 
   // print a node, and if nothing is emitted, emit something instead
   void print(Ref node, const char *otherwise) {
-    int last = used;
+    auto last = used;
     print(node);
     if (used == last) emit(otherwise);
   }
@@ -1118,7 +1118,7 @@ struct JSPrinter {
       if (c[1]->size() > 0) {
         indent++;
         newline();
-        int curr = used;
+        auto curr = used;
         printStats(c[1]);
         indent--;
         if (curr != used) newline();
diff --git a/src/s2wasm.h b/src/s2wasm.h
index 34afbef52..4f5a05fce 100644
--- a/src/s2wasm.h
+++ b/src/s2wasm.h
@@ -886,7 +886,9 @@ private:
         zero = false;
       } else if (match(".zero")) {
         int32_t size = getInt();
-        for (size_t i = 0; i < size; i++) {
+        if (size <= 0)
+          abort_on(".zero with zero or negative size");
+        for (size_t i = 0, e = size; i < e; i++) {
           raw->push_back(0);
         }
       } else if (match(".int32")) {
diff --git a/src/wasm-interpreter.h b/src/wasm-interpreter.h
index 94077c00a..185e920ca 100644
--- a/src/wasm-interpreter.h
+++ b/src/wasm-interpreter.h
@@ -248,7 +248,7 @@ private:
         NOTE_EVAL1(flow.value);
         int64_t index = flow.value.getInteger();
         Name target = curr->default_;
-        if (index >= 0 && index < curr->targets.size()) {
+        if (index >= 0 && (size_t)index < curr->targets.size()) {
           target = curr->targets[index];
         }
         // This is obviously very inefficient. This should be a cached data structure
diff --git a/src/wasm.h b/src/wasm.h
index 9571c0e7e..7baec88aa 100644
--- a/src/wasm.h
+++ b/src/wasm.h
@@ -52,6 +52,7 @@
 #include <map>
 #include <vector>
 
+#include "compiler-support.h"
 #include "emscripten-optimizer/simple_ast.h"
 #include "pretty_printing.h"
 
@@ -95,6 +96,7 @@ inline const char* printWasmType(WasmType type) {
     case WasmType::i64: return "i64";
     case WasmType::f32: return "f32";
     case WasmType::f64: return "f64";
+    default: WASM_UNREACHABLE();
   }
 }
 
@@ -105,6 +107,7 @@ inline unsigned getWasmTypeSize(WasmType type) {
     case WasmType::i64: return 8;
     case WasmType::f32: return 4;
     case WasmType::f64: return 8;
+    default: WASM_UNREACHABLE();
   }
 }
 
diff --git a/src/wasm2asm.h b/src/wasm2asm.h
index 3fe0a474f..b8b897ef9 100644
--- a/src/wasm2asm.h
+++ b/src/wasm2asm.h
@@ -44,7 +44,7 @@ void flattenAppend(Ref ast, Ref extra) {
   else if (ast[0] == DEFUN) index = 3;
   else abort();
   if (extra[0] == BLOCK) {
-    for (int i = 0; i < extra[1]->size(); i++) {
+    for (size_t i = 0; i < extra[1]->size(); i++) {
       ast[index]->push_back(extra[1][i]);
     }
   } else {
@@ -167,7 +167,7 @@ public:
 
 private:
   // How many temp vars we need
-  std::vector<int> temps; // type => num temps
+  std::vector<size_t> temps; // type => num temps
   // Which are currently free to use
   std::vector<std::vector<IString>> frees; // type => list of free names
 
@@ -283,7 +283,7 @@ void Wasm2AsmBuilder::addTables(Ref ast, Module *wasm) {
     if (table.size() == 0) {
       // fill it with the first of its type seen. we have to fill with something; and for asm2wasm output, the first is the null anyhow
       table.resize(tableSize);
-      for (int j = 0; j < tableSize; j++) {
+      for (size_t j = 0; j < tableSize; j++) {
         table[j] = fromName(name);
       }
     } else {
@@ -578,7 +578,7 @@ Ref Wasm2AsmBuilder::processFunctionBody(Expression* curr, IString result) {
       breakResults[curr->name] = result;
       Ref ret = ValueBuilder::makeBlock();
       size_t size = curr->list.size();
-      int noResults = result == NO_RESULT ? size : size-1;
+      auto noResults = result == NO_RESULT ? size : size-1;
       for (size_t i = 0; i < noResults; i++) {
         flattenAppend(ret, ValueBuilder::makeStatement(visit(curr->list[i], NO_RESULT)));
       }
@@ -769,7 +769,7 @@ Ref Wasm2AsmBuilder::processFunctionBody(Expression* curr, IString result) {
         switch (curr->type) {
           case i32: {
             rest = makeAsmCoercion(visit(&load, EXPRESSION_RESULT), ASM_INT);
-            for (int i = 1; i < curr->bytes; i++) {
+            for (size_t i = 1; i < curr->bytes; i++) {
               load.offset += 1;
               Ref add = makeAsmCoercion(visit(&load, EXPRESSION_RESULT), ASM_INT);
               add = ValueBuilder::makeBinary(add, LSHIFT, ValueBuilder::makeNum(8*i));
@@ -846,7 +846,7 @@ Ref Wasm2AsmBuilder::processFunctionBody(Expression* curr, IString result) {
             Const _255;
             _255.value = Literal(int32_t(255));
             _255.type = i32;
-            for (int i = 0; i < curr->bytes; i++) {
+            for (size_t i = 0; i < curr->bytes; i++) {
               Const shift;
               shift.value = Literal(int32_t(8*i));
               shift.type = i32;