Replace `RefIs` with `RefIsNull` (#5401)

* Replace `RefIs` with `RefIsNull`

The other `ref.is*` instructions are deprecated and expressible in terms of
`ref.test`. Update binary and text parsing to parse those instructions as
`RefTest` expressions. Also update the printing and emitting of `RefTest`
expressions to emit the legacy instructions for now to minimize test changes and
make this a mostly non-functional change. Since `ref.is_null` is the only
`RefIs` instruction left, remove the `RefIsOp` field and rename the expression
class to `RefIsNull`.

The few test changes are due to the fact that `ref.is*` instructions are now
subject to `ref.test` validation, and in particular it is no longer valid to
perform a `ref.is_func` on a value outside of the `func` type hierarchy.

6 files changed, 65 insertions, 54 deletions

diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp
index 2639599d5..b1f4b8907 100644
--- a/src/wasm/wasm-binary.cpp
+++ b/src/wasm/wasm-binary.cpp
@@ -3795,7 +3795,7 @@ BinaryConsts::ASTNodes WasmBinaryBuilder::readExpression(Expression*& curr) {
       visitRefNull((curr = allocator.alloc<RefNull>())->cast<RefNull>());
       break;
     case BinaryConsts::RefIsNull:
-      visitRefIs((curr = allocator.alloc<RefIs>())->cast<RefIs>(), code);
+      visitRefIsNull((curr = allocator.alloc<RefIsNull>())->cast<RefIsNull>());
       break;
     case BinaryConsts::RefFunc:
       visitRefFunc((curr = allocator.alloc<RefFunc>())->cast<RefFunc>());
@@ -4026,7 +4026,8 @@ BinaryConsts::ASTNodes WasmBinaryBuilder::readExpression(Expression*& curr) {
       if (opcode == BinaryConsts::RefIsFunc ||
           opcode == BinaryConsts::RefIsData ||
           opcode == BinaryConsts::RefIsI31) {
-        visitRefIs((curr = allocator.alloc<RefIs>())->cast<RefIs>(), opcode);
+        visitRefIs((curr = allocator.alloc<RefTest>())->cast<RefTest>(),
+                   opcode);
         break;
       }
       if (opcode == BinaryConsts::RefAsFunc ||
@@ -6591,25 +6592,28 @@ void WasmBinaryBuilder::visitRefNull(RefNull* curr) {
   curr->finalize(getHeapType().getBottom());
 }
 
-void WasmBinaryBuilder::visitRefIs(RefIs* curr, uint8_t code) {
+void WasmBinaryBuilder::visitRefIsNull(RefIsNull* curr) {
+  BYN_TRACE("zz node: RefIsNull\n");
+  curr->value = popNonVoidExpression();
+  curr->finalize();
+}
+
+void WasmBinaryBuilder::visitRefIs(RefTest* curr, uint8_t code) {
   BYN_TRACE("zz node: RefIs\n");
   switch (code) {
-    case BinaryConsts::RefIsNull:
-      curr->op = RefIsNull;
-      break;
     case BinaryConsts::RefIsFunc:
-      curr->op = RefIsFunc;
+      curr->castType = Type(HeapType::func, NonNullable);
       break;
     case BinaryConsts::RefIsData:
-      curr->op = RefIsData;
+      curr->castType = Type(HeapType::data, NonNullable);
       break;
     case BinaryConsts::RefIsI31:
-      curr->op = RefIsI31;
+      curr->castType = Type(HeapType::i31, NonNullable);
       break;
     default:
       WASM_UNREACHABLE("invalid code for ref.is_*");
   }
-  curr->value = popNonVoidExpression();
+  curr->ref = popNonVoidExpression();
   curr->finalize();
 }
 
diff --git a/src/wasm/wasm-s-parser.cpp b/src/wasm/wasm-s-parser.cpp
index 638dfbebf..4f725d48b 100644
--- a/src/wasm/wasm-s-parser.cpp
+++ b/src/wasm/wasm-s-parser.cpp
@@ -2536,9 +2536,8 @@ Expression* SExpressionWasmBuilder::makeRefNull(Element& s) {
   return ret;
 }
 
-Expression* SExpressionWasmBuilder::makeRefIs(Element& s, RefIsOp op) {
-  auto ret = allocator.alloc<RefIs>();
-  ret->op = op;
+Expression* SExpressionWasmBuilder::makeRefIsNull(Element& s) {
+  auto ret = allocator.alloc<RefIsNull>();
   ret->value = parseExpression(s[1]);
   ret->finalize();
   return ret;
@@ -2775,16 +2774,20 @@ Expression* SExpressionWasmBuilder::makeI31Get(Element& s, bool signed_) {
   return ret;
 }
 
-Expression* SExpressionWasmBuilder::makeRefTest(Element& s) {
+Expression* SExpressionWasmBuilder::makeRefTest(Element& s,
+                                                std::optional<Type> castType) {
   int i = 1;
-  auto nullability = NonNullable;
-  if (s[0]->str().str != "ref.test_static" && s[1]->str().str == "null") {
-    nullability = Nullable;
-    ++i;
+  if (!castType) {
+    auto nullability = NonNullable;
+    if (s[0]->str().str != "ref.test_static" && s[1]->str().str == "null") {
+      nullability = Nullable;
+      ++i;
+    }
+    auto type = parseHeapType(*s[i++]);
+    castType = Type(type, nullability);
   }
-  auto heapType = parseHeapType(*s[i++]);
   auto* ref = parseExpression(*s[i++]);
-  return Builder(wasm).makeRefTest(ref, Type(heapType, nullability));
+  return Builder(wasm).makeRefTest(ref, *castType);
 }
 
 Expression* SExpressionWasmBuilder::makeRefCast(Element& s) {
diff --git a/src/wasm/wasm-stack.cpp b/src/wasm/wasm-stack.cpp
index 24710f206..82c7c511f 100644
--- a/src/wasm/wasm-stack.cpp
+++ b/src/wasm/wasm-stack.cpp
@@ -1875,23 +1875,8 @@ void BinaryInstWriter::visitRefNull(RefNull* curr) {
   parent.writeHeapType(curr->type.getHeapType());
 }
 
-void BinaryInstWriter::visitRefIs(RefIs* curr) {
-  switch (curr->op) {
-    case RefIsNull:
-      o << int8_t(BinaryConsts::RefIsNull);
-      break;
-    case RefIsFunc:
-      o << int8_t(BinaryConsts::GCPrefix) << int8_t(BinaryConsts::RefIsFunc);
-      break;
-    case RefIsData:
-      o << int8_t(BinaryConsts::GCPrefix) << int8_t(BinaryConsts::RefIsData);
-      break;
-    case RefIsI31:
-      o << int8_t(BinaryConsts::GCPrefix) << int8_t(BinaryConsts::RefIsI31);
-      break;
-    default:
-      WASM_UNREACHABLE("unimplemented ref.is_*");
-  }
+void BinaryInstWriter::visitRefIsNull(RefIsNull* curr) {
+  o << int8_t(BinaryConsts::RefIsNull);
 }
 
 void BinaryInstWriter::visitRefFunc(RefFunc* curr) {
@@ -2025,6 +2010,23 @@ void BinaryInstWriter::visitCallRef(CallRef* curr) {
 
 void BinaryInstWriter::visitRefTest(RefTest* curr) {
   o << int8_t(BinaryConsts::GCPrefix);
+  // TODO: These instructions are deprecated. Remove them.
+  if (auto type = curr->castType.getHeapType();
+      curr->castType.isNonNullable() && type.isBasic()) {
+    switch (type.getBasic()) {
+      case HeapType::func:
+        o << U32LEB(BinaryConsts::RefIsFunc);
+        return;
+      case HeapType::data:
+        o << U32LEB(BinaryConsts::RefIsData);
+        return;
+      case HeapType::i31:
+        o << U32LEB(BinaryConsts::RefIsI31);
+        return;
+      default:
+        break;
+    }
+  }
   if (curr->castType.isNullable()) {
     o << U32LEB(BinaryConsts::RefTestNull);
   } else {
diff --git a/src/wasm/wasm-validator.cpp b/src/wasm/wasm-validator.cpp
index 486f13d24..28526cb33 100644
--- a/src/wasm/wasm-validator.cpp
+++ b/src/wasm/wasm-validator.cpp
@@ -427,7 +427,7 @@ public:
   void visitMemorySize(MemorySize* curr);
   void visitMemoryGrow(MemoryGrow* curr);
   void visitRefNull(RefNull* curr);
-  void visitRefIs(RefIs* curr);
+  void visitRefIsNull(RefIsNull* curr);
   void visitRefAs(RefAs* curr);
   void visitRefFunc(RefFunc* curr);
   void visitRefEq(RefEq* curr);
@@ -2132,14 +2132,15 @@ void FunctionValidator::visitRefNull(RefNull* curr) {
     curr->type.isNull(), curr, "ref.null must have a bottom heap type");
 }
 
-void FunctionValidator::visitRefIs(RefIs* curr) {
-  shouldBeTrue(getModule()->features.hasReferenceTypes(),
-               curr,
-               "ref.is_* requires reference-types [--enable-reference-types]");
+void FunctionValidator::visitRefIsNull(RefIsNull* curr) {
+  shouldBeTrue(
+    getModule()->features.hasReferenceTypes(),
+    curr,
+    "ref.is_null requires reference-types [--enable-reference-types]");
   shouldBeTrue(curr->value->type == Type::unreachable ||
                  curr->value->type.isRef(),
                curr->value,
-               "ref.is_*'s argument should be a reference type");
+               "ref.is_null's argument should be a reference type");
 }
 
 void FunctionValidator::visitRefAs(RefAs* curr) {
diff --git a/src/wasm/wasm.cpp b/src/wasm/wasm.cpp
index c6779a328..c121ba89a 100644
--- a/src/wasm/wasm.cpp
+++ b/src/wasm/wasm.cpp
@@ -805,7 +805,7 @@ void RefNull::finalize(Type type_) { type = type_; }
 
 void RefNull::finalize() {}
 
-void RefIs::finalize() {
+void RefIsNull::finalize() {
   if (value->type == Type::unreachable) {
     type = Type::unreachable;
   } else {
diff --git a/src/wasm/wat-parser.cpp b/src/wasm/wat-parser.cpp
index 14d30db1c..08eec41c6 100644
--- a/src/wasm/wat-parser.cpp
+++ b/src/wasm/wat-parser.cpp
@@ -758,7 +758,7 @@ struct NullInstrParserCtx {
   template<typename HeapTypeT> InstrT makeRefNull(Index, HeapTypeT) {
     return Ok{};
   }
-  InstrT makeRefIs(Index, RefIsOp) { return Ok{}; }
+  InstrT makeRefIsNull(Index) { return Ok{}; }
 
   InstrT makeRefEq(Index) { return Ok{}; }
 
@@ -2053,10 +2053,10 @@ struct ParseDefsCtx : TypeParserCtx<ParseDefsCtx> {
     return push(pos, builder.makeRefNull(type));
   }
 
-  Result<> makeRefIs(Index pos, RefIsOp op) {
+  Result<> makeRefIsNull(Index pos) {
     auto ref = pop(pos);
     CHECK_ERR(ref);
-    return push(pos, builder.makeRefIs(op, *ref));
+    return push(pos, builder.makeRefIsNull(*ref));
   }
 
   Result<> makeRefEq(Index pos) {
@@ -2322,8 +2322,7 @@ template<typename Ctx> Result<typename Ctx::InstrT> makeBreak(Ctx&, Index);
 template<typename Ctx> Result<typename Ctx::InstrT> makeBreakTable(Ctx&, Index);
 template<typename Ctx> Result<typename Ctx::InstrT> makeReturn(Ctx&, Index);
 template<typename Ctx> Result<typename Ctx::InstrT> makeRefNull(Ctx&, Index);
-template<typename Ctx>
-Result<typename Ctx::InstrT> makeRefIs(Ctx&, Index, RefIsOp op);
+template<typename Ctx> Result<typename Ctx::InstrT> makeRefIsNull(Ctx&, Index);
 template<typename Ctx> Result<typename Ctx::InstrT> makeRefFunc(Ctx&, Index);
 template<typename Ctx> Result<typename Ctx::InstrT> makeRefEq(Ctx&, Index);
 template<typename Ctx> Result<typename Ctx::InstrT> makeTableGet(Ctx&, Index);
@@ -2344,8 +2343,9 @@ Result<typename Ctx::InstrT> makeCallRef(Ctx&, Index, bool isReturn);
 template<typename Ctx> Result<typename Ctx::InstrT> makeI31New(Ctx&, Index);
 template<typename Ctx>
 Result<typename Ctx::InstrT> makeI31Get(Ctx&, Index, bool signed_);
-template<typename Ctx> Result<typename Ctx::InstrT> makeRefTest(Ctx&, Index);
-template<typename Ctx> Result<typename Ctx::InstrT> makeRefTest(Ctx&, Index);
+template<typename Ctx>
+Result<typename Ctx::InstrT>
+makeRefTest(Ctx&, Index, std::optional<Type> castType = std::nullopt);
 template<typename Ctx> Result<typename Ctx::InstrT> makeRefCast(Ctx&, Index);
 template<typename Ctx> Result<typename Ctx::InstrT> makeRefCastNop(Ctx&, Index);
 template<typename Ctx>
@@ -3357,8 +3357,8 @@ Result<typename Ctx::InstrT> makeRefNull(Ctx& ctx, Index pos) {
 }
 
 template<typename Ctx>
-Result<typename Ctx::InstrT> makeRefIs(Ctx& ctx, Index pos, RefIsOp op) {
-  return ctx.makeRefIs(pos, op);
+Result<typename Ctx::InstrT> makeRefIsNull(Ctx& ctx, Index pos) {
+  return ctx.makeRefIsNull(pos);
 }
 
 template<typename Ctx>
@@ -3438,7 +3438,8 @@ Result<typename Ctx::InstrT> makeI31Get(Ctx& ctx, Index pos, bool signed_) {
 }
 
 template<typename Ctx>
-Result<typename Ctx::InstrT> makeRefTest(Ctx& ctx, Index pos) {
+Result<typename Ctx::InstrT>
+makeRefTest(Ctx& ctx, Index pos, std::optional<Type> castType) {
   return ctx.in.err("unimplemented instruction");
 }