Support new result syntax for if/loop/block (#1047)

Support both syntax formats in input since the old spec
tests still need to be parsable.

7 files changed, 37 insertions, 25 deletions

diff --git a/src/ast_utils.h b/src/ast_utils.h
index 823c08f9c..83818daa8 100644
--- a/src/ast_utils.h
+++ b/src/ast_utils.h
@@ -325,7 +325,7 @@ struct ExpressionAnalyzer {
 // Re-Finalizes all node types
 // This removes "unnecessary' block/if/loop types, i.e., that are added
 // specifically, as in
-//  (block i32 (unreachable))
+//  (block (result i32) (unreachable))
 // vs
 //  (block (unreachable))
 // This converts to the latter form.
diff --git a/src/passes/DeadCodeElimination.cpp b/src/passes/DeadCodeElimination.cpp
index af06439c0..209180e23 100644
--- a/src/passes/DeadCodeElimination.cpp
+++ b/src/passes/DeadCodeElimination.cpp
@@ -68,7 +68,7 @@ struct DeadCodeElimination : public WalkerPass<PostWalker<DeadCodeElimination>>
     // we normally have already reduced unreachable code into (unreachable)
     // nodes, so we would not get to this function at all anyhow, the breaking
     // instruction itself would be removed. However, an exception are things
-    // like  (block i32 (call $x) (unreachable)) , which has type i32
+    // like  (block (result i32) (call $x) (unreachable)) , which has type i32
     // despite not being exited.
     // TODO: optimize such cases
     if (reachable) {
diff --git a/src/passes/MergeBlocks.cpp b/src/passes/MergeBlocks.cpp
index 48101e295..1ec8843d6 100644
--- a/src/passes/MergeBlocks.cpp
+++ b/src/passes/MergeBlocks.cpp
@@ -225,7 +225,7 @@ struct MergeBlocks : public WalkerPass<PostWalker<MergeBlocks>> {
       if (!block->name.is() && block->list.size() >= 2) {
         child = block->list.back();
         // we modified child (which is a reference to a pointer), which modifies curr, which might change its type
-        // (e.g. (drop (block i32 .. (unreachable)))
+        // (e.g. (drop (block (result i32) .. (unreachable)))
         // the child was a block of i32, and is being replaced with an unreachable, so the
         // parent will likely need to be unreachable too
         auto oldType = curr->type;
diff --git a/src/passes/Print.cpp b/src/passes/Print.cpp
index 95f60e5e2..340266fcf 100644
--- a/src/passes/Print.cpp
+++ b/src/passes/Print.cpp
@@ -134,7 +134,7 @@ struct PrintSExpression : public Visitor<PrintSExpression> {
         printName(curr->name);
       }
       if (isConcreteWasmType(curr->type)) {
-        o << ' ' << printWasmType(curr->type);
+        o << " (result " << printWasmType(curr->type) << ')';
       }
       incIndent();
       if (curr->list.size() > 0 && curr->list[0]->is<Block>()) {
@@ -165,7 +165,7 @@ struct PrintSExpression : public Visitor<PrintSExpression> {
   void visitIf(If *curr) {
     printOpening(o, "if");
     if (isConcreteWasmType(curr->type)) {
-      o << ' ' << printWasmType(curr->type);
+      o << " (result " << printWasmType(curr->type) << ')';
     }
     incIndent();
     printFullLine(curr->condition);
@@ -190,7 +190,7 @@ struct PrintSExpression : public Visitor<PrintSExpression> {
       o << ' ' << curr->name;
     }
     if (isConcreteWasmType(curr->type)) {
-      o << ' ' << printWasmType(curr->type);
+      o << " (result " << printWasmType(curr->type) << ')';
     }
     incIndent();
     auto block = curr->body->dynCast<Block>();
diff --git a/src/wasm-s-parser.h b/src/wasm-s-parser.h
index 53594e7aa..8b887d690 100644
--- a/src/wasm-s-parser.h
+++ b/src/wasm-s-parser.h
@@ -195,6 +195,8 @@ private:
   Expression* makeBreakTable(Element& s);
   Expression* makeReturn(Element& s);
 
+  WasmType parseOptionalResultType(Element& s, Index& i);
+
   void stringToBinary(const char* input, size_t size, std::vector<char>& data);
   void parseMemory(Element& s, bool preParseImport = false);
   void parseData(Element& s);
diff --git a/src/wasm-validator.h b/src/wasm-validator.h
index 04c42e4f2..9420179cf 100644
--- a/src/wasm-validator.h
+++ b/src/wasm-validator.h
@@ -768,7 +768,7 @@ public:
           // We accept concrete => undefined,
           // e.g.
           //
-          //  (drop (block i32 (unreachable)))
+          //  (drop (block (result i32) (unreachable)))
           //
           // The block has an added type, not derived from the ast itself, so it is
           // ok for it to be either i32 or unreachable.
diff --git a/src/wasm/wasm-s-parser.cpp b/src/wasm/wasm-s-parser.cpp
index 6536d78fb..59043f3b8 100644
--- a/src/wasm/wasm-s-parser.cpp
+++ b/src/wasm/wasm-s-parser.cpp
@@ -1048,7 +1048,7 @@ Expression* SExpressionWasmBuilder::makeBlock(Element& s) {
   while (1) {
     stack.emplace_back(sp, curr);
     auto& s = *sp;
-    size_t i = 1;
+    Index i = 1;
     Name sName;
     if (i < s.size() && s[i]->isStr()) {
       // could be a name or a type
@@ -1061,14 +1061,9 @@ Expression* SExpressionWasmBuilder::makeBlock(Element& s) {
       sName = "block";
     }
     curr->name = nameMapper.pushLabelName(sName);
+    // block signature
+    curr->type = parseOptionalResultType(s, i);
     if (i >= s.size()) break; // empty block
-    if (s[i]->isStr()) {
-      // block signature
-      curr->type = stringToWasmType(s[i++]->str());
-      if (i >= s.size()) break; // empty block
-    } else {
-      curr->type = none;
-    }
     auto& first = *s[i];
     if (first[0]->str() == BLOCK) {
       // recurse
@@ -1088,6 +1083,9 @@ Expression* SExpressionWasmBuilder::makeBlock(Element& s) {
       while (i < s.size() && s[i]->isStr()) {
         i++;
       }
+      if (i < s.size() && (*s[i])[0]->str() == RESULT) {
+        i++;
+      }
       if (t < int(stack.size()) - 1) {
         // first child is one of our recursions
         curr->list.push_back(stack[t + 1].second);
@@ -1217,10 +1215,8 @@ Expression* SExpressionWasmBuilder::makeIf(Element& s) {
     sName = "if";
   }
   auto label = nameMapper.pushLabelName(sName);
-  WasmType type = none;
-  if (s[i]->isStr()) {
-    type = stringToWasmType(s[i++]->str());
-  }
+  // if signature
+  WasmType type = parseOptionalResultType(s, i);
   ret->condition = parseExpression(s[i++]);
   ret->ifTrue = parseExpression(*s[i++]);
   if (i < s.size()) {
@@ -1255,9 +1251,27 @@ Expression* SExpressionWasmBuilder::makeMaybeBlock(Element& s, size_t i, WasmTyp
   return ret;
 }
 
+WasmType SExpressionWasmBuilder::parseOptionalResultType(Element& s, Index& i) {
+  if (s.size() == i)
+    return none;
+
+  // TODO(sbc): Remove support for old result syntax (bare streing) once the
+  // spec tests are updated.
+  if (s[i]->isStr())
+    return stringToWasmType(s[i++]->str());
+
+  Element& params = *s[i];
+  IString id = params[0]->str();
+  if (id != RESULT)
+    return none;
+
+  i++;
+  return stringToWasmType(params[1]->str());
+}
+
 Expression* SExpressionWasmBuilder::makeLoop(Element& s) {
   auto ret = allocator.alloc<Loop>();
-  size_t i = 1;
+  Index i = 1;
   Name sName;
   if (s.size() > i && s[i]->dollared()) {
     sName = s[i++]->str();
@@ -1265,11 +1279,7 @@ Expression* SExpressionWasmBuilder::makeLoop(Element& s) {
     sName = "loop-in";
   }
   ret->name = nameMapper.pushLabelName(sName);
-  ret->type = none;
-  if (i < s.size() && s[i]->isStr()) {
-    // block signature
-    ret->type = stringToWasmType(s[i++]->str());
-  }
+  ret->type = parseOptionalResultType(s, i);
   ret->body = makeMaybeBlock(s, i, ret->type);
   nameMapper.popLabelName(ret->name);
   ret->finalize(ret->type);