Support (module definition ...) and (module instance ...) in WAST spec tests

Author: stevenfontanella

check.py

@@ -221,8 +221,6 @@ def check_expected(actual, expected):
 
         check_expected(actual, expected)
 
-        run_spec_test(wast)
-
         # check binary format. here we can verify execution of the final
         # result, no need for an output verification
         actual = ''

scripts/test/shared.py

@@ -435,12 +435,11 @@ def get_tests(test_dir, extensions=[], recursive=False):
     'if.wast',      # Requires more precise unreachable validation
     'imports.wast',  # Missing validation of missing function on instantiation
     'linking.wast',  # Missing function type validation on instantiation
-    'memory.wast',   # Requires wast `module definition` support
     'memory64-imports.wast',  # Missing validation on instantiation
     'annotations.wast',  # String annotations IDs should be allowed
     'id.wast',       # Empty IDs should be disallowed
-    'instance.wast',  # Requires wast `module definition` support
-    'table64.wast',   # Requires wast `module definition` support
+    'instance.wast',  # Requires ref.null wast constants, and more correct handling for tag imports from different instances of the same module
+    'table64.wast',   # Requires additional validations on table size
     'table_grow.wast',  # Incorrect table linking semantics in interpreter
     'tag.wast',      # Non-empty tag results allowed by stack switching
     'try_table.wast',  # Requires try_table interpretation
@@ -464,7 +463,7 @@ def get_tests(test_dir, extensions=[], recursive=False):
     'type-rec.wast',  # Missing function type validation on instantiation
     'type-subtyping.wast',  # ShellExternalInterface::callTable does not handle subtyping
     'call_indirect.wast',   # Bug with 64-bit inline element segment parsing
-    'memory64.wast',        # Requires wast `module definition` support
+    'memory64.wast',        # Requires additional validations on memory size
     'imports0.wast',        # Missing memory type validation on instantiation
     'imports2.wast',        # Missing memory type validation on instantiation
     'imports3.wast',        # Missing memory type validation on instantiation

scripts/test/support.py

@@ -90,8 +90,13 @@ def untar(tarfile, outdir):
 
 QUOTED = re.compile(r'\(module\s*(\$\S*)?\s+(quote|binary)')
 
+MODULE_DEFINITION_OR_INSTANCE = re.compile(r'(?m)\(module\s+(instance|definition)')
 
 def split_wast(wastFile):
+    '''
+    Returns a list of pairs of module definitions and assertions.
+    Module invalidity tests, as well as (module definition ...) and (module instance ...) are skipped.
+    '''
     # if it's a binary, leave it as is, we can't split it
     wast = None
     if not wastFile.endswith('.wasm'):
@@ -152,6 +157,8 @@ def to_end(j):
             ignoring_quoted = True
             continue
         if chunk.startswith('(module'):
+            if MODULE_DEFINITION_OR_INSTANCE.match(chunk):
+                continue
             ignoring_quoted = False
             ret += [(chunk, [])]
         elif chunk.startswith('(assert_invalid'):
@@ -190,14 +197,13 @@ def run_command(cmd, expected_status=0, stderr=None,
     out, err = proc.communicate()
     code = proc.returncode
     if expected_status is not None and code != expected_status:
-        raise Exception(('run_command failed (%s)' % code, out + str(err or '')))
+        raise Exception(f"run_command `{" ".join(cmd)}` failed ({code}) {err or ""}")
     if expected_err is not None:
         if err_ignore is not None:
             err = "\n".join([line for line in err.split('\n') if err_ignore not in line])
         err_correct = expected_err in err if err_contains else expected_err == err
         if not err_correct:
-            raise Exception(('run_command unexpected stderr',
-                             "expected '%s', actual '%s'" % (expected_err, err)))
+            raise Exception(f"run_command unexpected stderr. Expected '{expected_err}', actual '{err}'")
     return out
 
 

src/parser/wast-parser.cpp

@@ -88,16 +88,20 @@ Result<Action> action(Lexer& in) {
   return in.err("expected action");
 }
 
-// (module id? binary string*)
-// (module id? quote string*)
-// (module ...)
+// (module id binary string*)
+// (module id quote string*)
+// (module definition id? ...)
 Result<WASTModule> wastModule(Lexer& in, bool maybeInvalid = false) {
   Lexer reset = in;
   if (!in.takeSExprStart("module"sv)) {
     return in.err("expected module");
   }
-  // TODO: use ID?
-  [[maybe_unused]] auto id = in.takeID();
+
+  bool isDefinition = in.takeKeyword("definition"sv);
+
+  // We'll read this again in the 'inline module' case
+  (void) in.takeID();
+
   QuotedModuleType type;
   if (in.takeKeyword("quote"sv)) {
     type = QuotedModuleType::Text;
@@ -118,14 +122,30 @@ Result<WASTModule> wastModule(Lexer& in, bool maybeInvalid = false) {
       }
     }
     std::string mod(reset.next().substr(0, in.getPos() - reset.getPos()));
-    return QuotedModule{QuotedModuleType::Text, mod};
+    return QuotedModule{!isDefinition, QuotedModuleType::Text, mod};
   } else {
-    // This is a normal inline module that should be parseable. Reset to the
-    // start and parse it normally.
+    // In this case the module is mostly valid WAT, unless it is a module definition in which case it will begin with (module definition ...)
     in = std::move(reset);
+
+    // We already checked this before resetting
+    if (!in.takeSExprStart("module"sv)) {
+      return in.err("expected module");
+    }
+
+    bool isDefinition = in.takeKeyword("definition"sv);
+
     auto wasm = std::make_shared<Module>();
+    if (auto id = in.takeID()) {
+      wasm->name = *id;
+    }
+
     wasm->features = FeatureSet::All;
-    CHECK_ERR(parseModule(*wasm, in));
+    CHECK_ERR(parseModuleBody(*wasm, in));
+    if (!in.takeRParen()) {
+      return in.err("expected end of module");
+    }
+
+    wasm->isDefinition = isDefinition;
     return wasm;
   }
 
@@ -139,7 +159,7 @@ Result<WASTModule> wastModule(Lexer& in, bool maybeInvalid = false) {
     return in.err("expected end of module");
   }
 
-  return QuotedModule{type, ss.str()};
+  return QuotedModule{isDefinition, type, ss.str()};
 }
 
 Result<NaNKind> nan(Lexer& in) {
@@ -440,17 +460,61 @@ MaybeResult<Register> register_(Lexer& in) {
     return in.err("expected name");
   }
 
-  // TODO: Do we need to use this optional id?
-  in.takeID();
+  auto instanceName = in.takeID();
 
   if (!in.takeRParen()) {
-    // TODO: handle optional module id.
     return in.err("expected end of register command");
   }
-  return Register{*name};
+
+  return Register{.name=*name, .instanceName=instanceName};
 }
 
