Merge pull request #493 from WebAssembly/error-improvements

Error handling improvements

3 files changed, 62 insertions, 20 deletions

diff --git a/src/wasm-s-parser.h b/src/wasm-s-parser.h
index e0f85227d..4dfba25f8 100644
--- a/src/wasm-s-parser.h
+++ b/src/wasm-s-parser.h
@@ -60,8 +60,10 @@ class Element {
   IString str_;
   bool dollared_;
 
+  size_t line, col;
+
 public:
-  Element(MixedArena& allocator) : isList_(true), list_(allocator) {}
+  Element(MixedArena& allocator) : isList_(true), list_(allocator), line(-1), col(-1) {}
 
   bool isList() { return isList_; }
   bool isStr() { return !isList_; }
@@ -70,7 +72,7 @@ public:
   // list methods
 
   List& list() {
-    assert(isList_);
+    if (!isList()) throw ParseException("expected list", line, col);
     return list_;
   }
 
@@ -101,6 +103,12 @@ public:
     return this;
   }
 
+  Element* setMetadata(size_t line_, size_t col_) {
+    line = line_;
+    col = col_;
+    return this;
+  }
+
   // printing
 
   friend std::ostream& operator<<(std::ostream& o, Element& e) {
@@ -125,6 +133,8 @@ public:
 
 class SExpressionParser {
   char* input;
+  size_t line;
+  char* lineStart;
 
   MixedArena allocator;
 
@@ -132,6 +142,8 @@ public:
   // Assumes control of and modifies the input.
   SExpressionParser(char* input) : input(input) {
     root = nullptr;
+    line = 0;
+    lineStart = input;
     while (!root) { // keep parsing until we pass an initial comment
       root = parse();
     }
@@ -145,12 +157,11 @@ private:
     Element *curr = allocator.alloc<Element>();
     while (1) {
       skipWhitespace();
-      if (input[0] == 0)
-        break;
+      if (input[0] == 0) break;
       if (input[0] == '(') {
         input++;
         stack.push_back(curr);
-        curr = allocator.alloc<Element>();
+        curr = allocator.alloc<Element>()->setMetadata(line, input - lineStart - 1);
       } else if (input[0] == ')') {
         input++;
         auto last = curr;
@@ -168,9 +179,17 @@ private:
 
   void skipWhitespace() {
     while (1) {
-      while (isspace(input[0])) input++;
+      while (isspace(input[0])) {
+        if (input[0] == '\n') {
+          line++;
+          lineStart = input + 1;
+        }
+        input++;
+      }
       if (input[0] == ';' && input[1] == ';') {
         while (input[0] && input[0] != '\n') input++;
+        line++;
+        lineStart = input;
       } else if (input[0] == '(' && input[1] == ';') {
         // Skip nested block comments.
         input += 2;
@@ -221,12 +240,12 @@ private:
         input++;
       }
       input++;
-      return allocator.alloc<Element>()->setString(IString(str.c_str(), false), dollared);
+      return allocator.alloc<Element>()->setString(IString(str.c_str(), false), dollared)->setMetadata(line, start - lineStart);
     }
-    while (input[0] && !isspace(input[0]) && input[0] != ')' && input[0] != '(') input++;
+    while (input[0] && !isspace(input[0]) && input[0] != ')' && input[0] != '(' && input[0] != ';') input++;
     char temp = input[0];
     input[0] = 0;
-    auto ret = allocator.alloc<Element>()->setString(IString(start, false), dollared); // TODO: reuse the string here, carefully
+    auto ret = allocator.alloc<Element>()->setString(IString(start, false), dollared)->setMetadata(line, start - lineStart);
     input[0] = temp;
     return ret;
   }
@@ -354,14 +373,29 @@ private:
 
   void parseFunction(Element& s) {
     size_t i = 1;
-    Name name;
+    Name name, exportName;
     if (s[i]->isStr()) {
-      name = s[i]->str();
-      i++;
-    } else {
+      if (!s[i]->dollared()) {
+        // an export name
+        exportName = s[i]->str();
+        i++;
+      }
+      if (s[i]->isStr()) {
+        assert(s[i]->dollared());
+        name = s[i]->str();
+        i++;
+      }
+    }
+    if (!name.is()) {
       // unnamed, use an index
       name = Name::fromInt(functionCounter);
     }
+    if (exportName.is()) {
+      auto ex = make_unique<Export>();
+      ex->name = exportName;
+      ex->value = name;
+      wasm.addExport(ex.release());
+    }
     functionCounter++;
     Expression* body = nullptr;
     localIndex = 0;
@@ -906,7 +940,7 @@ private:
       bool explicitThenElse = false;
       if (s[0]->str() == THEN || s[0]->str() == ELSE) {
         explicitThenElse = true;
-        if (s[1]->dollared()) {
+        if (s[1]->isStr() && s[1]->dollared()) {
           name = s[1]->str();
         }
       }
diff --git a/src/wasm-validator.h b/src/wasm-validator.h
index 49e2eb5d8..7315ba4e5 100644
--- a/src/wasm-validator.h
+++ b/src/wasm-validator.h
@@ -71,8 +71,10 @@ public:
     } else {
       if (breakTypes[name] == unreachable) {
         breakTypes[name] = valueType;
-      } else {
-        shouldBeEqual(valueType, breakTypes[name], name.str, "breaks to same target must have same type (ignoring unreachable)");
+      } else if (valueType != unreachable) {
+        if (valueType != breakTypes[name]) {
+          breakTypes[name] = none; // a poison value that must not be consumed
+        }
       }
     }
   }
@@ -86,7 +88,9 @@ public:
     noteBreak(curr->default_, curr->value);
   }
   void visitSetLocal(SetLocal *curr) {
-    shouldBeEqual(curr->type, curr->value->type, curr, "set_local type must be correct");
+    if (curr->value->type != unreachable) {
+      shouldBeEqual(curr->type, curr->value->type, curr, "set_local type must be correct");
+    }
   }
   void visitLoad(Load *curr) {
     validateAlignment(curr->align);
@@ -111,9 +115,9 @@ public:
       case Trunc:
       case Nearest:
       case Sqrt: {
-        //if (curr->value->type != unreachable) {
+        if (curr->value->type != unreachable) {
           shouldBeEqual(curr->value->type, curr->type, curr, "non-conversion unaries must return the same type");
-        //}
+        }
         break;
       }
       case EqZ: {
diff --git a/src/wasm.cpp b/src/wasm.cpp
index 2157992e3..98268310a 100644
--- a/src/wasm.cpp
+++ b/src/wasm.cpp
@@ -33,7 +33,11 @@ struct BlockTypeSeeker : public PostWalker<BlockTypeSeeker, Visitor<BlockTypeSee
       if (other == none) {
         type = none;
       } else if (other != unreachable) {
-        type = other;
+        if (type == unreachable) {
+          type = other;
+        } else if (type != other) {
+          type = none; // poison value, we saw multiple types; this should not be consumed
+        }
       }
     }
   }