[Parser] Parse pops (by doing nothing) (#6252)

Parse pop expressions and check that they have the expected types, but do not
actually create new Pop expressions or push anything onto the stack because we
already create Pop expressions as necessary when visiting the beginning of catch
blocks.

Unlike the legacy text parser, the new text parser is not capable of parsing
pops in invalid locations in the IR. This means that the new text parser will
never be able to parse test/lit/catch-pop-fixup-eh-old.wast, which deliberately
parses invalid IR to check that the pops can be fixed up and moved to the
correct locations. It should be acceptable to delete that test when we turn on
the new parser by default, though, so that won't be a problem.

4 files changed, 33 insertions, 3 deletions

diff --git a/src/parser/contexts.h b/src/parser/contexts.h
index 73be5e46e..c1ccefe53 100644
--- a/src/parser/contexts.h
+++ b/src/parser/contexts.h
@@ -440,6 +440,7 @@ struct NullInstrParserCtx {
 
   Result<> makeMemoryCopy(Index, MemoryIdxT*, MemoryIdxT*) { return Ok{}; }
   Result<> makeMemoryFill(Index, MemoryIdxT*) { return Ok{}; }
+  template<typename TypeT> Result<> makePop(Index, TypeT) { return Ok{}; }
   Result<> makeCall(Index, FuncIdxT, bool) { return Ok{}; }
   template<typename TypeUseT>
   Result<> makeCallIndirect(Index, TableIdxT*, TypeUseT, bool) {
@@ -1649,6 +1650,10 @@ struct ParseDefsCtx : TypeParserCtx<ParseDefsCtx> {
     return withLoc(pos, irBuilder.makeMemoryFill(*m));
   }
 
+  Result<> makePop(Index pos, Type type) {
+    return withLoc(pos, irBuilder.makePop(type));
+  }
+
   Result<> makeCall(Index pos, Name func, bool isReturn) {
     return withLoc(pos, irBuilder.makeCall(func, isReturn));
   }
diff --git a/src/parser/parsers.h b/src/parser/parsers.h
index 706c6525e..c7b9168b1 100644
--- a/src/parser/parsers.h
+++ b/src/parser/parsers.h
@@ -1555,7 +1555,9 @@ template<typename Ctx> Result<> makeMemoryFill(Ctx& ctx, Index pos) {
 }
 
 template<typename Ctx> Result<> makePop(Ctx& ctx, Index pos) {
-  return ctx.in.err("unimplemented instruction");
+  auto type = valtype(ctx);
+  CHECK_ERR(type);
+  return ctx.makePop(pos, *type);
 }
 
 template<typename Ctx> Result<> makeCall(Ctx& ctx, Index pos, bool isReturn) {
diff --git a/src/wasm-ir-builder.h b/src/wasm-ir-builder.h
index fce8bd431..d32dfebf2 100644
--- a/src/wasm-ir-builder.h
+++ b/src/wasm-ir-builder.h
@@ -145,7 +145,7 @@ public:
   [[nodiscard]] Result<> makeMemorySize(Name mem);
   [[nodiscard]] Result<> makeMemoryGrow(Name mem);
   [[nodiscard]] Result<> makeUnreachable();
-  // [[nodiscard]] Result<> makePop();
+  [[nodiscard]] Result<> makePop(Type type);
   [[nodiscard]] Result<> makeRefNull(HeapType type);
   [[nodiscard]] Result<> makeRefIsNull();
   [[nodiscard]] Result<> makeRefFunc(Name func);
@@ -232,6 +232,7 @@ public:
   [[nodiscard]] Result<>
   visitTupleExtract(TupleExtract*,
                     std::optional<uint32_t> arity = std::nullopt);
+  [[nodiscard]] Result<> visitPop(Pop*);
 
 private:
   Module& wasm;
diff --git a/src/wasm/wasm-ir-builder.cpp b/src/wasm/wasm-ir-builder.cpp
index 1eea777e0..f8c8cf0a0 100644
--- a/src/wasm/wasm-ir-builder.cpp
+++ b/src/wasm/wasm-ir-builder.cpp
@@ -604,6 +604,12 @@ Result<> IRBuilder::visitTupleExtract(TupleExtract* curr,
   return Ok{};
 }
 
+Result<> IRBuilder::visitPop(Pop*) {
+  // Do not actually push this pop onto the stack since we generate our own pops
+  // as necessary when visiting the beginnings of try blocks.
+  return Ok{};
+}
+
 Result<> IRBuilder::visitFunctionStart(Function* func) {
   if (!scopeStack.empty()) {
     return Err{"unexpected start of function"};
@@ -1300,7 +1306,23 @@ Result<> IRBuilder::makeUnreachable() {
   return Ok{};
 }
 
-// Result<> IRBuilder::makePop() {}
+Result<> IRBuilder::makePop(Type type) {
+  // We don't actually want to create a new Pop expression here because we
+  // already create them automatically when starting a legacy catch block that
+  // needs one. Just verify that the Pop we are being asked to make is the same
+  // type as the Pop we have already made.
+  auto& scope = getScope();
+  if (!scope.getCatch() || scope.exprStack.size() != 1 ||
+      !scope.exprStack[0]->is<Pop>()) {
+    return Err{
+      "pop instructions may only appear at the beginning of catch blocks"};
+  }
+  auto expectedType = scope.exprStack[0]->type;
+  if (type != expectedType) {
+    return Err{std::string("Expected pop of type ") + expectedType.toString()};
+  }
+  return Ok{};
+}
 
 Result<> IRBuilder::makeRefNull(HeapType type) {
   push(builder.makeRefNull(type));