Remove old syntax from table and elem in parser (#3753)

We've been keeping old syntax in the text format parser although they've
been removed from the parser and hardly any test case relies on them.
This PR will remove old syntax support for tables and element segments
and simplify the corresponding parser functions. A few test files were
affected by this that are updated.

8 files changed, 62 insertions, 97 deletions

diff --git a/src/wasm/wasm-s-parser.cpp b/src/wasm/wasm-s-parser.cpp
index d6cef72cd..c1d25f10d 100644
--- a/src/wasm/wasm-s-parser.cpp
+++ b/src/wasm/wasm-s-parser.cpp
@@ -3178,9 +3178,6 @@ void SExpressionWasmBuilder::parseGlobal(Element& s, bool preParseImport) {
 void SExpressionWasmBuilder::parseTable(Element& s, bool preParseImport) {
   std::unique_ptr<Table> table = make_unique<Table>();
   Index i = 1;
-  if (i == s.size()) {
-    return; // empty table in old notation
-  }
   if (s[i]->dollared()) {
     table->setExplicitName(s[i++]->str());
   } else {
@@ -3188,10 +3185,6 @@ void SExpressionWasmBuilder::parseTable(Element& s, bool preParseImport) {
   }
   tableNames.push_back(table->name);
 
-  if (i == s.size()) {
-    wasm.addTable(std::move(table));
-    return;
-  }
   Name importModule, importBase;
   if (s[i]->isList()) {
     auto& inner = *s[i];
@@ -3216,13 +3209,31 @@ void SExpressionWasmBuilder::parseTable(Element& s, bool preParseImport) {
       throw ParseException("invalid table", inner.line, inner.col);
     }
   }
-  if (i == s.size()) {
-    wasm.addTable(std::move(table));
-    return;
+
+  bool hasExplicitLimit = false;
+
+  if (s[i]->isStr() && String::isNumber(s[i]->c_str())) {
+    table->initial = atoi(s[i++]->c_str());
+    hasExplicitLimit = true;
+  }
+
+  if (s[i]->isStr() && String::isNumber(s[i]->c_str())) {
+    table->max = atoi(s[i++]->c_str());
+  }
+
+  if (!s[i]->isStr() || s[i]->str() != FUNCREF) {
+    throw ParseException("Expected funcref");
+  } else {
+    i += 1;
   }
 
-  auto parseTableElem = [&](Table* table, Element& s) {
-    parseElem(s, table);
+  if (i < s.size() && s[i]->isList()) {
+    if (hasExplicitLimit) {
+      throw ParseException(
+        "Table cannot have both explicit limits and an inline (elem ...)");
+    }
+    // (table type (elem ..))
+    parseElem(*s[i], table.get());
     auto it = std::find_if(wasm.elementSegments.begin(),
                            wasm.elementSegments.end(),
                            [&](std::unique_ptr<ElementSegment>& segment) {
@@ -3233,32 +3244,8 @@ void SExpressionWasmBuilder::parseTable(Element& s, bool preParseImport) {
     } else {
       table->initial = table->max = 0;
     }
-  };
-
-  if (!s[i]->dollared()) {
-    if (s[i]->str() == FUNCREF) {
-      // (table type (elem ..))
-      parseTableElem(table.get(), *s[i + 1]);
-      wasm.addTable(std::move(table));
-      return;
-    }
-    // first element isn't dollared, and isn't funcref. this could be old syntax
-    // for (table 0 1) which means function 0 and 1, or it could be (table
-    // initial max? type), look for type
-    if (s[s.size() - 1]->str() == FUNCREF) {
-      // (table initial max? type)
-      if (i < s.size() - 1) {
-        table->initial = atoi(s[i++]->c_str());
-      }
-      if (i < s.size() - 1) {
-        table->max = atoi(s[i++]->c_str());
-      }
-      wasm.addTable(std::move(table));
-      return;
-    }
   }
-  // old notation (table func1 func2 ..)
-  parseTableElem(table.get(), s);
+
   wasm.addTable(std::move(table));
 }
 
@@ -3278,14 +3265,14 @@ void SExpressionWasmBuilder::parseElem(Element& s, Table* table) {
   Index i = 1;
   Name name = Name::fromInt(elemCounter++);
   bool hasExplicitName = false;
-  bool isPassive = false;
+  bool isPassive = true;
   bool usesExpressions = false;
 
   if (table) {
     Expression* offset = allocator.alloc<Const>()->set(Literal(int32_t(0)));
     auto segment = std::make_unique<ElementSegment>(table->name, offset);
     segment->setName(name, hasExplicitName);
-    parseElemFinish(s, segment, i, false);
+    parseElemFinish(s, segment, i, s[i]->isList());
     return;
   }
 
@@ -3298,70 +3285,48 @@ void SExpressionWasmBuilder::parseElem(Element& s, Table* table) {
     return;
   }
 
-  if (s[i]->isStr()) {
-    if (s[i]->str() == FUNC) {
-      isPassive = true;
-      usesExpressions = false;
-    } else if (s[i]->str() == FUNCREF) {
-      isPassive = true;
-      usesExpressions = true;
-    }
-  }
-
-  if (isPassive) {
-    auto segment = std::make_unique<ElementSegment>();
-    segment->setName(name, hasExplicitName);
-    parseElemFinish(s, segment, i + 1, usesExpressions);
-    return;
-  }
-
-  // old style refers to the pre-reftypes form of (elem 0? (expr) vec(funcidx))
-  bool oldStyle = true;
-
-  // At this point, we know that we're parsing an active element segment. A
-  // table will be mandatory now.
-  if (wasm.tables.empty()) {
-    throw ParseException("elem without table", s.line, s.col);
-  }
-
-  // Old style table index (elem 0 (i32.const 0) ...)
-  if (s[i]->isStr()) {
-    i += 1;
-  }
-
-  if (s[i]->isList() && elementStartsWith(s[i], TABLE)) {
-    oldStyle = false;
-    auto& inner = *s[i++];
-    Name tableName = getTableName(*inner[1]);
-    table = wasm.getTable(tableName);
-  }
+  auto segment = std::make_unique<ElementSegment>();
+  segment->setName(name, hasExplicitName);
 
-  Expression* offset = nullptr;
   if (s[i]->isList()) {
+    // Optional (table <tableidx>)
+    if (elementStartsWith(s[i], TABLE)) {
+      auto& inner = *s[i++];
+      segment->table = getTableName(*inner[1]);
+    }
+
+    // Offset expression (offset (<expr>)) | (<expr>)
     auto& inner = *s[i++];
     if (elementStartsWith(inner, OFFSET)) {
-      offset = parseExpression(inner[1]);
-      oldStyle = false;
+      segment->offset = parseExpression(inner[1]);
     } else {
-      offset = parseExpression(inner);
+      segment->offset = parseExpression(inner);
     }
+    isPassive = false;
   }
 
-  if (!oldStyle) {
-    if (s[i]->str() == FUNCREF) {
+  if (i < s.size()) {
+    if (s[i]->isStr() && s[i]->dollared()) {
+      usesExpressions = false;
+    } else if (s[i]->isStr() && s[i]->str() == FUNC) {
+      usesExpressions = false;
+      i += 1;
+    } else if (s[i]->isStr() && s[i]->str() == FUNCREF) {
       usesExpressions = true;
-    } else if (s[i]->str() != FUNC) {
+      i += 1;
+    } else {
       throw ParseException("expected func or funcref.");
     }
-    i += 1;
   }
 
-  if (!table) {
+  if (!isPassive && segment->table.isNull()) {
+    if (wasm.tables.empty()) {
+      throw ParseException("active element without table", s.line, s.col);
+    }
     table = wasm.tables.front().get();
+    segment->table = table->name;
   }
 
-  auto segment = std::make_unique<ElementSegment>(table->name, offset);
-  segment->setName(name, hasExplicitName);
   parseElemFinish(s, segment, i, usesExpressions);
 }
 
diff --git a/test/passes/asyncify.wast b/test/passes/asyncify.wast
index 281934aea..e919a7e86 100644
--- a/test/passes/asyncify.wast
+++ b/test/passes/asyncify.wast
@@ -3,7 +3,7 @@
   (memory 1 2)
   (import "env" "import" (func $import))
   (import "env" "import2" (func $import2 (param i32)))
-  (table 1 1)
+  (table funcref (elem $liveness2 $liveness2))
   (func $liveness1 (param $live0 i32) (param $dead0 i32)
     (local $live1 i32)
     (local $dead1 i32)
diff --git a/test/passes/asyncify_pass-arg=asyncify-asserts_pass-arg=asyncify-onlylist@waka.wast b/test/passes/asyncify_pass-arg=asyncify-asserts_pass-arg=asyncify-onlylist@waka.wast
index 7d67c89ed..4233cb304 100644
--- a/test/passes/asyncify_pass-arg=asyncify-asserts_pass-arg=asyncify-onlylist@waka.wast
+++ b/test/passes/asyncify_pass-arg=asyncify-asserts_pass-arg=asyncify-onlylist@waka.wast
@@ -4,7 +4,7 @@
   (import "env" "import" (func $import))
   (import "env" "import2" (func $import2 (result i32)))
   (import "env" "import3" (func $import3 (param i32)))
-  (table 1 1)
+  (table funcref (elem $calls-import2-drop $calls-import2-drop))
   (func $calls-import
     (call $import)
   )
diff --git a/test/passes/asyncify_pass-arg=asyncify-ignore-imports.wast b/test/passes/asyncify_pass-arg=asyncify-ignore-imports.wast
index 33500baba..80f0851d0 100644
--- a/test/passes/asyncify_pass-arg=asyncify-ignore-imports.wast
+++ b/test/passes/asyncify_pass-arg=asyncify-ignore-imports.wast
@@ -4,7 +4,7 @@
   (import "env" "import" (func $import))
   (import "env" "import2" (func $import2 (result i32)))
   (import "env" "import3" (func $import3 (param i32)))
-  (table 1 1)
+  (table funcref (elem $calls-import2-drop $calls-import2-drop))
   (func $calls-import
     (call $import)
   )
diff --git a/test/passes/asyncify_pass-arg=asyncify-ignore-indirect.wast b/test/passes/asyncify_pass-arg=asyncify-ignore-indirect.wast
index 33500baba..80f0851d0 100644
--- a/test/passes/asyncify_pass-arg=asyncify-ignore-indirect.wast
+++ b/test/passes/asyncify_pass-arg=asyncify-ignore-indirect.wast
@@ -4,7 +4,7 @@
   (import "env" "import" (func $import))
   (import "env" "import2" (func $import2 (result i32)))
   (import "env" "import3" (func $import3 (param i32)))
-  (table 1 1)
+  (table funcref (elem $calls-import2-drop $calls-import2-drop))
   (func $calls-import
     (call $import)
   )
diff --git a/test/spec/elem.wast b/test/spec/elem.wast
index 1ea2b0618..fff4a83ef 100644
--- a/test/spec/elem.wast
+++ b/test/spec/elem.wast
@@ -8,10 +8,10 @@
   (elem (i32.const 0) $f $f)
   (elem (offset (i32.const 0)))
   (elem (offset (i32.const 0)) $f $f)
-  (elem 0 (i32.const 0))
+  (elem (i32.const 0))
   (elem 0x0 (i32.const 0) $f $f)
   (elem 0x000 (offset (i32.const 0)))
-  (elem 0 (offset (i32.const 0)) $f $f)
+  (elem (offset (i32.const 0)) $f $f)
   (elem $t (i32.const 0))
   (elem $t (i32.const 0) $f $f)
   (elem $t (offset (i32.const 0)))
diff --git a/test/spec/imports.wast b/test/spec/imports.wast
index 04c1746c7..c39ae8dd3 100644
--- a/test/spec/imports.wast
+++ b/test/spec/imports.wast
@@ -271,7 +271,7 @@
 (module
   (type (func (result i32)))
   (import "spectest" "table" (table 10 20 funcref))
-  (elem 0 (i32.const 1) $f $g)
+  (elem (i32.const 1) $f $g)
 
   (func (export "call") (param i32) (result i32)
     (call_indirect (type 0) (local.get 0))
@@ -290,7 +290,7 @@
 (module
   (type (func (result i32)))
   (table (import "spectest" "table") 10 20 funcref)
-  (elem 0 (i32.const 1) $f $g)
+  (elem (i32.const 1) $f $g)
 
   (func (export "call") (param i32) (result i32)
     (call_indirect (type 0) (local.get 0))
diff --git a/test/spec/old_import.wast b/test/spec/old_import.wast
index bc6144185..a26c16e48 100644
--- a/test/spec/old_import.wast
+++ b/test/spec/old_import.wast
@@ -96,7 +96,7 @@
 (module
   (type (func (result i32)))
   (import "spectest" "table" (table 10 20 funcref))
-  (elem 0 (i32.const 1) $f $g)
+  (elem (i32.const 1) $f $g)
 
   (func (export "call") (param i32) (result i32) (call_indirect (type 0) (local.get 0)))
   (func $f (result i32) (i32.const 11))
@@ -113,7 +113,7 @@
 (module
   (type (func (result i32)))
   (table (import "spectest" "table") 10 20 funcref)
-  (elem 0 (i32.const 1) $f $g)
+  (elem (i32.const 1) $f $g)
 
   (func (export "call") (param i32) (result i32) (call_indirect (type 0) (local.get 0)))
   (func $f (result i32) (i32.const 11))