From 10624834529814d5efc4582e848b6fbaf7237631 Mon Sep 17 00:00:00 2001
From: Thomas Lively <tlively@google.com>
Date: Thu, 25 Feb 2021 00:08:58 -0800
Subject: [PATCH 1/3] Support comparing and subtyping of recursive types

When the type section is emitted, types with an equal amount of references are
ordered by an arbitrary measure of simplicity, which previously would infinitely
recurse on structurally equivalent recursive types. Similarly, calculating
whether an recursive type was a subtype of another recursive type could have
infinitely recursed. This PR avoids infinite recursions in both cases by
switching the algorithms from using normal inductive recursion to using
coinductive recursion. The difference is that while the inductive algorithms
assume the relations do not hold for a pair of HeapTypes until they have been
exhaustively shown to hold, the coinductive algorithms assume the relations hold
unless a counterexample can be found.

There are additional algorithms that will need to be switched from inductive to
coinductive recursion, such as the type name generation and least upper bound
generation, but these two presented a good starting point and are sufficient to
get some interesting programs working end-to-end.

Also includes small drive-by cleanups in some related code.
---
 src/ir/module-splitting.cpp   |  13 -
 src/ir/module-utils.h         |   7 +-
 src/support/hash.h            |  13 +
 src/wasm-type.h               |  14 +-
 src/wasm/wasm-type.cpp        | 435 +++++++++++++++++++++-------------
 test/lit/recursive-types.wast |  32 +++
 test/subtypes.wast            |  15 ++
 7 files changed, 334 insertions(+), 195 deletions(-)
 create mode 100644 test/lit/recursive-types.wast

diff --git a/src/ir/module-splitting.cpp b/src/ir/module-splitting.cpp
index b8abbb42e6b..7215629e481 100644
--- a/src/ir/module-splitting.cpp
+++ b/src/ir/module-splitting.cpp
@@ -80,19 +80,6 @@
 #include "wasm-builder.h"
 #include "wasm.h"
 