-// module | register | action | assertion
+// (module instance instance_name module_name)
+MaybeResult<ModuleInstantiation> instantiation(Lexer& in) {
+  if (!in.takeSExprStart("module"sv)) {
+    std::optional<std::string_view> actual = in.peekKeyword();
+    return in.err((std::stringstream()<<"expected `module` keyword but got "<< actual.value_or("<not a keyword>")).str());
+  }
+  
+  if (!in.takeKeyword("instance"sv)) {
+    // This is not a module instance and probably a module instead.
+    return {};
+  }
+
+  auto instanceId = in.takeID();
+  if (!instanceId.has_value()) {
+    return in.err("expected an instance id in module instantiation");
+  }
+  auto moduleId = in.takeID();
+  if (!moduleId.has_value()) {
+    return in.err("expected a module id in module instantiation");
+  }
+
+  if (!in.takeRParen()) {
+    return in.err("expected end of module instantiation");
+  }
+
+  return ModuleInstantiation{.moduleName = *moduleId, .instanceName = *instanceId};
+}
+
+using ModuleOrInstantiation = std::variant<ModuleInstantiation, WASTModule>;
+
+// (module instance ...) | (module ...)
+Result<ModuleOrInstantiation> moduleOrInstantiation(Lexer& in) {
+  auto reset = in;
+
+  if (auto inst = instantiation(in)) {
+    CHECK_ERR(inst);
+    return *inst;
+  }
+  in = reset;
+
+  auto module = wastModule(in);
+  CHECK_ERR(module);
+  return *module;
+}
+
+// instantiate | module | register | action | assertion
 Result<WASTCommand> command(Lexer& in) {
   if (auto cmd = register_(in)) {
     CHECK_ERR(cmd);
@@ -464,9 +528,10 @@ Result<WASTCommand> command(Lexer& in) {
     CHECK_ERR(cmd);
     return *cmd;
   }
-  auto mod = wastModule(in);
-  CHECK_ERR(mod);
-  return *mod;
+  auto cmd = moduleOrInstantiation(in);
+  CHECK_ERR(cmd);
+
+  return std::visit([](auto&& modOrInstantiation) -> WASTCommand { return modOrInstantiation; }, *cmd);
 }
 
 #pragma GCC diagnostic push

src/parser/wat-parser.cpp

@@ -137,4 +137,8 @@ Result<> parseModule(Module& wasm, Lexer& lexer) {
   return doParseModule(wasm, lexer, true);
 }
 
+Result<> parseModuleBody(Module& wasm, Lexer& lexer) {
+  return doParseModule(wasm, lexer, true);
+}
+
 } // namespace wasm::WATParser

src/parser/wat-parser.h

@@ -34,6 +34,9 @@ Result<> parseModule(Module& wasm,
 // file.
 Result<> parseModule(Module& wasm, Lexer& lexer);
 
+// Similar to `parseModule`, parse the fields of a single WAT module (after the initial module definition including its name) and stop at the ending right paren.
+Result<> parseModuleBody(Module& wasm, Lexer& lexer);
+
 Result<Literal> parseConst(Lexer& lexer);
 
 #pragma GCC diagnostic push
@@ -88,6 +91,7 @@ struct AssertAction {
 enum class QuotedModuleType { Text, Binary };
 
 struct QuotedModule {
+  bool isDefinition = false;
   QuotedModuleType type;
   std::string module;
 };
@@ -104,10 +108,17 @@ struct AssertModule {
 using Assertion = std::variant<AssertReturn, AssertAction, AssertModule>;
 
 struct Register {
+  // todo rename this
   Name name;
+  std::optional<Name> instanceName = std::nullopt;
+};
+
+struct ModuleInstantiation {
+  Name moduleName;
+  Name instanceName;
 };
 
-using WASTCommand = std::variant<WASTModule, Register, Action, Assertion>;
+using WASTCommand = std::variant<WASTModule, Register, Action, Assertion, ModuleInstantiation>;
 
 struct ScriptEntry {
   WASTCommand cmd;