Refactor type and function parsing (#2143)

- Refactored & fixed typeuse parsing rules so now the rules more closely
  follow the spec. There have been multiple parsing rules that were
  different in subtle ways, which are supposed to be the same according
  to the spec.
  - Duplicate types, i.e., types with the same signature, in the type
    section are allowed as long as they don't have the same given name.
    If a name is given, we use it; if type name is not given, we
    generate one in the form of `$FUNCSIG$` + signature string. If the
    same generated name already exists in the type section, we append
    `_` at the end. This causes most of the changes in the autogenerated
    type names in test outputs.
  - A typeuse has to be in the order of (type) -> (param) -> (result),
    if more than one of them exist. In case of function definitions,
    (local) has to be after all of these. Fixed some test cases that
    violate this rule.
  - When only (param)/(result) are given, its type will be the type with
    the smallest existing type index whose parameter and result are the
    same. If there's no such type, a new type will be created and
    inserted.
  - Added a test case `duplicate_types.wast` to test type namings for
    duplicate types.

- Refactored `parseFunction` function.

- Add more overrides to helper functions: `getSig` and
  `ensureFunctionType`.

Author: aheejin

CHANGELOG.md

@@ -15,6 +15,17 @@ full changeset diff at the end of each section.
 Current Trunk
 -------------
 
+- Wast file parsing rules now don't allow a few invalid formats for typeuses
+  that were previously allowed. Typeuse entries should follow this format,
+  meaning they should have (type) -> (param) -> (result) order if more than one
+  of them exist.
+  ```
+  typeuse ::= (type index|name)+ |
+              (type index|name)+ (param ..)* (result ..)* |
+              (param ..)* (result ..)*
+  ```
+  Also, all (local) nodes in function definition should be after all typeuse
+  elements.
 - Removed APIs related to deprecated instruction names in Binaryen.js:
   - `get_local` / `getLocal`
   - `set_local` / `setLocal`

src/asm_v_wasm.h

@@ -29,8 +29,17 @@ AsmType wasmToAsmType(Type type);
 
 char getSig(Type type);
 
-std::string getSig(const FunctionType* type);
+template<typename ListType>
+std::string getSig(const ListType& params, Type result) {
+  std::string ret;
+  ret += getSig(result);
+  for (auto param : params) {
+    ret += getSig(param);
+  }
+  return ret;
+}
 
+std::string getSig(const FunctionType* type);
 std::string getSig(Function* func);
 
 template<typename T,
@@ -67,9 +76,18 @@ std::string getSigFromStructs(Type result, const ListType& operands) {
 
 Type sigToType(char sig);
 
-FunctionType sigToFunctionType(std::string sig);
+FunctionType sigToFunctionType(const std::string& sig);
+
+FunctionType*
+ensureFunctionType(const std::string& sig, Module* wasm, Name name = Name());
 
-FunctionType* ensureFunctionType(std::string sig, Module* wasm);
+template<typename ListType>
+FunctionType* ensureFunctionType(const ListType& params,
+                                 Type result,
+                                 Module* wasm,
+                                 Name name = Name()) {
+  return ensureFunctionType(getSig(params, result), wasm, name);
+}
 
 // converts an f32 to an f64 if necessary
 Expression* ensureDouble(Expression* expr, MixedArena& allocator);

src/asmjs/asm_v_wasm.cpp

@@ -86,21 +86,11 @@ char getSig(Type type) {
 }
 
 std::string getSig(const FunctionType* type) {
-  std::string ret;
-  ret += getSig(type->result);
-  for (auto param : type->params) {
-    ret += getSig(param);
-  }
-  return ret;
+  return getSig(type->params, type->result);
 }
 
 std::string getSig(Function* func) {
-  std::string ret;
-  ret += getSig(func->result);
-  for (auto type : func->params) {
-    ret += getSig(type);
-  }
-  return ret;
+  return getSig(func->params, func->result);
 }
 
 Type sigToType(char sig) {
@@ -124,7 +114,7 @@ Type sigToType(char sig) {
   }
 }
 
-FunctionType sigToFunctionType(std::string sig) {
+FunctionType sigToFunctionType(const std::string& sig) {
   FunctionType ret;
   ret.result = sigToType(sig[0]);
   for (size_t i = 1; i < sig.size(); i++) {
@@ -133,8 +123,11 @@ FunctionType sigToFunctionType(std::string sig) {
   return ret;
 }
 
-FunctionType* ensureFunctionType(std::string sig, Module* wasm) {
-  cashew::IString name(("FUNCSIG$" + sig).c_str(), false);
+FunctionType*
+ensureFunctionType(const std::string& sig, Module* wasm, Name name) {
+  if (!name.is()) {
+    name = "FUNCSIG$" + sig;
+  }
   if (wasm->getFunctionTypeOrNull(name)) {
     return wasm->getFunctionType(name);
   }

src/wasm-s-parser.h

@@ -112,7 +112,6 @@ class SExpressionWasmBuilder {
   Module& wasm;
   MixedArena& allocator;
   std::vector<Name> functionNames;
-  std::vector<Name> functionTypeNames;
   std::vector<Name> globalNames;
   int functionCounter;
   int globalCounter = 0;
@@ -222,9 +221,20 @@ class SExpressionWasmBuilder {
   Type parseOptionalResultType(Element& s, Index& i);
   Index parseMemoryLimits(Element& s, Index i);
   std::vector<Type> parseParamOrLocal(Element& s);
-  std::vector<NameType> parseNamedParamOrLocal(Element& s, size_t& localIndex);
+  std::vector<NameType> parseParamOrLocal(Element& s, size_t& localIndex);
   Type parseResult(Element& s);
   FunctionType* parseTypeRef(Element& s);
+  size_t parseTypeUse(Element& s,
+                      size_t startPos,
+                      FunctionType*& functionType,
+                      std::vector<NameType>& namedParams,
+                      Type& result);
+  size_t parseTypeUse(Element& s,
+                      size_t startPos,
+                      FunctionType*& functionType,
+                      std::vector<Type>& params,
+                      Type& result);
+  size_t parseTypeUse(Element& s, size_t startPos, FunctionType*& functionType);
 
   void stringToBinary(const char* input, size_t size, std::vector<char>& data);
   void parseMemory(Element& s, bool preParseImport = false);

src/wasm.h

@@ -632,6 +632,7 @@ class FunctionType {
   FunctionType() = default;
 
   bool structuralComparison(FunctionType& b);
+  bool structuralComparison(const std::vector<Type>& params, Type result);
 
   bool operator==(FunctionType& b);
   bool operator!=(FunctionType& b);