Fix SubtypingDiscoverer with shared i31 (#7774)

SubtypingDiscoverer previously noted that the child of I31Get
expressions were subtypes of i31, but they can alternatively be subtypes
of (shared i31) and this was not handled. This should properly be
modeled as a new kind of type constraint in SubtypingDiscoverer, but the
more robust solution would be to refactor SubtypingDiscoverer to use
principal types so the sharedness can be a type variable. For now, just
look at the child to figure out what shareness to use.

This bug was found because it caused an infinite loop in Unsubtyping, but
add assertions that would have caught it directly and add a simpler test
that would have triggered the assertions had they existed before.

As drive-bys, add a new `insert` overload to InsertOrderedSet to make
the compiler happy with UNSUBTYPING_DEBUG set and make the whitespace in
another unsubtyping test consistent with the whitespace in the rest of
the file.

Fixes #7773.

Author: tlively

src/ir/subtype-exprs.h

@@ -273,7 +273,15 @@ struct SubtypingDiscoverer : public OverriddenVisitor<SubType> {
   void visitI31Get(I31Get* curr) {
     // This could be |noteNonFlowSubtype| but as there are no subtypes of i31
     // it does not matter.
-    self()->noteSubtype(curr->i31, Type(HeapType::i31, Nullable));
+    // TODO: This should be i31 with a sharedness type variable, but we will
+    // have to refactor this to use principal types to represent that
+    // accurately. For now look at the operand to guess the proper sharedness.
+    auto share = Unshared;
+    if (curr->i31->type.isRef() && curr->i31->type.getHeapType().isShared()) {
+      share = Shared;
+    }
+    self()->noteSubtype(curr->i31,
+                        Type(HeapTypes::i31.getBasic(share), Nullable));
   }
   void visitCallRef(CallRef* curr) {
     // Even if we are unreachable, the target must be valid, and in particular

src/passes/Unsubtyping.cpp

@@ -323,7 +323,6 @@ struct Unsubtyping : Pass {
     if (sub == super || sub.isBottom()) {
       return;
     }
-
     work.push_back({sub, super});
   }
 
@@ -377,6 +376,7 @@ struct Unsubtyping : Pass {
         noteSubtype(sub.getHeapType(), super.getHeapType());
       }
       void noteSubtype(HeapType sub, HeapType super) {
+        assert(HeapType::isSubType(sub, super));
         if (sub == super || sub.isBottom()) {
           return;
         }
@@ -481,6 +481,7 @@ struct Unsubtyping : Pass {
   }
 
   void process(HeapType sub, HeapType super) {
+    assert(HeapType::isSubType(sub, super));
     auto oldSuper = types.getSupertype(sub);
     if (oldSuper) {
       // We already had a recorded supertype. The new supertype might be

src/support/insert_ordered.h

@@ -59,6 +59,12 @@ template<typename T> struct InsertOrderedSet {
     return inserted;
   }
 
+  template<typename It> void insert(It begin, It end) {
+    for (; begin != end; ++begin) {
+      insert(*begin);
+    }
+  }
+
   size_t size() const { return Map.size(); }
   bool empty() const { return Map.empty(); }
 

test/lit/passes/unsubtyping.wast

@@ -1854,21 +1854,41 @@
 ;; Regression test for assertion failure on incorrect updating of type tree
 ;; state.
 (module
-  (rec
-    ;; CHECK:      (rec
-    ;; CHECK-NEXT:  (type $0 (sub (struct)))
-    (type $0 (sub (struct)))
-    ;; CHECK:       (type $1 (sub $0 (struct (field (ref null $0)))))
-    (type $1 (sub $0 (struct (field (ref null $0)))))
-    ;; CHECK:       (type $2 (sub $1 (struct (field (ref null $3)))))
-    (type $2 (sub $1 (struct (field (ref null $3)))))
-    ;; CHECK:       (type $3 (sub $0 (struct)))
-    (type $3 (sub $0 (struct)))
+ (rec
+  ;; CHECK:      (rec
+  ;; CHECK-NEXT:  (type $0 (sub (struct)))
+  (type $0 (sub (struct)))
+  ;; CHECK:       (type $1 (sub $0 (struct (field (ref null $0)))))
+  (type $1 (sub $0 (struct (field (ref null $0)))))
+  ;; CHECK:       (type $2 (sub $1 (struct (field (ref null $3)))))
+  (type $2 (sub $1 (struct (field (ref null $3)))))
+  ;; CHECK:       (type $3 (sub $0 (struct)))
+  (type $3 (sub $0 (struct)))
+ )
+ ;; CHECK:      (global $g (ref struct) (struct.new_default $2))
+ (global $g (ref struct) (struct.new_default $2))
+ ;; CHECK:      (global $g2 (ref null $1) (ref.null none))
+ (global $g2 (ref null $1) (ref.null none))
+ ;; CHECK:      (export "" (global $g2))
+ (export "" (global $g2))
+)
+
+;; Regression for a mishandling of shared i31 with i31.get.
+(module
+ ;; CHECK:      (type $0 (func (result i32)))
+
+ ;; CHECK:      (func $i31-get (type $0) (result i32)
+ ;; CHECK-NEXT:  (i31.get_s
+ ;; CHECK-NEXT:   (ref.i31_shared
+ ;; CHECK-NEXT:    (i32.const 0)
+ ;; CHECK-NEXT:   )
+ ;; CHECK-NEXT:  )
+ ;; CHECK-NEXT: )
+ (func $i31-get (result i32)
+  (i31.get_s
+   (ref.i31_shared
+    (i32.const 0)
    )
-   ;; CHECK:      (global $g (ref struct) (struct.new_default $2))
-   (global $g (ref struct) (struct.new_default $2))
-   ;; CHECK:      (global $g2 (ref null $1) (ref.null none))
-   (global $g2 (ref null $1) (ref.null none))
-   ;; CHECK:      (export "" (global $g2))
-   (export "" (global $g2))
+  )
+ )
 )