-namespace std {
-
-// Used in ModuleSplitter::shareImportableItems
-template<> struct hash<pair<wasm::ExternalKind, wasm::Name>> {
-  size_t operator()(const pair<wasm::ExternalKind, wasm::Name>& p) const {
-    auto digest = wasm::hash(p.first);
-    wasm::rehash(digest, p.second);
-    return digest;
-  }
-};
-
-} // namespace std
-
 namespace wasm {
 
 namespace ModuleSplitting {
diff --git a/src/ir/module-utils.h b/src/ir/module-utils.h
index 84ef3a50851..4792c5d23a3 100644
--- a/src/ir/module-utils.h
+++ b/src/ir/module-utils.h
@@ -485,11 +485,8 @@ inline void collectHeapTypes(Module& wasm,
   for (auto& curr : wasm.functions) {
     counts.note(curr->sig);
     for (auto type : curr->vars) {
-      counts.maybeNote(type);
-      if (type.isTuple()) {
-        for (auto t : type) {
-          counts.maybeNote(t);
-        }
+      for (auto t : type) {
+        counts.maybeNote(t);
       }
     }
   }
diff --git a/src/support/hash.h b/src/support/hash.h
index 1af462b7e55..d3a858698ae 100644
--- a/src/support/hash.h
+++ b/src/support/hash.h
@@ -50,4 +50,17 @@ template<typename T> inline void rehash(std::size_t& digest, const T& value) {
 
 } // namespace wasm
 
+namespace std {
+
+// Hashing pairs is often useful
+template<typename T1, typename T2> struct hash<pair<T1, T2>> {
+  size_t operator()(const pair<T1, T2>& p) const {
+    auto digest = wasm::hash(p.first);
+    wasm::rehash(digest, p.second);
+    return digest;
+  }
+};
+
+} // namespace std
+
 #endif // wasm_support_hash_h
diff --git a/src/wasm-type.h b/src/wasm-type.h
index 3dcd53d0e77..d0008d357f9 100644
--- a/src/wasm-type.h
+++ b/src/wasm-type.h
@@ -171,7 +171,7 @@ class Type {
   bool operator!=(const Type& other) const { return id != other.id; }
   bool operator!=(const BasicType& other) const { return id != other; }
 
-  // Order types by some notion of simplicity
+  // Order types by some notion of simplicity.
   bool operator<(const Type& other) const;
 
   // Returns the type size in bytes. Only single types are supported.
@@ -184,6 +184,10 @@ class Type {
   // Returns the feature set required to use this type.
   FeatureSet getFeatures() const;
 
+  // Returns the tuple, assuming that this is a tuple type. Note that it is
+  // normally simpler to use operator[] and size() on the Type directly.
+  const Tuple& getTuple() const;
+
   // Gets the heap type corresponding to this type, assuming that it is a
   // reference or Rtt type.
   HeapType getHeapType() const;
@@ -349,6 +353,7 @@ class HeapType {
   bool operator!=(const HeapType& other) const { return id != other.id; }
   bool operator!=(const BasicHeapType& other) const { return id != other; }
 
+  // Order heap types by some notion of simplicity.
   bool operator<(const HeapType& other) const;
   std::string toString() const;
 
@@ -368,7 +373,6 @@ struct Tuple {
   Tuple(TypeList&& types) : types(std::move(types)) { validate(); }
   bool operator==(const Tuple& other) const { return types == other.types; }
   bool operator!=(const Tuple& other) const { return !(*this == other); }
-  bool operator<(const Tuple& other) const { return types < other.types; }
   std::string toString() const;
 
   // Prevent accidental copies
@@ -424,7 +428,6 @@ struct Field {
            mutable_ == other.mutable_;
   }
   bool operator!=(const Field& other) const { return !(*this == other); }
-  bool operator<(const Field& other) const;
   std::string toString() const;
 };
 
@@ -439,7 +442,6 @@ struct Struct {
   Struct(FieldList&& fields) : fields(std::move(fields)) {}
   bool operator==(const Struct& other) const { return fields == other.fields; }
   bool operator!=(const Struct& other) const { return !(*this == other); }
-  bool operator<(const Struct& other) const { return fields < other.fields; }
   std::string toString() const;
 
   // Prevent accidental copies
@@ -452,7 +454,6 @@ struct Array {
   Array(Field element) : element(element) {}
   bool operator==(const Array& other) const { return element == other.element; }
   bool operator!=(const Array& other) const { return !(*this == other); }
-  bool operator<(const Array& other) const { return element < other.element; }
   std::string toString() const;
 };
 
@@ -467,8 +468,7 @@ struct Rtt {
     return depth == other.depth && heapType == other.heapType;
   }
   bool operator!=(const Rtt& other) const { return !(*this == other); }
-  bool operator<(const Rtt& other) const;
-  bool hasDepth() { return depth != uint32_t(NoDepth); }
+  bool hasDepth() const { return depth != uint32_t(NoDepth); }
   std::string toString() const;
 };
 
diff --git a/src/wasm/wasm-type.cpp b/src/wasm/wasm-type.cpp
index b920dd587cc..a77288c8408 100644
--- a/src/wasm/wasm-type.cpp
+++ b/src/wasm/wasm-type.cpp
@@ -64,7 +64,6 @@ struct TypeInfo {
 
   bool operator==(const TypeInfo& other) const;
   bool operator!=(const TypeInfo& other) const { return !(*this == other); }
-  bool operator<(const TypeInfo& other) const;
 };
 
 struct HeapTypeInfo {
@@ -96,7 +95,33 @@ struct HeapTypeInfo {
   HeapTypeInfo& operator=(const HeapTypeInfo& other);
   bool operator==(const HeapTypeInfo& other) const;
   bool operator!=(const HeapTypeInfo& other) const { return !(*this == other); }
-  bool operator<(const HeapTypeInfo& other) const;
+};
+
+struct TypeComparator {
+  std::unordered_set<std::pair<HeapType, HeapType>> cache;
+  bool cmp(Type a, Type b);
+  bool cmp(HeapType a, HeapType b);
+  bool cmp(const TypeInfo& a, const TypeInfo& b);
+  bool cmp(const HeapTypeInfo& a, const HeapTypeInfo& b);
+  bool cmp(const Tuple& a, const Tuple& b);
+  bool cmp(const Field& a, const Field& b);
+  bool cmp(const Signature& a, const Signature& b);
+  bool cmp(const Struct& a, const Struct& b);
+  bool cmp(const Array& a, const Array& b);
+  bool cmp(const Rtt& a, const Rtt& b);
+};
+
+struct SubTyper {
+  std::unordered_set<std::pair<HeapType, HeapType>> cache;
+  bool isSubType(Type a, Type b);
+  bool isSubType(HeapType a, HeapType b);
+  bool isSubType(const Tuple& a, const Tuple& b);
+  bool isSubType(const Field& a, const Field& b);
+  bool isSubType(const HeapTypeInfo& a, const HeapTypeInfo& b);
+  bool isSubType(const Signature& a, const Signature& b);
+  bool isSubType(const Struct& a, const Struct& b);
+  bool isSubType(const Array& a, const Array& b);
+  bool isSubType(const Rtt& a, const Rtt& b);
 };
 
 } // anonymous namespace
@@ -176,24 +201,6 @@ bool TypeInfo::operator==(const TypeInfo& other) const {
   WASM_UNREACHABLE("unexpected kind");
 }
 
-bool TypeInfo::operator<(const TypeInfo& other) const {
-  if (kind != other.kind) {
-    return kind < other.kind;
-  }
-  switch (kind) {
-    case TupleKind:
-      return tuple < other.tuple;
-    case RefKind:
-      if (ref.nullable != other.ref.nullable) {
-        return ref.nullable < other.ref.nullable;
-      }
-      return ref.heapType < other.ref.heapType;
-    case RttKind:
-      return rtt < other.rtt;
-  }
-  WASM_UNREACHABLE("unexpected kind");
-}
-
 HeapTypeInfo::HeapTypeInfo(const HeapTypeInfo& other) {
   kind = other.kind;
   switch (kind) {
@@ -248,21 +255,6 @@ bool HeapTypeInfo::operator==(const HeapTypeInfo& other) const {
   WASM_UNREACHABLE("unexpected kind");
 }
 
-bool HeapTypeInfo::operator<(const HeapTypeInfo& other) const {
-  if (kind != other.kind) {
-    return kind < other.kind;
-  }
-  switch (kind) {
-    case SignatureKind:
-      return signature < other.signature;
-    case StructKind:
-      return struct_ < other.struct_;
-    case ArrayKind:
-      return array < other.array;
-  }
-  WASM_UNREACHABLE("unexpected kind");
-}
-
 template<typename Info> struct Store {
   std::mutex mutex;
 
@@ -423,20 +415,8 @@ bool Type::isDefaultable() const {
 }
 
 bool Type::operator<(const Type& other) const {
-  if (*this == other) {
-    return false;
-  }
-  if (isBasic() && other.isBasic()) {
-    return getBasic() < other.getBasic();
-  }
-  if (isBasic()) {
-    return true;
-  }
-  if (other.isBasic()) {
-    return false;
-  }
-  return *getTypeInfo(*this) < *getTypeInfo(other);
-};
+  return TypeComparator().cmp(*this, other);
+}
 
 unsigned Type::getByteSize() const {
   // TODO: alignment?
@@ -536,6 +516,11 @@ FeatureSet Type::getFeatures() const {
   return getSingleFeatures(*this);
 }
 
+const Tuple& Type::getTuple() const {
+  assert(isTuple());
+  return getTypeInfo(*this)->tuple;
+}
+
 HeapType Type::getHeapType() const {
   if (isBasic()) {
     switch (getBasic()) {
@@ -597,39 +582,7 @@ Type Type::get(unsigned byteSize, bool float_) {
 }
 
 bool Type::isSubType(Type left, Type right) {
-  if (left == right) {
-    return true;
-  }
-  if (left.isRef() && right.isRef()) {
-    // Consider HeapType subtyping as well, and also that a non-nullable type is
-    // potentially a supertype of a nullable one.
-    if (HeapType::isSubType(left.getHeapType(), right.getHeapType()) &&
-        (left.isNullable() == right.isNullable() || !left.isNullable())) {
-      return true;
-    }
-    return false;
-  }
-  if (left.isTuple() && right.isTuple()) {
-    if (left.size() != right.size()) {
-      return false;
-    }
-    for (size_t i = 0; i < left.size(); ++i) {
-      if (!isSubType(left[i], right[i])) {
-        return false;
-      }
-    }
-    return true;
-  }
-  if (left.isRtt() && right.isRtt()) {
-    auto leftRtt = left.getRtt();
-    auto rightRtt = right.getRtt();
-    // (rtt n $x) is a subtype of (rtt $x), that is, if the only difference in
-    // information is that the left side specifies a depth while the right side
-    // allows any depth.
-    return leftRtt.heapType == rightRtt.heapType && leftRtt.hasDepth() &&
-           !rightRtt.hasDepth();
-  }
-  return false;
+  return SubTyper().isSubType(left, right);
 }
 
 Type Type::getLeastUpperBound(Type a, Type b) {
@@ -774,19 +727,7 @@ bool HeapType::isArray() const {
 }
 
 bool HeapType::operator<(const HeapType& other) const {
-  if (*this == other) {
-    return false;
-  }
-  if (isBasic() && other.isBasic()) {
-    return getBasic() < other.getBasic();
-  }
-  if (isBasic()) {
-    return true;
-  }
-  if (other.isBasic()) {
-    return false;
-  }
-  return *getHeapTypeInfo(*this) < *getHeapTypeInfo(other);
+  return TypeComparator().cmp(*this, other);
 }
 
 Signature HeapType::getSignature() const {
@@ -804,85 +745,12 @@ Array HeapType::getArray() const {
   return getHeapTypeInfo(*this)->array;
 }
 
-static bool isFieldSubType(const Field& left, const Field& right) {
-  if (left == right) {
-    return true;
-  }
-  // Immutable fields can be subtypes.
-  if (left.mutable_ == Immutable && right.mutable_ == Immutable) {
-    return left.packedType == right.packedType &&
-           Type::isSubType(left.type, right.type);
-  }
-  return false;
-}
-
 bool HeapType::isSubType(HeapType left, HeapType right) {
-  // See:
-  // https://github.com/WebAssembly/function-references/blob/master/proposals/function-references/Overview.md#subtyping
-  // https://github.com/WebAssembly/gc/blob/master/proposals/gc/MVP.md#defined-types
-  if (left == right) {
-    return true;
-  }
-  // Everything is a subtype of any.
-  if (right == HeapType::any) {
-    return true;
-  }
-  // Various things are subtypes of eq.
-  if ((left == HeapType::i31 || left.isArray() || left.isStruct()) &&
-      right == HeapType::eq) {
-    return true;
-  }
-  // All typed function signatures are subtypes of funcref.
-  // Note: signatures may get covariance at some point, but do not yet.
-  if (left.isSignature() && right == HeapType::func) {
-    return true;
-  }
-  if (left.isArray() && right.isArray()) {
-    auto leftField = left.getArray().element;
-    auto rightField = left.getArray().element;
-    // Array types support depth subtyping.
-    return isFieldSubType(leftField, rightField);
-  }
-  if (left.isStruct() && right.isStruct()) {
-    // Structure types support width and depth subtyping.
-    auto leftFields = left.getStruct().fields;
-    auto rightFields = left.getStruct().fields;
-    // There may be more fields on the left, but not less.
-    if (leftFields.size() < rightFields.size()) {
-      return false;
-    }
-    for (size_t i = 0; i < rightFields.size(); i++) {
-      if (!isFieldSubType(leftFields[i], rightFields[i])) {
-        return false;
-      }
-    }
-    return true;
-  }
-  return false;
+  return SubTyper().isSubType(left, right);
 }
 
 bool Signature::operator<(const Signature& other) const {
-  if (results != other.results) {
-    return results < other.results;
-  }
-  return params < other.params;
-}
-
-bool Field::operator<(const Field& other) const {
-  if (mutable_ != other.mutable_) {
-    return mutable_ < other.mutable_;
-  }
-  if (type == Type::i32 && other.type == Type::i32) {
-    return packedType < other.packedType;
-  }
-  return type < other.type;
-}
-
-bool Rtt::operator<(const Rtt& other) const {
-  if (depth != other.depth) {
-    return depth < other.depth;
-  }
-  return heapType < other.heapType;
+  return TypeComparator().cmp(*this, other);
 }
 
 namespace {
@@ -1087,6 +955,233 @@ std::ostream& operator<<(std::ostream& os, HeapTypeInfo info) {
   WASM_UNREACHABLE("unexpected kind");
 }
 
+namespace {
+
+bool TypeComparator::cmp(Type a, Type b) {
+  if (a == b) {
+    return false;
+  }
+  if (a.isBasic() && b.isBasic()) {
+    return a.getBasic() < b.getBasic();
+  }
+  if (a.isBasic()) {
+    return true;
+  }
+  if (b.isBasic()) {
+    return false;
+  }
+  return cmp(*getTypeInfo(a), *getTypeInfo(b));
+}
+
+bool TypeComparator::cmp(HeapType a, HeapType b) {
+  if (a == b) {
+    return false;
+  }
+  if (cache.count({a, b})) {
+    return true;
+  }
+  if (a.isBasic() && b.isBasic()) {
+    return a.getBasic() < b.getBasic();
+  }
+  if (a.isBasic()) {
+    return true;
+  }
+  if (b.isBasic()) {
+    return false;
+  }
+  cache.insert({a, b});
+  return cmp(*getHeapTypeInfo(a), *getHeapTypeInfo(b));
+}
+
+bool TypeComparator::cmp(const TypeInfo& a, const TypeInfo& b) {
+  if (a.kind != b.kind) {
+    return a.kind < b.kind;
+  }
+  switch (a.kind) {
+    case TypeInfo::TupleKind:
+      return cmp(a.tuple, b.tuple);
+    case TypeInfo::RefKind:
+      if (a.ref.nullable != b.ref.nullable) {
+        return a.ref.nullable < b.ref.nullable;
+      }
+      return cmp(a.ref.heapType, b.ref.heapType);
+    case TypeInfo::RttKind:
+      return cmp(a.rtt, b.rtt);
+  }
+  WASM_UNREACHABLE("unexpected kind");
+}
+
+bool TypeComparator::cmp(const HeapTypeInfo& a, const HeapTypeInfo& b) {
+  if (a.kind != b.kind) {
+    return a.kind < b.kind;
+  }
+  switch (a.kind) {
+    case HeapTypeInfo::SignatureKind:
+      return cmp(a.signature, b.signature);
+    case HeapTypeInfo::StructKind:
+      return cmp(a.struct_, b.struct_);
+    case HeapTypeInfo::ArrayKind:
+      return cmp(a.array, b.array);
+  }
+  WASM_UNREACHABLE("unexpected kind");
+}
+
+bool TypeComparator::cmp(const Tuple& a, const Tuple& b) {
+  return std::lexicographical_compare(
+    a.types.begin(),
+    a.types.end(),
+    b.types.begin(),
+    b.types.end(),
+    [&](Type ta, Type tb) { return cmp(ta, tb); });
+}
+
+bool TypeComparator::cmp(const Field& a, const Field& b) {
+  if (a.mutable_ != b.mutable_) {
+    return a.mutable_ < b.mutable_;
+  }
+  if (a.type == Type::i32 && b.type == Type::i32) {
+    return a.packedType < b.packedType;
+  }
+  return cmp(a.type, b.type);
+}
+
+bool TypeComparator::cmp(const Signature& a, const Signature& b) {
+  if (a.results != b.results) {
+    return cmp(a.results, b.results);
+  }
+  return cmp(a.params, b.params);
+}
+
+bool TypeComparator::cmp(const Struct& a, const Struct& b) {
+  return std::lexicographical_compare(
+    a.fields.begin(),
+    a.fields.end(),
+    b.fields.begin(),
+    b.fields.end(),
+    [&](const Field& fa, const Field& fb) { return cmp(fa, fb); });
+}
+
+bool TypeComparator::cmp(const Array& a, const Array& b) {
+  return cmp(a.element, b.element);
+}
+
+bool TypeComparator::cmp(const Rtt& a, const Rtt& b) {
+  if (a.depth != b.depth) {
+    return a.depth < b.depth;
+  }
+  return cmp(a.heapType, b.heapType);
+}
+
+bool SubTyper::isSubType(Type a, Type b) {
+  if (a == b) {
+    return true;
+  }
+  if (a.isRef() && b.isRef()) {
+    return (a.isNullable() == b.isNullable() || !b.isNullable()) &&
+           isSubType(a.getHeapType(), b.getHeapType());
+  }
+  if (a.isTuple() && b.isTuple()) {
+    return isSubType(a.getTuple(), b.getTuple());
+  }
+  if (a.isRtt() && b.isRtt()) {
+    return isSubType(a.getRtt(), b.getRtt());
+  }
+  return false;
+}
+
+bool SubTyper::isSubType(HeapType a, HeapType b) {
+  // See:
+  // https://github.com/WebAssembly/function-references/blob/master/proposals/function-references/Overview.md#subtyping
+  // https://github.com/WebAssembly/gc/blob/master/proposals/gc/MVP.md#defined-types
+  if (a == b) {
+    return true;
+  }
+  if (cache.count({a, b})) {
+    return true;
+  }
+  // Everything is a subtype of any.
+  if (b == HeapType::any) {
+    return true;
+  }
+  // Various things are subtypes of eq.
+  if (b == HeapType::eq) {
+    return a == HeapType::i31 || a.isArray() || a.isStruct();
+  }
+  // Some are also subtypes of data.
+  if (b == HeapType::data) {
+    return a.isData();
+  }
+  // Signatures are subtypes of funcref.
+  if (b == HeapType::func) {
+    return a.isSignature();
+  }
+  cache.insert({a, b});
+  if (a.isSignature() && b.isSignature()) {
+    return isSubType(a.getSignature(), b.getSignature());
+  }
+  if (a.isArray() && b.isArray()) {
+    return isSubType(a.getArray(), b.getArray());
+  }
+  if (a.isStruct() && b.isStruct()) {
+    return isSubType(a.getStruct(), b.getStruct());
+  }
+  return false;
+}
+
+bool SubTyper::isSubType(const Tuple& a, const Tuple& b) {
+  if (a.types.size() != b.types.size()) {
+    return false;
+  }
+  for (size_t i = 0; i < a.types.size(); ++i) {
+    if (!isSubType(a.types[i], b.types[i])) {
+      return false;
+    }
+  }
+  return true;
+}
+
+bool SubTyper::isSubType(const Field& a, const Field& b) {
+  if (a == b) {
+    return true;
+  }
+  // Immutable fields can be subtypes.
+  return a.mutable_ == Immutable && b.mutable_ == Immutable &&
+         a.packedType == b.packedType && isSubType(a.type, b.type);
+}
+
+bool SubTyper::isSubType(const Signature& a, const Signature& b) {
+  // TODO: Implement proper signature subtyping, covariant in results and
+  // contravariant in params, once V8 implements it.
+  // return isSubType(b.params, a.params) && isSubType(a.results, b.results);
+  return a == b;
+}
+
+bool SubTyper::isSubType(const Struct& a, const Struct& b) {
+  // There may be more fields on the left, but not fewer.
+  if (a.fields.size() < b.fields.size()) {
+    return false;
+  }
+  for (size_t i = 0; i < b.fields.size(); ++i) {
+    if (!isSubType(a.fields[i], b.fields[i])) {
+      return false;
+    }
+  }
+  return true;
+}
+
+bool SubTyper::isSubType(const Array& a, const Array& b) {
+  return isSubType(a.element, b.element);
+}
+
+bool SubTyper::isSubType(const Rtt& a, const Rtt& b) {
+  // (rtt n $x) is a subtype of (rtt $x), that is, if the only difference in
+  // information is that the left side specifies a depth while the right side
+  // allows any depth.
+  return a.heapType == b.heapType && a.hasDepth() && !b.hasDepth();
+}
+
+} // anonymous namespace
+
 struct TypeBuilder::Impl {
   TypeStore typeStore;
 
diff --git a/test/lit/recursive-types.wast b/test/lit/recursive-types.wast
new file mode 100644
index 00000000000..4589c6b5109
--- /dev/null
+++ b/test/lit/recursive-types.wast
@@ -0,0 +1,32 @@
+;; Test a trivial recursive type works properly
+
+;; RUN: wasm-opt %s -all -S -o - | filecheck %s
+
+;; TODO: Store function signatures as the originally parsed HeapTypes rather
+;; than directly as Signatures so that self-referential function signatures are
+;; not emitted twice. This happens because collectHeapTypes has to reconstruct a
+;; HeapType from each function's signature, but self-referential HeapTypes
+;; aren't canonicalized when they are parsed.
+
+;; CHECK:      (module
+;; CHECK-NEXT:   (type $t (func (param (ref null $t)) (result (ref null $t))))
+;; CHECK-NEXT:   (type $s (func (param (ref null $s)) (result (ref null $s))))
+;; CHECK-NEXT:   (type $ref?|$t|_=>_ref?|$t| (func (param (ref null $t)) (result (ref null $t))))
+;; CHECK-NEXT:   (type $ref?|$s|_=>_ref?|$s| (func (param (ref null $s)) (result (ref null $s))))
+;; CHECK-NEXT:   (func $foo (param $0 (ref null $t)) (result (ref null $t))
+;; CHECK-NEXT:     (unreachable)
+;; CHECK-NEXT:   )
+;; CHECK-NEXT:   (func $bar (param $0 (ref null $s)) (result (ref null $s))
+;; CHECK-NEXT:     (unreachable)
+;; CHECK-NEXT:   )
+
+(module
+  (type $t (func (param (ref null $t)) (result (ref null $t))))
+  (type $s (func (param (ref null $s)) (result (ref null $s))))
+  (func $foo (type $t)
+    (unreachable)
+  )
+  (func $bar (type $s)
+    (unreachable)
+  )
+)
diff --git a/test/subtypes.wast b/test/subtypes.wast
index d37735777a2..06f85a2733f 100644
--- a/test/subtypes.wast
+++ b/test/subtypes.wast
@@ -18,6 +18,15 @@
     (field (ref any))
   ))
 
+  ;; Recursive structs
+  (type $struct-rec-one (struct
+    (field (ref $struct-rec-one))
+  ))
+  (type $struct-rec-two (struct
+    (field (ref $struct-rec-two))
+    (field (ref $struct-rec-two))
+  ))
+
   (func $foo (param $no-null (ref $vector-i32))
              (param $yes-null (ref null $vector-i32))
     ;; ok to set a non-nullable reference to a nullable target
@@ -41,4 +50,10 @@
     ;; also ok to have extra fields
     (local.set $s-i31 (local.get $s-i31_any))
   )
+
+  (func $coinductive (param $rec-one (ref $struct-rec-one))
+                     (param $rec-two (ref $struct-rec-two))
+    ;; Do not infinitely recurse when determining this subtype relation!
+    (local.set $rec-one (local.get $rec-two))
+  )
 )

From 6fae7a9ac08af76416ab7ae33279005ee8d08e68 Mon Sep 17 00:00:00 2001
From: Thomas Lively <tlively@google.com>
Date: Thu, 25 Feb 2021 10:41:12 -0800
Subject: [PATCH 2/3] Address feedback, fix bug, update tests

---
 src/wasm/wasm-type.cpp       | 106 ++++++++++++++++++++---------------
 test/subtypes.wast.from-wast |   8 +++
 2 files changed, 69 insertions(+), 45 deletions(-)

diff --git a/src/wasm/wasm-type.cpp b/src/wasm/wasm-type.cpp
index a77288c8408..a2132d09ac9 100644
--- a/src/wasm/wasm-type.cpp
+++ b/src/wasm/wasm-type.cpp
@@ -97,22 +97,30 @@ struct HeapTypeInfo {
   bool operator!=(const HeapTypeInfo& other) const { return !(*this == other); }
 };
 
+// Helper for coinductively comparing Types and HeapTypes according to some
+// arbitrary notion of complexity.
 struct TypeComparator {
-  std::unordered_set<std::pair<HeapType, HeapType>> cache;
-  bool cmp(Type a, Type b);
-  bool cmp(HeapType a, HeapType b);
-  bool cmp(const TypeInfo& a, const TypeInfo& b);
-  bool cmp(const HeapTypeInfo& a, const HeapTypeInfo& b);
-  bool cmp(const Tuple& a, const Tuple& b);
-  bool cmp(const Field& a, const Field& b);
-  bool cmp(const Signature& a, const Signature& b);
-  bool cmp(const Struct& a, const Struct& b);
-  bool cmp(const Array& a, const Array& b);
-  bool cmp(const Rtt& a, const Rtt& b);
+  // Set of HeapTypes we are assuming satisfy the relation as long as we cannot
+  // prove otherwise.
+  std::unordered_set<std::pair<HeapType, HeapType>> seen;
+  bool lessThan(Type a, Type b);
+  bool lessThan(HeapType a, HeapType b);
+  bool lessThan(const TypeInfo& a, const TypeInfo& b);
+  bool lessThan(const HeapTypeInfo& a, const HeapTypeInfo& b);
+  bool lessThan(const Tuple& a, const Tuple& b);
+  bool lessThan(const Field& a, const Field& b);
+  bool lessThan(const Signature& a, const Signature& b);
+  bool lessThan(const Struct& a, const Struct& b);
+  bool lessThan(const Array& a, const Array& b);
+  bool lessThan(const Rtt& a, const Rtt& b);
 };
 
+// Helper for coinductively checking whether a pair of Types or HeapTypes are in
+// a subtype relation.
 struct SubTyper {
-  std::unordered_set<std::pair<HeapType, HeapType>> cache;
+  // Set of HeapTypes we are assuming satisfy the relation as long as we cannot
+  // prove otherwise.
+  std::unordered_set<std::pair<HeapType, HeapType>> seen;
   bool isSubType(Type a, Type b);
   bool isSubType(HeapType a, HeapType b);
   bool isSubType(const Tuple& a, const Tuple& b);
@@ -415,7 +423,7 @@ bool Type::isDefaultable() const {
 }
 
 bool Type::operator<(const Type& other) const {
-  return TypeComparator().cmp(*this, other);
+  return TypeComparator().lessThan(*this, other);
 }
 
 unsigned Type::getByteSize() const {
@@ -727,7 +735,7 @@ bool HeapType::isArray() const {
 }
 
 bool HeapType::operator<(const HeapType& other) const {
-  return TypeComparator().cmp(*this, other);
+  return TypeComparator().lessThan(*this, other);
 }
 
 Signature HeapType::getSignature() const {
@@ -750,7 +758,7 @@ bool HeapType::isSubType(HeapType left, HeapType right) {
 }
 
 bool Signature::operator<(const Signature& other) const {
-  return TypeComparator().cmp(*this, other);
+  return TypeComparator().lessThan(*this, other);
 }
 
 namespace {
@@ -957,7 +965,7 @@ std::ostream& operator<<(std::ostream& os, HeapTypeInfo info) {
 
 namespace {
 
-bool TypeComparator::cmp(Type a, Type b) {
+bool TypeComparator::lessThan(Type a, Type b) {
   if (a == b) {
     return false;
   }
@@ -970,14 +978,16 @@ bool TypeComparator::cmp(Type a, Type b) {
   if (b.isBasic()) {
     return false;
   }
-  return cmp(*getTypeInfo(a), *getTypeInfo(b));
+  return lessThan(*getTypeInfo(a), *getTypeInfo(b));
 }
 
-bool TypeComparator::cmp(HeapType a, HeapType b) {
+bool TypeComparator::lessThan(HeapType a, HeapType b) {
   if (a == b) {
     return false;
   }
-  if (cache.count({a, b})) {
+  if (seen.count({a, b})) {
+    // We weren't able to disprove that a < b since we last saw them, so the
+    // relation holds coinductively.
     return true;
   }
   if (a.isBasic() && b.isBasic()) {
@@ -989,87 +999,89 @@ bool TypeComparator::cmp(HeapType a, HeapType b) {
   if (b.isBasic()) {
     return false;
   }
-  cache.insert({a, b});
-  return cmp(*getHeapTypeInfo(a), *getHeapTypeInfo(b));
+  // As we recurse, we will coinductively assume that a < b unless proven
+  // otherwise.
+  seen.insert({a, b});
+  return lessThan(*getHeapTypeInfo(a), *getHeapTypeInfo(b));
 }
 
-bool TypeComparator::cmp(const TypeInfo& a, const TypeInfo& b) {
+bool TypeComparator::lessThan(const TypeInfo& a, const TypeInfo& b) {
   if (a.kind != b.kind) {
     return a.kind < b.kind;
   }
   switch (a.kind) {
     case TypeInfo::TupleKind:
-      return cmp(a.tuple, b.tuple);
+      return lessThan(a.tuple, b.tuple);
     case TypeInfo::RefKind:
       if (a.ref.nullable != b.ref.nullable) {
         return a.ref.nullable < b.ref.nullable;
       }
-      return cmp(a.ref.heapType, b.ref.heapType);
+      return lessThan(a.ref.heapType, b.ref.heapType);
     case TypeInfo::RttKind:
-      return cmp(a.rtt, b.rtt);
+      return lessThan(a.rtt, b.rtt);
   }
   WASM_UNREACHABLE("unexpected kind");
 }
 
-bool TypeComparator::cmp(const HeapTypeInfo& a, const HeapTypeInfo& b) {
+bool TypeComparator::lessThan(const HeapTypeInfo& a, const HeapTypeInfo& b) {
   if (a.kind != b.kind) {
     return a.kind < b.kind;
   }
   switch (a.kind) {
     case HeapTypeInfo::SignatureKind:
-      return cmp(a.signature, b.signature);
+      return lessThan(a.signature, b.signature);
     case HeapTypeInfo::StructKind:
-      return cmp(a.struct_, b.struct_);
+      return lessThan(a.struct_, b.struct_);
     case HeapTypeInfo::ArrayKind:
-      return cmp(a.array, b.array);
+      return lessThan(a.array, b.array);
   }
   WASM_UNREACHABLE("unexpected kind");
 }
 
-bool TypeComparator::cmp(const Tuple& a, const Tuple& b) {
+bool TypeComparator::lessThan(const Tuple& a, const Tuple& b) {
   return std::lexicographical_compare(
     a.types.begin(),
     a.types.end(),
     b.types.begin(),
     b.types.end(),
-    [&](Type ta, Type tb) { return cmp(ta, tb); });
+    [&](Type ta, Type tb) { return lessThan(ta, tb); });
 }
 
-bool TypeComparator::cmp(const Field& a, const Field& b) {
+bool TypeComparator::lessThan(const Field& a, const Field& b) {
   if (a.mutable_ != b.mutable_) {
     return a.mutable_ < b.mutable_;
   }
   if (a.type == Type::i32 && b.type == Type::i32) {
     return a.packedType < b.packedType;
   }
-  return cmp(a.type, b.type);
+  return lessThan(a.type, b.type);
 }
 
-bool TypeComparator::cmp(const Signature& a, const Signature& b) {
+bool TypeComparator::lessThan(const Signature& a, const Signature& b) {
   if (a.results != b.results) {
-    return cmp(a.results, b.results);
+    return lessThan(a.results, b.results);
   }
-  return cmp(a.params, b.params);
+  return lessThan(a.params, b.params);
 }
 
-bool TypeComparator::cmp(const Struct& a, const Struct& b) {
+bool TypeComparator::lessThan(const Struct& a, const Struct& b) {
   return std::lexicographical_compare(
     a.fields.begin(),
     a.fields.end(),
     b.fields.begin(),
     b.fields.end(),
-    [&](const Field& fa, const Field& fb) { return cmp(fa, fb); });
+    [&](const Field& fa, const Field& fb) { return lessThan(fa, fb); });
 }
 
-bool TypeComparator::cmp(const Array& a, const Array& b) {
-  return cmp(a.element, b.element);
+bool TypeComparator::lessThan(const Array& a, const Array& b) {
+  return lessThan(a.element, b.element);
 }
 
-bool TypeComparator::cmp(const Rtt& a, const Rtt& b) {
+bool TypeComparator::lessThan(const Rtt& a, const Rtt& b) {
   if (a.depth != b.depth) {
     return a.depth < b.depth;
   }
-  return cmp(a.heapType, b.heapType);
+  return lessThan(a.heapType, b.heapType);
 }
 
 bool SubTyper::isSubType(Type a, Type b) {
@@ -1077,7 +1089,7 @@ bool SubTyper::isSubType(Type a, Type b) {
     return true;
   }
   if (a.isRef() && b.isRef()) {
-    return (a.isNullable() == b.isNullable() || !b.isNullable()) &&
+    return (a.isNullable() == b.isNullable() || !a.isNullable()) &&
            isSubType(a.getHeapType(), b.getHeapType());
   }
   if (a.isTuple() && b.isTuple()) {
@@ -1096,7 +1108,9 @@ bool SubTyper::isSubType(HeapType a, HeapType b) {
   if (a == b) {
     return true;
   }
-  if (cache.count({a, b})) {
+  if (seen.count({a, b})) {
+    // We weren't able to disprove that a is a subtype of b since we last saw
+    // them, so the relation holds coinductively.
     return true;
   }
   // Everything is a subtype of any.
@@ -1115,7 +1129,9 @@ bool SubTyper::isSubType(HeapType a, HeapType b) {
   if (b == HeapType::func) {
     return a.isSignature();
   }
-  cache.insert({a, b});
+  // As we recurse, we will coinductively assume that a is a subtype of b unless
+  // proven otherwise.
+  seen.insert({a, b});
   if (a.isSignature() && b.isSignature()) {
     return isSubType(a.getSignature(), b.getSignature());
   }
diff --git a/test/subtypes.wast.from-wast b/test/subtypes.wast.from-wast
index db4d69f6d12..3e811109a93 100644
--- a/test/subtypes.wast.from-wast
+++ b/test/subtypes.wast.from-wast
@@ -1,8 +1,11 @@
 (module
+ (type $struct-rec-two (struct (field (ref null $struct-rec-two)) (field (ref null $struct-rec-two))))
  (type $struct-i31 (struct (field (ref null i31))))
+ (type $struct-rec-one (struct (field (ref null $struct-rec-one))))
  (type $vector-i32 (array i32))
  (type $ref?|$struct-i31|_ref?|$struct-any|_=>_none (func (param (ref null $struct-i31) (ref null $struct-any))))
  (type $ref?|$struct-i31|_ref?|$struct-i31_any|_=>_none (func (param (ref null $struct-i31) (ref null $struct-i31_any))))
+ (type $ref?|$struct-rec-one|_ref?|$struct-rec-two|_=>_none (func (param (ref null $struct-rec-one) (ref null $struct-rec-two))))
  (type $ref?|$vector-i32|_ref?|$vector-i32|_=>_none (func (param (ref null $vector-i32) (ref null $vector-i32))))
  (type $ref?|$vector-i31|_ref?|$vector-any|_=>_none (func (param (ref null $vector-i31) (ref null $vector-any))))
  (type $struct-any (struct (field anyref)))
@@ -29,4 +32,9 @@
    (local.get $s-i31_any)
   )
  )
+ (func $coinductive (param $rec-one (ref null $struct-rec-one)) (param $rec-two (ref null $struct-rec-two))
+  (local.set $rec-one
+   (local.get $rec-two)
+  )
+ )
 )

From 49eb5431e06d9473f2fb41a4296b1b55554e1e22 Mon Sep 17 00:00:00 2001
From: Thomas Lively <tlively@google.com>
Date: Thu, 25 Feb 2021 14:21:34 -0800
Subject: [PATCH 3/3] Support recursive type names as well

---
 src/passes/Print.cpp                        | 245 ++++++++++++--------
 test/heap-types.wast.fromBinary.noDebugInfo |   4 +-
 test/lit/recursive-types.wast               |  34 +--
 test/subtypes.wast.fromBinary               |   8 +
 test/subtypes.wast.fromBinary.noDebugInfo   |   8 +
 5 files changed, 187 insertions(+), 112 deletions(-)

diff --git a/src/passes/Print.cpp b/src/passes/Print.cpp
index 7a160a8eb3f..2c10a44a93a 100644
--- a/src/passes/Print.cpp
+++ b/src/passes/Print.cpp
@@ -67,70 +67,56 @@ static std::ostream& printLocal(Index index, Function* func, std::ostream& o) {
   return printName(name, o);
 }
 
-static void printHeapTypeName(std::ostream& os,
-                              HeapType type,
-                              Module* wasm = nullptr,
-                              bool first = true);
+namespace {
+
+// Helper for printing the name of a type. This output is guaranteed to not
+// contain spaces.
+struct TypeNamePrinter {
+  // Optional. If present, the module's HeapType names will be used.
+  Module* wasm;
+
+  // Keep track of the first depth at which we see each HeapType so if we see it
+  // again, we can unambiguously refer to it without infinitely recursing.
+  size_t currHeapTypeDepth = 0;
+  std::unordered_map<HeapType, size_t> heapTypeDepths;
+
+  // The stream we are printing to.
+  std::ostream& os;
+
+  TypeNamePrinter(std::ostream& os, Module* wasm = nullptr)
+    : wasm(wasm), os(os) {}
+
+  void print(Type type);
+  void print(HeapType heapType);
+  void print(const Tuple& tuple);
+  void print(const Field& field);
+  void print(const Signature& sig);
+  void print(const Struct& struct_);
+  void print(const Array& array);
+  void print(const Rtt& rtt);
+};
 
-// Prints the name of a type. This output is guaranteed to not contain spaces.
-static void printTypeName(std::ostream& os, Type type, Module* wasm = nullptr) {
+void TypeNamePrinter::print(Type type) {
   if (type.isBasic()) {
     os << type;
-    return;
-  }
-  if (type.isRtt()) {
-    auto rtt = type.getRtt();
-    os << "rtt_";
-    if (rtt.hasDepth()) {
-      os << rtt.depth << '_';
-    }
-    printHeapTypeName(os, rtt.heapType, wasm);
-    return;
-  }
-  if (type.isTuple()) {
-    auto sep = "";
-    for (auto t : type) {
-      os << sep;
-      sep = "_";
-      printTypeName(os, t, wasm);
-    }
-    return;
-  }
-  if (type.isRef()) {
+  } else if (type.isTuple()) {
+    print(type.getTuple());
+  } else if (type.isRtt()) {
+    print(type.getRtt());
+  } else if (type.isRef()) {
     os << "ref";
     if (type.isNullable()) {
       os << "?";
     }
-    os << "|";
-    printHeapTypeName(os, type.getHeapType(), wasm, false);
-    os << "|";
-    return;
-  }
-  WASM_UNREACHABLE("unsupported print type");
-}
-
-static void
-printFieldName(std::ostream& os, const Field& field, Module* wasm = nullptr) {
-  if (field.mutable_) {
-    os << "mut:";
-  }
-  if (field.type == Type::i32 && field.packedType != Field::not_packed) {
-    if (field.packedType == Field::i8) {
-      os << "i8";
-    } else if (field.packedType == Field::i16) {
-      os << "i16";
-    } else {
-      WASM_UNREACHABLE("invalid packed type");
-    }
+    os << '|';
+    print(type.getHeapType());
+    os << '|';
   } else {
-    printTypeName(os, field.type, wasm);
+    WASM_UNREACHABLE("unexpected type");
   }
 }
 
-// Prints the name of a heap type. As with printTypeName, this output is
-// guaranteed to not contain spaces.
-static void
-printHeapTypeName(std::ostream& os, HeapType type, Module* wasm, bool first) {
+void TypeNamePrinter::print(HeapType type) {
   if (type.isBasic()) {
     os << type;
     return;
@@ -144,37 +130,104 @@ printHeapTypeName(std::ostream& os, HeapType type, Module* wasm, bool first) {
     os << '$' << wasm->typeNames[type].name;
     return;
   }
-  if (first) {
-    os << '$';
+  // If we have seen this HeapType before, just print its relative depth instead
+  // of infinitely recursing.
+  auto it = heapTypeDepths.find(type);
+  if (it != heapTypeDepths.end()) {
+    assert(it->second <= currHeapTypeDepth);
+    size_t relativeDepth = currHeapTypeDepth - it->second;
+    os << "..." << relativeDepth;
+    return;
+  }
+
+  // If this is the top-level heap type, add a $
+  if (currHeapTypeDepth == 0) {
+    os << "$";
   }
+
+  // Update the context for the current HeapType before recursing.
+  heapTypeDepths[type] = ++currHeapTypeDepth;
+
   if (type.isSignature()) {
-    auto sig = type.getSignature();
-    printTypeName(os, sig.params, wasm);
-    if (first) {
-      os << "_=>_";
-    } else {
-      os << "_->_";
-    }
-    printTypeName(os, sig.results, wasm);
+    print(type.getSignature());
   } else if (type.isStruct()) {
-    auto struct_ = type.getStruct();
-    os << "{";
-    auto sep = "";
-    for (auto& field : struct_.fields) {
-      os << sep;
-      sep = "_";
-      printFieldName(os, field, wasm);
-    }
-    os << "}";
+    print(type.getStruct());
   } else if (type.isArray()) {
-    os << "[";
-    printFieldName(os, type.getArray().element, wasm);
-    os << "]";
+    print(type.getArray());
   } else {
-    os << type;
+    WASM_UNREACHABLE("unexpected type");
+  }
+
+  // Restore the previous context after the recursion.
+  heapTypeDepths.erase(type);
+  --currHeapTypeDepth;
+}
+
+void TypeNamePrinter::print(const Tuple& tuple) {
+  auto sep = "";
+  for (auto type : tuple.types) {
+    os << sep;
+    sep = "_";
+    print(type);
   }
 }
 
+void TypeNamePrinter::print(const Field& field) {
+  if (field.mutable_) {
+    os << "mut:";
+  }
+  if (field.type == Type::i32 && field.packedType != Field::not_packed) {
+    if (field.packedType == Field::i8) {
+      os << "i8";
+    } else if (field.packedType == Field::i16) {
+      os << "i16";
+    } else {
+      WASM_UNREACHABLE("invalid packed type");
+    }
+  } else {
+    print(field.type);
+  }
+}
+
+void TypeNamePrinter::print(const Signature& sig) {
+  // TODO: Switch to using an unambiguous delimiter rather than differentiating
+  // only the top level with a different arrow.
+  print(sig.params);
+  if (currHeapTypeDepth == 1) {
+    os << "_=>_";
+  } else {
+    os << "_->_";
+  }
+  print(sig.results);
+}
+
+void TypeNamePrinter::print(const Struct& struct_) {
+  os << '{';
+  auto sep = "";
+  for (const auto& field : struct_.fields) {
+    os << sep;
+    sep = "_";
+    print(field);
+  }
+  os << '}';
+}
+
+void TypeNamePrinter::print(const Array& array) {
+  os << '[';
+  print(array.element);
+  os << ']';
+}
+
+void TypeNamePrinter::print(const Rtt& rtt) {
+  os << "rtt_";
+  if (rtt.hasDepth()) {
+    os << rtt.depth << '_';
+  }
+  print(rtt.heapType);
+}
+
+} // anonymous namespace
+
 // Unlike the default format, tuple types in s-expressions should not have
 // commas.
 struct SExprType {
@@ -186,7 +239,9 @@ static std::ostream& printSExprType(std::ostream& o,
                                     const SExprType& sType,
                                     Module* wasm = nullptr) {
   Type type = sType.type;
-  if (type.isTuple()) {
+  if (type.isBasic()) {
+    o << type;
+  } else if (type.isTuple()) {
     o << '(';
     auto sep = "";
     for (const auto& t : type) {
@@ -201,17 +256,17 @@ static std::ostream& printSExprType(std::ostream& o,
     if (rtt.hasDepth()) {
       o << rtt.depth << ' ';
     }
-    printHeapTypeName(o, rtt.heapType, wasm);
+    TypeNamePrinter(o, wasm).print(rtt.heapType);
     o << ')';
   } else if (type.isRef() && !type.isBasic()) {
     o << "(ref ";
     if (type.isNullable()) {
       o << "null ";
     }
-    printHeapTypeName(o, type.getHeapType(), wasm);
+    TypeNamePrinter(o, wasm).print(type.getHeapType());
     o << ')';
   } else {
-    printTypeName(o, sType.type, wasm);
+    WASM_UNREACHABLE("unexpected type");
   }
   return o;
 }
@@ -356,7 +411,7 @@ struct PrintExpressionContents
 
     o << '(';
     printMinor(o, "type ");
-    printHeapTypeName(o, curr->sig, wasm);
+    TypeNamePrinter(o, wasm).print(HeapType(curr->sig));
     o << ')';
   }
   void visitLocalGet(LocalGet* curr) {
@@ -1793,7 +1848,7 @@ struct PrintExpressionContents
   void visitMemoryGrow(MemoryGrow* curr) { printMedium(o, "memory.grow"); }
   void visitRefNull(RefNull* curr) {
     printMedium(o, "ref.null ");
-    printHeapTypeName(o, curr->type.getHeapType(), wasm);
+    TypeNamePrinter(o, wasm).print(curr->type.getHeapType());
   }
   void visitRefIs(RefIs* curr) {
     switch (curr->op) {
@@ -1864,11 +1919,11 @@ struct PrintExpressionContents
   }
   void visitRefTest(RefTest* curr) {
     printMedium(o, "ref.test ");
-    printHeapTypeName(o, curr->getCastType().getHeapType(), wasm);
+    TypeNamePrinter(o, wasm).print(curr->getCastType().getHeapType());
   }
   void visitRefCast(RefCast* curr) {
     printMedium(o, "ref.cast ");
-    printHeapTypeName(o, curr->getCastType().getHeapType(), wasm);
+    TypeNamePrinter(o, wasm).print(curr->getCastType().getHeapType());
   }
   void visitBrOn(BrOn* curr) {
     switch (curr->op) {
@@ -1894,11 +1949,11 @@ struct PrintExpressionContents
   }
   void visitRttCanon(RttCanon* curr) {
     printMedium(o, "rtt.canon ");
-    printHeapTypeName(o, curr->type.getRtt().heapType, wasm);
+    TypeNamePrinter(o, wasm).print(curr->type.getRtt().heapType);
   }
   void visitRttSub(RttSub* curr) {
     printMedium(o, "rtt.sub ");
-    printHeapTypeName(o, curr->type.getRtt().heapType, wasm);
+    TypeNamePrinter(o, wasm).print(curr->type.getRtt().heapType);
   }
   void visitStructNew(StructNew* curr) {
     printMedium(o, "struct.new_");
@@ -1906,7 +1961,7 @@ struct PrintExpressionContents
       o << "default_";
     }
     o << "with_rtt ";
-    printHeapTypeName(o, curr->rtt->type.getHeapType(), wasm);
+    TypeNamePrinter(o, wasm).print(curr->rtt->type.getHeapType());
   }
   void printUnreachableReplacement() {
     // If we cannot print a valid unreachable instruction (say, a struct.get,
@@ -1940,7 +1995,7 @@ struct PrintExpressionContents
     } else {
       printMedium(o, "struct.get ");
     }
-    printHeapTypeName(o, heapType, wasm);
+    TypeNamePrinter(o, wasm).print(heapType);
     o << ' ';
     printFieldName(heapType, curr->index);
   }
@@ -1951,7 +2006,7 @@ struct PrintExpressionContents
     }
     printMedium(o, "struct.set ");
     auto heapType = curr->ref->type.getHeapType();
-    printHeapTypeName(o, heapType, wasm);
+    TypeNamePrinter(o, wasm).print(heapType);
     o << ' ';
     printFieldName(heapType, curr->index);
   }
@@ -1961,7 +2016,7 @@ struct PrintExpressionContents
       o << "default_";
     }
     o << "with_rtt ";
-    printHeapTypeName(o, curr->rtt->type.getHeapType(), wasm);
+    TypeNamePrinter(o, wasm).print(curr->rtt->type.getHeapType());
   }
   void visitArrayGet(ArrayGet* curr) {
     const auto& element = curr->ref->type.getHeapType().getArray().element;
@@ -1974,15 +2029,15 @@ struct PrintExpressionContents
     } else {
       printMedium(o, "array.get ");
     }
-    printHeapTypeName(o, curr->ref->type.getHeapType(), wasm);
+    TypeNamePrinter(o, wasm).print(curr->ref->type.getHeapType());
   }
   void visitArraySet(ArraySet* curr) {
     printMedium(o, "array.set ");
-    printHeapTypeName(o, curr->ref->type.getHeapType(), wasm);
+    TypeNamePrinter(o, wasm).print(curr->ref->type.getHeapType());
   }
   void visitArrayLen(ArrayLen* curr) {
     printMedium(o, "array.len ");
-    printHeapTypeName(o, curr->ref->type.getHeapType(), wasm);
+    TypeNamePrinter(o, wasm).print(curr->ref->type.getHeapType());
   }
   void visitRefAs(RefAs* curr) {
     switch (curr->op) {
@@ -3251,7 +3306,7 @@ struct PrintSExpression : public OverriddenVisitor<PrintSExpression> {
       doIndent(o, indent);
       o << '(';
       printMedium(o, "type") << ' ';
-      printHeapTypeName(o, type, curr);
+      TypeNamePrinter(o, curr).print(type);
       o << ' ';
       handleHeapType(type);
       o << ")" << maybeNewLine;
@@ -3432,7 +3487,7 @@ printStackInst(StackInst* inst, std::ostream& o, Function* func) {
     case StackInst::TryEnd: {
       printMedium(o, "end");
       o << " ;; type: ";
-      printTypeName(o, inst->type);
+      TypeNamePrinter(o).print(inst->type);
       break;
     }
     case StackInst::IfElse: {
diff --git a/test/heap-types.wast.fromBinary.noDebugInfo b/test/heap-types.wast.fromBinary.noDebugInfo
index 1ae5792cb5f..a9718b010cd 100644
--- a/test/heap-types.wast.fromBinary.noDebugInfo
+++ b/test/heap-types.wast.fromBinary.noDebugInfo
@@ -7,8 +7,8 @@
  (type ${} (struct ))
  (type ${mut:f32} (struct (field (mut f32))))
  (type $none_=>_none (func))
- (type $rtt_1_${}_=>_none (func (param (rtt 1 ${}))))
- (type $rtt_${}_=>_none (func (param (rtt ${}))))
+ (type $rtt_1_{}_=>_none (func (param (rtt 1 ${}))))
+ (type $rtt_{}_=>_none (func (param (rtt ${}))))
  (type $ref?|{i32_f32_f64}|_=>_ref?|{i8_mut:i16_ref?|{i32_f32_f64}|_mut:ref?|{i32_f32_f64}|}| (func (param (ref null ${i32_f32_f64})) (result (ref null ${i8_mut:i16_ref?|{i32_f32_f64}|_mut:ref?|{i32_f32_f64}|}))))
  (type $ref?|[mut:f64]|_=>_ref?|[ref?|[mut:f64]|]| (func (param (ref null $[mut:f64])) (result (ref null $[ref?|[mut:f64]|]))))
  (type ${i32} (struct (field i32)))
diff --git a/test/lit/recursive-types.wast b/test/lit/recursive-types.wast
index 4589c6b5109..c825c9cad30 100644
--- a/test/lit/recursive-types.wast
+++ b/test/lit/recursive-types.wast
@@ -2,31 +2,35 @@
 
 ;; RUN: wasm-opt %s -all -S -o - | filecheck %s
 
-;; TODO: Store function signatures as the originally parsed HeapTypes rather
-;; than directly as Signatures so that self-referential function signatures are
-;; not emitted twice. This happens because collectHeapTypes has to reconstruct a
-;; HeapType from each function's signature, but self-referential HeapTypes
-;; aren't canonicalized when they are parsed.
+;; TODO: Fix the bug where self-referential function signatures are emitted
+;; twice. This happens because collectHeapTypes has to reconstruct a HeapType
+;; from each function's signature, but self-referential HeapTypes aren't
+;; canonicalized when they are parsed so collectHeapTypes ends up with a
+;; separate, unfolded version of the type.
+
+;; TODO: Fix the bug where structurally identical types are given the same
+;; generated name, making the wast invalid due to duplicate names.
 
 ;; CHECK:      (module
-;; CHECK-NEXT:   (type $t (func (param (ref null $t)) (result (ref null $t))))
-;; CHECK-NEXT:   (type $s (func (param (ref null $s)) (result (ref null $s))))
-;; CHECK-NEXT:   (type $ref?|$t|_=>_ref?|$t| (func (param (ref null $t)) (result (ref null $t))))
-;; CHECK-NEXT:   (type $ref?|$s|_=>_ref?|$s| (func (param (ref null $s)) (result (ref null $s))))
-;; CHECK-NEXT:   (func $foo (param $0 (ref null $t)) (result (ref null $t))
+;; CHECK-NEXT:   (type $ref?|...0|_=>_ref?|...0| (func (param (ref null $ref?|...0|_=>_ref?|...0|)) (result (ref null $ref?|...0|_=>_ref?|...0|))))
+;; CHECK-NEXT:   (type $ref?|...0|_=>_ref?|...0| (func (param (ref null $ref?|...0|_=>_ref?|...0|)) (result (ref null $ref?|...0|_=>_ref?|...0|))))
+;; CHECK-NEXT:   (type $ref?|ref?|...0|_->_ref?|...0||_=>_ref?|ref?|...0|_->_ref?|...0|| (func (param (ref null $ref?|...0|_=>_ref?|...0|)) (result (ref null $ref?|...0|_=>_ref?|...0|))))
+;; CHECK-NEXT:   (type $ref?|ref?|...0|_->_ref?|...0||_=>_ref?|ref?|...0|_->_ref?|...0|| (func (param (ref null $ref?|...0|_=>_ref?|...0|)) (result (ref null $ref?|...0|_=>_ref?|...0|))))
+;; CHECK-NEXT:   (func $foo (param $0 (ref null $ref?|...0|_=>_ref?|...0|)) (result (ref null $ref?|...0|_=>_ref?|...0|))
 ;; CHECK-NEXT:     (unreachable)
 ;; CHECK-NEXT:   )
-;; CHECK-NEXT:   (func $bar (param $0 (ref null $s)) (result (ref null $s))
+;; CHECK-NEXT:   (func $bar (param $0 (ref null $ref?|...0|_=>_ref?|...0|)) (result (ref null $ref?|...0|_=>_ref?|...0|))
 ;; CHECK-NEXT:     (unreachable)
 ;; CHECK-NEXT:   )
+;; CHECK-NEXT: )
 
 (module
-  (type $t (func (param (ref null $t)) (result (ref null $t))))
-  (type $s (func (param (ref null $s)) (result (ref null $s))))
-  (func $foo (type $t)
+  (type (func (param (ref null 0)) (result (ref null 0))))
+  (type (func (param (ref null 1)) (result (ref null 1))))
+  (func $foo (type 0)
     (unreachable)
   )
-  (func $bar (type $s)
+  (func $bar (type 1)
     (unreachable)
   )
 )
diff --git a/test/subtypes.wast.fromBinary b/test/subtypes.wast.fromBinary
index d169a4086cc..c2fcb0e3d94 100644
--- a/test/subtypes.wast.fromBinary
+++ b/test/subtypes.wast.fromBinary
@@ -1,8 +1,11 @@
 (module
+ (type $struct-rec-two (struct (field (ref null $struct-rec-two)) (field (ref null $struct-rec-two))))
  (type $struct-i31 (struct (field (ref null i31))))
+ (type $struct-rec-one (struct (field (ref null $struct-rec-one))))
  (type $vector-i32 (array i32))
  (type $ref?|$struct-i31|_ref?|$struct-any|_=>_none (func (param (ref null $struct-i31) (ref null $struct-any))))
  (type $ref?|$struct-i31|_ref?|$struct-i31_any|_=>_none (func (param (ref null $struct-i31) (ref null $struct-i31_any))))
+ (type $ref?|$struct-rec-one|_ref?|$struct-rec-two|_=>_none (func (param (ref null $struct-rec-one) (ref null $struct-rec-two))))
  (type $ref?|$vector-i32|_ref?|$vector-i32|_=>_none (func (param (ref null $vector-i32) (ref null $vector-i32))))
  (type $ref?|$vector-i31|_ref?|$vector-any|_=>_none (func (param (ref null $vector-i31) (ref null $vector-any))))
  (type $struct-any (struct (field anyref)))
@@ -29,5 +32,10 @@
    (local.get $s-i31_any)
   )
  )
+ (func $coinductive (param $rec-one (ref null $struct-rec-one)) (param $rec-two (ref null $struct-rec-two))
+  (local.set $rec-one
+   (local.get $rec-two)
+  )
+ )
 )
 
diff --git a/test/subtypes.wast.fromBinary.noDebugInfo b/test/subtypes.wast.fromBinary.noDebugInfo
index d80e5afd011..a108cf151e8 100644
--- a/test/subtypes.wast.fromBinary.noDebugInfo
+++ b/test/subtypes.wast.fromBinary.noDebugInfo
@@ -1,8 +1,11 @@
 (module
+ (type ${ref?|...0|_ref?|...0|} (struct (field (ref null ${ref?|...0|_ref?|...0|})) (field (ref null ${ref?|...0|_ref?|...0|}))))
  (type ${ref?|i31|} (struct (field (ref null i31))))
+ (type ${ref?|...0|} (struct (field (ref null ${ref?|...0|}))))
  (type $[i32] (array i32))
  (type $ref?|{ref?|i31|}|_ref?|{anyref}|_=>_none (func (param (ref null ${ref?|i31|}) (ref null ${anyref}))))
  (type $ref?|{ref?|i31|}|_ref?|{ref?|i31|_anyref}|_=>_none (func (param (ref null ${ref?|i31|}) (ref null ${ref?|i31|_anyref}))))
+ (type $ref?|{ref?|...0|}|_ref?|{ref?|...0|_ref?|...0|}|_=>_none (func (param (ref null ${ref?|...0|}) (ref null ${ref?|...0|_ref?|...0|}))))
  (type $ref?|[i32]|_ref?|[i32]|_=>_none (func (param (ref null $[i32]) (ref null $[i32]))))
  (type $ref?|[ref?|i31|]|_ref?|[anyref]|_=>_none (func (param (ref null $[ref?|i31|]) (ref null $[anyref]))))
  (type ${anyref} (struct (field anyref)))
@@ -29,5 +32,10 @@
    (local.get $1)
   )
  )
+ (func $4 (param $0 (ref null ${ref?|...0|})) (param $1 (ref null ${ref?|...0|_ref?|...0|}))
+  (local.set $0
+   (local.get $1)
+  )
+ )
 )