5 files changed, 27 insertions, 43 deletions

diff --git a/src/passes/OptimizeInstructions.cpp b/src/passes/OptimizeInstructions.cpp
index 097e0299a..e61373711 100644
--- a/src/passes/OptimizeInstructions.cpp
+++ b/src/passes/OptimizeInstructions.cpp
@@ -1872,28 +1872,28 @@ struct OptimizeInstructions
 
     auto intendedType = curr->type.getHeapType();
 
-    // If the value is a null, it will just flow through, and we do not need
-    // the cast. However, if that would change the type, then things are less
-    // simple: if the original type was non-nullable, replacing it with a null
-    // would change the type, which can happen in e.g.
-    //   (ref.cast (ref.as_non_null (.. (ref.null)
+    // If the value is a null, then we know a nullable cast will succeed and a
+    // non-nullable cast will fail. Either way, we do not need the cast.
+    // However, we have to avoid changing the type when replacing a cast with
+    // its potentially more refined child, e.g.
+    //   (ref.cast null (ref.as_non_null (.. (ref.null)))
     if (fallthrough->is<RefNull>()) {
-      // Replace the expression with drops of the inputs, and a null. Note
-      // that we provide a null of the previous type, so that we do not alter
-      // the type received by our parent.
-      Expression* rep = builder.makeSequence(builder.makeDrop(curr->ref),
-                                             builder.makeRefNull(intendedType));
-      if (curr->ref->type.isNonNullable()) {
-        // Avoid a type change by forcing to be non-nullable. In practice,
-        // this would have trapped before we get here, so this is just for
-        // validation.
-        rep = builder.makeRefAs(RefAsNonNull, rep);
-      }
-      replaceCurrent(rep);
-      return;
-      // TODO: The optimal ordering of this and the other ref.as_non_null
-      //       stuff later down in this functions is unclear and may be worth
-      //       looking into.
+      if (curr->type.isNullable()) {
+        // Replace the expression to drop the input and directly produce the
+        // null.
+        replaceCurrent(builder.makeSequence(builder.makeDrop(curr->ref),
+                                            builder.makeRefNull(intendedType)));
+        return;
+        // TODO: The optimal ordering of this and the other ref.as_non_null
+        //       stuff later down in this functions is unclear and may be worth
+        //       looking into.
+      } else {
+        // The cast will trap on the null, so replace it with an unreachable
+        // wrapped in a block of the original type.
+        replaceCurrent(builder.makeSequence(
+          builder.makeDrop(curr->ref), builder.makeUnreachable(), curr->type));
+        return;
+      }
     }
 
     // For the cast to be able to succeed, the value being cast must be a
diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp
index 386f89060..c6fb361bd 100644
--- a/src/wasm/wasm-binary.cpp
+++ b/src/wasm/wasm-binary.cpp
@@ -6920,14 +6920,6 @@ bool WasmBinaryBuilder::maybeVisitRefCast(Expression*& out, uint32_t code) {
     } else {
       nullability = code == BinaryConsts::RefCast ? NonNullable : Nullable;
     }
-    // Only accept instructions emulating the legacy behavior for now.
-    if (ref->type.isRef()) {
-      if (nullability == NonNullable && ref->type.isNullable()) {
-        throwError("ref.cast on nullable input not yet supported");
-      } else if (nullability == Nullable && ref->type.isNonNullable()) {
-        throwError("ref.cast null on non-nullable input not yet supported");
-      }
-    }
     auto safety =
       code == BinaryConsts::RefCastNop ? RefCast::Unsafe : RefCast::Safe;
     auto type = Type(heapType, nullability);
diff --git a/src/wasm/wasm-s-parser.cpp b/src/wasm/wasm-s-parser.cpp
index 4fdc29a66..2f791609a 100644
--- a/src/wasm/wasm-s-parser.cpp
+++ b/src/wasm/wasm-s-parser.cpp
@@ -2805,15 +2805,6 @@ Expression* SExpressionWasmBuilder::makeRefCast(Element& s) {
   if (legacy) {
     // Legacy polymorphic behavior.
     nullability = ref->type.getNullability();
-  } else if (ref->type.isRef()) {
-    // Only accept instructions emulating the legacy behavior for now.
-    if (nullability == NonNullable && ref->type.isNullable()) {
-      throw ParseException(
-        "ref.cast on nullable input not yet supported", s.line, s.col);
-    } else if (nullability == Nullable && ref->type.isNonNullable()) {
-      throw ParseException(
-        "ref.cast null on non-nullable input not yet supported", s.line, s.col);
-    }
   }
   auto type = Type(heapType, nullability);
   return Builder(wasm).makeRefCast(ref, type, RefCast::Safe);
diff --git a/src/wasm/wasm-validator.cpp b/src/wasm/wasm-validator.cpp
index 68fb58ec4..bec9b0ad0 100644
--- a/src/wasm/wasm-validator.cpp
+++ b/src/wasm/wasm-validator.cpp
@@ -2535,11 +2535,11 @@ void FunctionValidator::visitRefCast(RefCast* curr) {
     curr,
     "ref.cast target type and ref type must have a common supertype");
 
-  // TODO: Remove this restriction
-  shouldBeEqual(curr->type.getNullability(),
-                curr->ref->type.getNullability(),
-                curr,
-                "ref.cast to a different nullability not yet implemented");
+  // We should never have a nullable cast of a non-nullable reference, since
+  // that unnecessarily loses type information.
+  shouldBeTrue(curr->ref->type.isNullable() || curr->type.isNonNullable(),
+               curr,
+               "ref.cast null of non-nullable references are not allowed");
 }
 
 void FunctionValidator::visitBrOn(BrOn* curr) {
diff --git a/src/wasm/wasm.cpp b/src/wasm/wasm.cpp
index cf71689b1..682936461 100644
--- a/src/wasm/wasm.cpp
+++ b/src/wasm/wasm.cpp
@@ -945,6 +945,7 @@ void RefTest::finalize() {
 void RefCast::finalize() {
   if (ref->type == Type::unreachable) {
     type = Type::unreachable;
+    return;
   }
   // Do not unnecessarily lose non-nullability information.
   if (ref->type.isNonNullable() && type.isNullable()) {