Require `then` and `else` with `if` (#6201)

We previously supported (and primarily used) a non-standard text format for conditionals in which the condition, if-true expression, and if-false expression were all simply s-expression children of the `if` expression. The standard text format, however, requires the use of `then` and `else` forms to introduce the if-true and if-false arms of the conditional. Update the legacy text parser to require the standard format and update all tests to match. Update the printer to print the standard format as well. The .wast and .wat test inputs were mechanically updated with this script: https://gist.github.com/tlively/85ae7f01f92f772241ec994c840ccbb1
author: Thomas Lively <tlively@google.com> 2024-01-04 14:25:18 -0800
committer: GitHub <noreply@github.com> 2024-01-04 14:25:18 -0800
commit: a58281ca114359cd6e65f5daaf086636aa18b0b0 (patch)
tree: ff98bd31d1c87b598027c2303b17855a44346515 /src
parent: 0ed42cf976ce9a3dfbe9cbb0885122e8fb6a377b (diff)
download: binaryen-a58281ca114359cd6e65f5daaf086636aa18b0b0.tar.gz
binaryen-a58281ca114359cd6e65f5daaf086636aa18b0b0.tar.bz2
binaryen-a58281ca114359cd6e65f5daaf086636aa18b0b0.zip
3 files changed, 189 insertions, 160 deletions
diff --git a/src/passes/Print.cpp b/src/passes/Print.cpp
index dcd43c323..b6060a853 100644
--- a/src/passes/Print.cpp
+++ b/src/passes/Print.cpp
@@ -306,7 +306,7 @@ struct PrintSExpression : public UnifiedExpressionVisitor<PrintSExpression> {
 
   // loop, if, and try can contain implicit blocks. But they are not needed to
   // be printed in some cases.
-  void maybePrintImplicitBlock(Expression* curr, bool allowMultipleInsts);
+  void maybePrintImplicitBlock(Expression* curr);
 
   // Generic visitor, overridden only when necessary.
   void visitExpression(Expression* curr);
@@ -2563,11 +2563,9 @@ void PrintSExpression::printFullLine(Expression* expression) {
   o << maybeNewLine;
 }
 
-void PrintSExpression::maybePrintImplicitBlock(Expression* curr,
-                                               bool allowMultipleInsts) {
+void PrintSExpression::maybePrintImplicitBlock(Expression* curr) {
   auto block = curr->dynCast<Block>();
-  if (!full && block && block->name.isNull() &&
-      (allowMultipleInsts || block->list.size() == 1)) {
+  if (!full && block && block->name.isNull()) {
     for (auto expression : block->list) {
       printFullLine(expression);
     }
@@ -2657,13 +2655,23 @@ void PrintSExpression::visitIf(If* curr) {
   printExpressionContents(curr);
   incIndent();
   printFullLine(curr->condition);
-  maybePrintImplicitBlock(curr->ifTrue, false);
+  doIndent(o, indent);
+  o << "(then";
+  incIndent();
+  maybePrintImplicitBlock(curr->ifTrue);
+  decIndent();
+  o << maybeNewLine;
   if (curr->ifFalse) {
+    doIndent(o, indent);
+    o << "(else";
+    incIndent();
     // Note: debug info here is not used as LLVM does not emit ifs, and since
     // LLVM is the main source of DWARF, effectively we never encounter ifs
     // with DWARF.
     printDebugDelimiterLocation(curr, BinaryLocations::Else);
-    maybePrintImplicitBlock(curr->ifFalse, false);
+    maybePrintImplicitBlock(curr->ifFalse);
+    decIndent();
+    o << maybeNewLine;
   }
   decIndent();
   if (full) {
@@ -2677,7 +2685,7 @@ void PrintSExpression::visitLoop(Loop* curr) {
   o << '(';
   printExpressionContents(curr);
   incIndent();
-  maybePrintImplicitBlock(curr->body, true);
+  maybePrintImplicitBlock(curr->body);
   decIndent();
   if (full) {
     o << " ;; end loop";
@@ -2722,7 +2730,7 @@ void PrintSExpression::visitTry(Try* curr) {
   o << '(';
   printMedium(o, "do");
   incIndent();
-  maybePrintImplicitBlock(curr->body, true);
+  maybePrintImplicitBlock(curr->body);
   decIndent();
   o << "\n";
   for (size_t i = 0; i < curr->catchTags.size(); i++) {
@@ -2732,7 +2740,7 @@ void PrintSExpression::visitTry(Try* curr) {
     printMedium(o, "catch ");
     printName(curr->catchTags[i], o);
     incIndent();
-    maybePrintImplicitBlock(curr->catchBodies[i], true);
+    maybePrintImplicitBlock(curr->catchBodies[i]);
     decIndent();
     o << "\n";
   }
@@ -2742,7 +2750,7 @@ void PrintSExpression::visitTry(Try* curr) {
     o << '(';
     printMedium(o, "catch_all");
     incIndent();
-    maybePrintImplicitBlock(curr->catchBodies.back(), true);
+    maybePrintImplicitBlock(curr->catchBodies.back());
     decIndent();
     o << "\n";
   }
@@ -2770,7 +2778,7 @@ void PrintSExpression::visitTryTable(TryTable* curr) {
   o << '(';
   printExpressionContents(curr);
   incIndent();
-  maybePrintImplicitBlock(curr->body, true);
+  maybePrintImplicitBlock(curr->body);
   decIndent();
   if (full) {
     o << " ;; end if";
diff --git a/src/passes/wasm-intrinsics.wat b/src/passes/wasm-intrinsics.wat
index 185bef961..efa312645 100644
--- a/src/passes/wasm-intrinsics.wat
+++ b/src/passes/wasm-intrinsics.wat
@@ -146,16 +146,18 @@
  (func $__wasm_ctz_i32 (; 7 ;) (type $3) (param $var$0 i32) (result i32)
   (if
    (local.get $var$0)
-   (return
-    (i32.sub
-     (i32.const 31)
-     (i32.clz
-      (i32.xor
-       (i32.add
+   (then
+    (return
+     (i32.sub
+      (i32.const 31)
+      (i32.clz
+       (i32.xor
+        (i32.add
+         (local.get $var$0)
+         (i32.const -1)
+        )
         (local.get $var$0)
-        (i32.const -1)
        )
-       (local.get $var$0)
       )
      )
     )
@@ -171,16 +173,18 @@
      (local.get $var$0)
     )
    )
-   (return
-    (i64.sub
-     (i64.const 63)
-     (i64.clz
-      (i64.xor
-       (i64.add
+   (then
+    (return
+     (i64.sub
+      (i64.const 63)
+      (i64.clz
+       (i64.xor
+        (i64.add
+         (local.get $var$0)
+         (i64.const -1)
+        )
         (local.get $var$0)
-        (i64.const -1)
        )
-       (local.get $var$0)
       )
      )
     )
@@ -369,38 +373,42 @@
      (f32.const 0.5)
     )
    )
-   (block
-    (local.set $var$0
-     (f32.ceil
-      (local.get $var$0)
-     )
-    )
-    (if
-     (f32.gt
-      (local.get $var$2)
-      (f32.const 0.5)
+   (then
+    (block
+     (local.set $var$0
+      (f32.ceil
+       (local.get $var$0)
+      )
      )
-     (return
-      (local.get $var$0)
+     (if
+      (f32.gt
+       (local.get $var$2)
+       (f32.const 0.5)
+      )
+      (then
+       (return
+        (local.get $var$0)
+       )
+      )
      )
-    )
-    (local.set $var$1
-     (select
-      (local.get $var$1)
-      (local.get $var$0)
-      (f32.eq
-       (f32.sub
-        (local.tee $var$2
-         (f32.mul
-          (local.get $var$1)
-          (f32.const 0.5)
+     (local.set $var$1
+      (select
+       (local.get $var$1)
+       (local.get $var$0)
+       (f32.eq
+        (f32.sub
+         (local.tee $var$2
+          (f32.mul
+           (local.get $var$1)
+           (f32.const 0.5)
+          )
+         )
+         (f32.floor
+          (local.get $var$2)
          )
         )
-        (f32.floor
-         (local.get $var$2)
-        )
+        (f32.const 0)
        )
-       (f32.const 0)
       )
      )
     )
@@ -429,38 +437,42 @@
      (f64.const 0.5)
     )
    )
-   (block
-    (local.set $var$0
-     (f64.ceil
-      (local.get $var$0)
-     )
-    )
-    (if
-     (f64.gt
-      (local.get $var$2)
-      (f64.const 0.5)
+   (then
+    (block
+     (local.set $var$0
+      (f64.ceil
+       (local.get $var$0)
+      )
      )
-     (return
-      (local.get $var$0)
+     (if
+      (f64.gt
+       (local.get $var$2)
+       (f64.const 0.5)
+      )
+      (then
+       (return
+        (local.get $var$0)
+       )
+      )
      )
-    )
-    (local.set $var$1
-     (select
-      (local.get $var$1)
-      (local.get $var$0)
-      (f64.eq
-       (f64.sub
-        (local.tee $var$2
-         (f64.mul
-          (local.get $var$1)
-          (f64.const 0.5)
+     (local.set $var$1
+      (select
+       (local.get $var$1)
+       (local.get $var$0)
+       (f64.eq
+        (f64.sub
+         (local.tee $var$2
+          (f64.mul
+           (local.get $var$1)
+           (f64.const 0.5)
+          )
+         )
+         (f64.floor
+          (local.get $var$2)
          )
         )
-        (f64.floor
-         (local.get $var$2)
-        )
+        (f64.const 0)
        )
-       (f64.const 0)
       )
      )
     )
@@ -496,44 +508,46 @@
                 )
                )
               )
-              (block
-               (br_if $label$11
-                (i32.eqz
-                 (local.tee $var$3
-                  (i32.wrap_i64
-                   (local.get $var$1)
+              (then
+               (block
+                (br_if $label$11
+                 (i32.eqz
+                  (local.tee $var$3
+                   (i32.wrap_i64
+                    (local.get $var$1)
+                   )
                   )
                  )
                 )
-               )
-               (br_if $label$9
-                (i32.eqz
-                 (local.tee $var$4
-                  (i32.wrap_i64
-                   (i64.shr_u
-                    (local.get $var$1)
-                    (i64.const 32)
+                (br_if $label$9
+                 (i32.eqz
+                  (local.tee $var$4
+                   (i32.wrap_i64
+                    (i64.shr_u
+                     (local.get $var$1)
+                     (i64.const 32)
+                    )
                    )
                   )
                  )
                 )
-               )
-               (br_if $label$8
-                (i32.le_u
-                 (local.tee $var$2
-                  (i32.sub
-                   (i32.clz
-                    (local.get $var$4)
-                   )
-                   (i32.clz
-                    (local.get $var$2)
+                (br_if $label$8
+                 (i32.le_u
+                  (local.tee $var$2
+                   (i32.sub
+                    (i32.clz
+                     (local.get $var$4)
+                    )
+                    (i32.clz
+                     (local.get $var$2)
+                    )
                    )
                   )
+                  (i32.const 31)
                  )
-                 (i32.const 31)
                 )
+                (br $label$2)
                )
-               (br $label$2)
               )
              )
              (br_if $label$2
@@ -790,69 +804,71 @@
     (block $label$13
      (if
       (local.get $var$2)
-      (block
-       (local.set $var$8
-        (i64.add
-         (local.get $var$1)
-         (i64.const -1)
+      (then
+       (block
+        (local.set $var$8
+         (i64.add
+          (local.get $var$1)
+          (i64.const -1)
+         )
         )
-       )
-       (loop $label$15
-        (local.set $var$5
-         (i64.sub
-          (local.tee $var$5
-           (i64.or
-            (i64.shl
-             (local.get $var$5)
-             (i64.const 1)
-            )
-            (i64.shr_u
-             (local.get $var$0)
-             (i64.const 63)
+        (loop $label$15
+         (local.set $var$5
+          (i64.sub
+           (local.tee $var$5
+            (i64.or
+             (i64.shl
+              (local.get $var$5)
+              (i64.const 1)
+             )
+             (i64.shr_u
+              (local.get $var$0)
+              (i64.const 63)
+             )
             )
            )
-          )
-          (i64.and
-           (local.tee $var$6
-            (i64.shr_s
-             (i64.sub
-              (local.get $var$8)
-              (local.get $var$5)
+           (i64.and
+            (local.tee $var$6
+             (i64.shr_s
+              (i64.sub
+               (local.get $var$8)
+               (local.get $var$5)
+              )
+              (i64.const 63)
              )
-             (i64.const 63)
             )
+            (local.get $var$1)
            )
-           (local.get $var$1)
           )
          )
-        )
-        (local.set $var$0
-         (i64.or
-          (i64.shl
-           (local.get $var$0)
-           (i64.const 1)
+         (local.set $var$0
+          (i64.or
+           (i64.shl
+            (local.get $var$0)
+            (i64.const 1)
+           )
+           (local.get $var$7)
           )
-          (local.get $var$7)
          )
-        )
-        (local.set $var$7
-         (local.tee $var$6
-          (i64.and
-           (local.get $var$6)
-           (i64.const 1)
+         (local.set $var$7
+          (local.tee $var$6
+           (i64.and
+            (local.get $var$6)
+            (i64.const 1)
+           )
           )
          )
-        )
-        (br_if $label$15
-         (local.tee $var$2
-          (i32.add
-           (local.get $var$2)
-           (i32.const -1)
+         (br_if $label$15
+          (local.tee $var$2
+           (i32.add
+            (local.get $var$2)
+            (i32.const -1)
+           )
           )
          )
         )
+        (br $label$13)
        )
-       (br $label$13)
       )
      )
     )
diff --git a/src/wasm/wasm-s-parser.cpp b/src/wasm/wasm-s-parser.cpp
index 41a5b1884..9270a19b0 100644
--- a/src/wasm/wasm-s-parser.cpp
+++ b/src/wasm/wasm-s-parser.cpp
@@ -1703,12 +1703,11 @@ Expression* SExpressionWasmBuilder::makeBlock(Element& s) {
 // Similar to block, but the label is handled by the enclosing if (since there
 // might not be a then or else, ick)
 Expression* SExpressionWasmBuilder::makeThenOrElse(Element& s) {
-  auto ret = allocator.alloc<Block>();
-  size_t i = 1;
-  if (s.size() > 1 && s[1]->isStr()) {
-    i++;
+  if (s.size() == 2) {
+    return parseExpression(s[1]);
   }
-  for (; i < s.size(); i++) {
+  auto ret = allocator.alloc<Block>();
+  for (size_t i = 1; i < s.size(); i++) {
     ret->list.push_back(parseExpression(s[i]));
   }
   ret->finalize();
@@ -2415,8 +2414,14 @@ Expression* SExpressionWasmBuilder::makeIf(Element& s) {
   // if signature
   Type type = parseBlockType(s, i);
   ret->condition = parseExpression(s[i++]);
+  if (!elementStartsWith(*s[i], "then")) {
+    throw SParseException("expected 'then'", *s[i]);
+  }
   ret->ifTrue = parseExpression(*s[i++]);
   if (i < s.size()) {
+    if (!elementStartsWith(*s[i], "else")) {
+      throw SParseException("expected 'else'", *s[i]);
+    }
     ret->ifFalse = parseExpression(*s[i++]);
   }
   ret->finalize(type);
author	Thomas Lively <tlively@google.com>	2024-01-04 14:25:18 -0800
committer	GitHub <noreply@github.com>	2024-01-04 14:25:18 -0800
commit	a58281ca114359cd6e65f5daaf086636aa18b0b0 (patch)
tree	ff98bd31d1c87b598027c2303b17855a44346515 /src
parent	0ed42cf976ce9a3dfbe9cbb0885122e8fb6a377b (diff)
download	binaryen-a58281ca114359cd6e65f5daaf086636aa18b0b0.tar.gz binaryen-a58281ca114359cd6e65f5daaf086636aa18b0b0.tar.bz2 binaryen-a58281ca114359cd6e65f5daaf086636aa18b0b0.zip