Remove open "ignorable public" array types (#6940)

There are a few heap types that are hard-coded to be considered public
and therefore allowed on module boundaries even in --closed-world mode,
specifically to support js-string-builtins. We previously considered
both open and closed (i.e. final) mutable i8 arrays to be public in this
manner, but js-string-builtins only uses the closed versions, so remove
the open versions.

This fixes a particular bug in which Unsubtyping optimized a private
array type to be equivalent to an ignorable public array type,
incorrectly changing the behavior of a cast, but it does not address the
larger problem of optimizations producing types that are equivalent to
public types. Add a TODO about that problem for now.

Fixes #6935.

Author: tlively

src/ir/type-updating.cpp

@@ -175,6 +175,10 @@ GlobalTypeRewriter::TypeMap GlobalTypeRewriter::rebuildTypes(
 #endif
   auto& newTypes = *buildResults;
 
+  // TODO: It is possible that the newly built rec group matches some public rec
+  // group. If that is the case, we need to try a different permutation of the
+  // types or add a brand type to distinguish the private types.
+
   // Map the old types to the new ones.
   TypeMap oldToNewTypes;
   for (auto [type, index] : typeIndices) {

src/wasm/wasm-type.cpp

@@ -2887,17 +2887,9 @@ void TypeBuilder::dump() {
 std::unordered_set<HeapType> getIgnorablePublicTypes() {
   auto array8 = Array(Field(Field::i8, Mutable));
   auto array16 = Array(Field(Field::i16, Mutable));
-  TypeBuilder builder(4);
-  // We handle final and non-final here, but should remove one of them
-  // eventually TODO
+  TypeBuilder builder(2);
   builder[0] = array8;
-  builder[0].setOpen(false);
   builder[1] = array16;
-  builder[1].setOpen(false);
-  builder[2] = array8;
-  builder[2].setOpen(true);
-  builder[3] = array16;
-  builder[3].setOpen(true);
   auto result = builder.build();
   assert(result);
   std::unordered_set<HeapType> ret;

test/lit/passes/unsubtyping.wast

@@ -1747,3 +1747,35 @@
  ;; CHECK:      (global $g (ref $super) (struct.new_default $sub))
  (global $g (ref $super) (struct.new_default $sub))
 )
+
+;; Regression test for a bug where we considered $super to be a public type
+;; (because it was once in contention to appear in js-string-builtin
+;; signatures), so we only updated $sub, but that caused $sub and $super to be
+;; the same type, changing the behavior of the cast.
+(module
+ ;; CHECK:      (type $super (sub (array (mut i8))))
+ (type $super (sub (array (mut i8))))
+ (type $sub (sub $super (array (mut i8))))
+ ;; CHECK:      (type $1 (func))
+
+ ;; CHECK:      (export "test" (func $0))
+
+ ;; CHECK:      (func $0 (type $1)
+ ;; CHECK-NEXT:  (drop
+ ;; CHECK-NEXT:   (ref.cast (ref none)
+ ;; CHECK-NEXT:    (array.new_default $super
+ ;; CHECK-NEXT:     (i32.const 0)
+ ;; CHECK-NEXT:    )
+ ;; CHECK-NEXT:   )
+ ;; CHECK-NEXT:  )
+ ;; CHECK-NEXT: )
+ (func $0 (export "test")
+  (drop
+   (ref.cast (ref $sub)
+    (array.new_default $super
+     (i32.const 0)
+    )
+   )
+  )
+ )
+)