[GC] More HeapType instead of Type (#3475)

To handle both nullable and non-nullable i31s and other heap types, we cannot
just look at the isBasic case (which is just one of the two).

This may fix this issue on the release builder:
https://github.com/WebAssembly/binaryen/runs/1669944081?check_suite_focus=true
but the issue does not reproduce locally, so I worry it is something else...

2 files changed, 92 insertions, 55 deletions

diff --git a/src/literal.h b/src/literal.h
index 5b590e07f..fb5396147 100644
--- a/src/literal.h
+++ b/src/literal.h
@@ -279,7 +279,7 @@ public:
     return i32;
   }
   int32_t geti31(bool signed_ = true) const {
-    assert(type == Type::i31ref);
+    assert(type.getHeapType() == HeapType::i31);
     return signed_ ? (i32 << 1) >> 1 : i32;
   }
   int64_t geti64() const {
diff --git a/src/wasm/literal.cpp b/src/wasm/literal.cpp
index 3f6ca8ff5..91fc91e1e 100644
--- a/src/wasm/literal.cpp
+++ b/src/wasm/literal.cpp
@@ -73,40 +73,62 @@ Literal::Literal(const Literal& other) : type(other.type) {
     } else {
       new (&exn) std::unique_ptr<ExceptionPackage>();
     }
-  } else if (other.isGCData()) {
+    return;
+  }
+  if (other.isGCData()) {
     new (&gcData) std::shared_ptr<GCData>(other.gcData);
-  } else if (type.isFunction()) {
+    return;
+  }
+  if (type.isFunction()) {
     func = other.func;
-  } else if (type.isRtt()) {
+    return;
+  }
+  if (type.isRtt()) {
     // Allocate a new RttSupers with a copy of the other's data.
     new (&rttSupers) auto(std::make_unique<RttSupers>(*other.rttSupers));
-  } else {
-    TODO_SINGLE_COMPOUND(type);
-    switch (type.getBasic()) {
-      case Type::i32:
-      case Type::f32:
-      case Type::i31ref:
-        i32 = other.i32;
-        break;
-      case Type::i64:
-      case Type::f64:
-        i64 = other.i64;
-        break;
-      case Type::v128:
-        memcpy(&v128, other.v128, 16);
-        break;
-      case Type::none:
-        break;
-      case Type::externref:
-      case Type::anyref:
-      case Type::eqref:
-        break; // null
-      case Type::funcref:
-      case Type::exnref:
-      case Type::unreachable:
-        WASM_UNREACHABLE("unexpected type");
+    return;
+  }
+  if (type.isRef()) {
+    auto heapType = type.getHeapType();
+    if (heapType.isBasic()) {
+      switch (heapType.getBasic()) {
+        case HeapType::any:
+        case HeapType::ext:
+        case HeapType::eq:
+          return; // null
+        case HeapType::i31:
+          i32 = other.i32;
+          return;
+        case HeapType::func:
+        case HeapType::exn:
+          WASM_UNREACHABLE("invalid type");
+      }
     }
   }
+  TODO_SINGLE_COMPOUND(type);
+  switch (type.getBasic()) {
+    case Type::i32:
+    case Type::f32:
+      i32 = other.i32;
+      break;
+    case Type::i64:
+    case Type::f64:
+      i64 = other.i64;
+      break;
+    case Type::v128:
+      memcpy(&v128, other.v128, 16);
+      break;
+    case Type::none:
+      break;
+    case Type::unreachable:
+    case Type::funcref:
+    case Type::externref:
+    case Type::exnref:
+    case Type::anyref:
+    case Type::eqref:
+    case Type::i31ref:
+      WASM_UNREACHABLE("invalid type");
+  }
 }
 
 Literal::~Literal() {
@@ -116,8 +138,10 @@ Literal::~Literal() {
     gcData.~shared_ptr();
   } else if (type.isRtt()) {
     rttSupers.~unique_ptr();
-  } else if (type.isFunction()) {
-    // Nothing special to do.
+  } else if (type.isFunction() || type.isRef()) {
+    // Nothing special to do for a function or a non-GC reference (GC data was
+    // handled earlier). For references, this handles the case of (ref ? i31)
+    // for example, which may or may not be basic.
   } else {
     // Basic types need no special handling.
     // TODO: change this to an assert after we figure out the underlying issue
@@ -483,12 +507,41 @@ std::ostream& operator<<(std::ostream& o, Literal literal) {
     } else {
       o << "funcref(" << literal.getFunc() << ")";
     }
-  } else if (literal.isGCData()) {
-    auto data = literal.getGCData();
-    if (data) {
-      o << "[ref " << data->rtt << ' ' << data->values << ']';
+  } else if (literal.type.isRef()) {
+    if (literal.isGCData()) {
+      auto data = literal.getGCData();
+      if (data) {
+        o << "[ref " << data->rtt << ' ' << data->values << ']';
+      } else {
+        o << "[ref null " << literal.type << ']';
+      }
     } else {
-      o << "[ref null " << literal.type << ']';
+      switch (literal.type.getHeapType().getBasic()) {
+        case HeapType::ext:
+          assert(literal.isNull() && "unexpected non-null externref literal");
+          o << "externref(null)";
+          break;
+        case HeapType::exn:
+          if (literal.isNull()) {
+            o << "exnref(null)";
+          } else {
+            o << "exnref(" << literal.getExceptionPackage() << ")";
+          }
+          break;
+        case HeapType::any:
+          assert(literal.isNull() && "unexpected non-null anyref literal");
+          o << "anyref(null)";
+          break;
+        case HeapType::eq:
+          assert(literal.isNull() && "unexpected non-null eqref literal");
+          o << "eqref(null)";
+          break;
+        case HeapType::i31:
+          o << "i31ref(" << literal.geti31() << ")";
+          break;
+        case HeapType::func:
+          WASM_UNREACHABLE("invalid type");
+      }
     }
   } else if (literal.type.isRtt()) {
     o << "[rtt ";
@@ -518,30 +571,14 @@ std::ostream& operator<<(std::ostream& o, Literal literal) {
         o << "i32x4 ";
         literal.printVec128(o, literal.getv128());
         break;
+      case Type::funcref:
       case Type::externref:
-        assert(literal.isNull() && "unexpected non-null externref literal");
-        o << "externref(null)";
-        break;
       case Type::exnref:
-        if (literal.isNull()) {
-          o << "exnref(null)";
-        } else {
-          o << "exnref(" << literal.getExceptionPackage() << ")";
-        }
-        break;
       case Type::anyref:
-        assert(literal.isNull() && "unexpected non-null anyref literal");
-        o << "anyref(null)";
-        break;
       case Type::eqref:
-        assert(literal.isNull() && "unexpected non-null eqref literal");
-        o << "eqref(null)";
-        break;
       case Type::i31ref:
-        o << "i31ref(" << literal.geti31() << ")";
-        break;
-      default:
-        WASM_UNREACHABLE("invalid type");
+      case Type::unreachable:
+        WASM_UNREACHABLE("unexpected type");
     }
   }
   restoreNormalColor(o);