[Strings] Make string a subtype of ext, not any

CHANGELOG.md

@@ -18,6 +18,9 @@ Current Trunk
  - Add an option to preserve imports and exports in the fuzzer (for fuzzer
    harnesses where they only want Binaryen to modify their given testcases, not
    generate new things in them).
+ - `string` is now a subtype of `ext` (rather than `any`). This allows better
+   transformations for strings, like an inverse of StringLowering, but will
+   error on codebases that depend on being able to pass strings into anyrefs.
 
 v122
 ----

src/tools/fuzzing/fuzzing.cpp

@@ -3397,6 +3397,10 @@ Expression* TranslateToFuzzReader::makeBasicRef(Type type) {
   auto share = heapType.getShared();
   switch (heapType.getBasic(Unshared)) {
     case HeapType::ext: {
+      if (wasm.features.hasStrings() && share == Unshared && oneIn(2)) {
+        // Shared strings not yet supported.
+        return makeConst(Type(HeapType::string, NonNullable));
+      }
       auto null = builder.makeRefNull(HeapTypes::ext.getBasic(share));
       // TODO: support actual non-nullable externrefs via imported globals or
       // similar.
@@ -3427,10 +3431,6 @@ Expression* TranslateToFuzzReader::makeBasicRef(Type type) {
         HeapType::i31,
         HeapType::struct_,
         HeapType::array);
-      if (share == Unshared) {
-        // Shared strings not yet supported.
-        subtypeOpts.add(FeatureSet::Strings, HeapType::string);
-      }
       auto subtype = pick(subtypeOpts).getBasic(share);
       return makeConst(Type(subtype, nullability));
     }
@@ -5342,11 +5342,16 @@ HeapType TranslateToFuzzReader::getSubType(HeapType type) {
           .getBasic(share);
       case HeapType::cont:
         return pick(HeapTypes::cont, HeapTypes::nocont).getBasic(share);
-      case HeapType::ext:
-        return pick(FeatureOptions<HeapType>()
-                      .add(FeatureSet::ReferenceTypes, HeapType::ext)
-                      .add(FeatureSet::GC, HeapType::noext))
-          .getBasic(share);
+      case HeapType::ext: {
+        auto options = FeatureOptions<HeapType>()
+                       .add(FeatureSet::ReferenceTypes, HeapType::ext)
+                       .add(FeatureSet::GC, HeapType::noext);
+        if (share == Unshared) {
+          // Shared strings not yet supported.
+          options.add(FeatureSet::Strings, HeapType::string);
+        }
+        return pick(options).getBasic(share);
+      }
       case HeapType::any: {
         assert(wasm.features.hasReferenceTypes());
         assert(wasm.features.hasGC());
@@ -5357,10 +5362,6 @@ HeapType TranslateToFuzzReader::getSubType(HeapType type) {
                                                       HeapType::struct_,
                                                       HeapType::array,
                                                       HeapType::none);
-        if (share == Unshared) {
-          // Shared strings not yet supported.
-          options.add(FeatureSet::Strings, HeapType::string);
-        }
         return pick(options).getBasic(share);
       }
       case HeapType::eq:

src/tools/fuzzing/heap-types.cpp

@@ -510,7 +510,7 @@ struct HeapTypeGeneratorImpl {
         candidates.push_back(HeapTypes::any.getBasic(share));
         break;
       case HeapType::string:
-        candidates.push_back(HeapTypes::any.getBasic(share));
+        candidates.push_back(HeapTypes::ext.getBasic(share));
         break;
       case HeapType::none:
         return pickSubAny(share);

src/wasm/wasm-type.cpp

@@ -403,6 +403,11 @@ std::optional<HeapType> getBasicHeapTypeLUB(HeapType::BasicHeapType a,
   HeapType lubUnshared;
   switch (HeapType(a).getBasic(Unshared)) {
     case HeapType::ext:
+      if (bUnshared != HeapType::string) {
+        return std::nullopt;
+      }
+      lubUnshared = HeapType::ext;
+      break;
     case HeapType::func:
     case HeapType::cont:
     case HeapType::exn:
@@ -433,15 +438,17 @@ std::optional<HeapType> getBasicHeapTypeLUB(HeapType::BasicHeapType a,
       }
       break;
     case HeapType::array:
-    case HeapType::string:
       lubUnshared = HeapType::any;
       break;
+    case HeapType::string:
     case HeapType::none:
     case HeapType::noext:
     case HeapType::nofunc:
     case HeapType::nocont:
     case HeapType::noexn:
-      // Bottom types already handled.
+      // Bottom types already handled (including string as the switch value,
+      // which implies the other value is bottom, which again would have been
+      // handled).
       WASM_UNREACHABLE("unexpected basic type");
   }
   auto share = HeapType(a).getShared();
@@ -949,8 +956,9 @@ std::optional<HeapType> HeapType::getSuperType() const {
       case none:
       case exn:
       case noexn:
-      case string:
         return {};
+      case string:
+        return HeapType(ext).getBasic(share);
       case eq:
         return HeapType(any).getBasic(share);
       case i31:
@@ -997,12 +1005,12 @@ size_t HeapType::getDepth() const {
         case HeapType::exn:
           break;
         case HeapType::eq:
+        case HeapType::string:
           depth++;
           break;
         case HeapType::i31:
         case HeapType::struct_:
         case HeapType::array:
-        case HeapType::string:
           depth += 2;
           break;
         case HeapType::none:
@@ -1046,9 +1054,9 @@ HeapType::BasicHeapType HeapType::getUnsharedBottom() const {
       case i31:
       case struct_:
       case array:
-      case string:
       case none:
         return none;
+      case string:
       case noext:
         return noext;
       case nofunc:
@@ -1496,8 +1504,9 @@ bool SubTyper::isSubType(HeapType a, HeapType b) {
                aUnshared == HeapType::struct_ || aUnshared == HeapType::array ||
                a.isStruct() || a.isArray();
       case HeapType::i31:
-      case HeapType::string:
         return aUnshared == HeapType::none;
+      case HeapType::string:
+        return aUnshared == HeapType::noext;
       case HeapType::struct_:
         return aUnshared == HeapType::none || a.isStruct();
       case HeapType::array:

test/gtest/type-builder.cpp

@@ -655,7 +655,7 @@ TEST_F(TypeTest, TestHeapTypeRelations) {
   assertLUB(ext, i31, {});
   assertLUB(ext, struct_, {});
   assertLUB(ext, array, {});
-  assertLUB(ext, string, {});
+  assertLUB(ext, string, ext);
   assertLUB(ext, none, {});
   assertLUB(ext, noext, ext);
   assertLUB(ext, nofunc, {});
@@ -728,7 +728,7 @@ TEST_F(TypeTest, TestHeapTypeRelations) {
   assertLUB(any, i31, any);
   assertLUB(any, struct_, any);
   assertLUB(any, array, any);
-  assertLUB(any, string, any);
+  assertLUB(any, string, {});
   assertLUB(any, none, any);
   assertLUB(any, noext, {});
   assertLUB(any, nofunc, {});
@@ -751,7 +751,7 @@ TEST_F(TypeTest, TestHeapTypeRelations) {
   assertLUB(eq, i31, eq);
   assertLUB(eq, struct_, eq);
   assertLUB(eq, array, eq);
-  assertLUB(eq, string, any);
+  assertLUB(eq, string, {});
   assertLUB(eq, none, eq);
   assertLUB(eq, noext, {});
   assertLUB(eq, nofunc, {});
@@ -773,7 +773,7 @@ TEST_F(TypeTest, TestHeapTypeRelations) {
   assertLUB(i31, cont, {});
   assertLUB(i31, struct_, eq);
   assertLUB(i31, array, eq);
-  assertLUB(i31, string, any);
+  assertLUB(i31, string, {});
   assertLUB(i31, none, i31);
   assertLUB(i31, noext, {});
   assertLUB(i31, nofunc, {});
@@ -794,7 +794,7 @@ TEST_F(TypeTest, TestHeapTypeRelations) {
   assertLUB(struct_, struct_, struct_);
   assertLUB(struct_, cont, {});
   assertLUB(struct_, array, eq);
-  assertLUB(struct_, string, any);
+  assertLUB(struct_, string, {});
   assertLUB(struct_, none, struct_);
   assertLUB(struct_, noext, {});
   assertLUB(struct_, nofunc, {});
@@ -814,7 +814,7 @@ TEST_F(TypeTest, TestHeapTypeRelations) {
 
   assertLUB(array, array, array);
   assertLUB(array, cont, {});
-  assertLUB(array, string, any);
+  assertLUB(array, string, {});
   assertLUB(array, none, array);
   assertLUB(array, noext, {});
   assertLUB(array, nofunc, {});
@@ -834,14 +834,14 @@ TEST_F(TypeTest, TestHeapTypeRelations) {
 
   assertLUB(string, string, string);
   assertLUB(string, cont, {});
-  assertLUB(string, none, string);
-  assertLUB(string, noext, {});
+  assertLUB(string, none, {});
+  assertLUB(string, noext, string);
   assertLUB(string, nofunc, {});
   assertLUB(string, nocont, {});
   assertLUB(string, defFunc, {});
   assertLUB(string, defCont, {});
-  assertLUB(string, defStruct, any);
-  assertLUB(string, defArray, any);
+  assertLUB(string, defStruct, {});
+  assertLUB(string, defArray, {});
   assertLUB(string, sharedAny, {});
   assertLUB(string, sharedEq, {});
   assertLUB(string, sharedI31, {});
@@ -1599,7 +1599,7 @@ TEST_F(TypeTest, TestDepth) {
   EXPECT_EQ(HeapTypes::ext.getDepth(), 0U);
 
   EXPECT_EQ(HeapTypes::i31.getDepth(), 2U);
-  EXPECT_EQ(HeapTypes::string.getDepth(), 2U);
+  EXPECT_EQ(HeapTypes::string.getDepth(), 1U);
 
   EXPECT_EQ(HeapTypes::none.getDepth(), size_t(-1));
   EXPECT_EQ(HeapTypes::nofunc.getDepth(), size_t(-1));
@@ -1680,7 +1680,7 @@ TEST_F(TypeTest, TestSupertypes) {
   ASSERT_EQ(HeapTypes::i31.getSuperType(), HeapType::eq);
   ASSERT_EQ(HeapTypes::struct_.getSuperType(), HeapType::eq);
   ASSERT_EQ(HeapTypes::array.getSuperType(), HeapType::eq);
-  ASSERT_FALSE(HeapTypes::string.getSuperType());
+  ASSERT_EQ(HeapTypes::string.getSuperType(), HeapType::ext);
   ASSERT_FALSE(HeapTypes::none.getSuperType());
   ASSERT_FALSE(HeapTypes::noext.getSuperType());
   ASSERT_FALSE(HeapTypes::nofunc.getSuperType());

test/lit/ctor-eval/string.wast

@@ -9,14 +9,14 @@
 
  (export "test" (func $test))
 
- (func $test (result anyref)
+ (func $test (result externref)
   (global.get $global)
  )
 )
-;; CHECK:      (type $0 (func (result anyref)))
+;; CHECK:      (type $0 (func (result externref)))
 
 ;; CHECK:      (export "test" (func $test_1))
 
-;; CHECK:      (func $test_1 (type $0) (result anyref)
+;; CHECK:      (func $test_1 (type $0) (result externref)
 ;; CHECK-NEXT:  (string.const "one")
 ;; CHECK-NEXT: )

test/lit/exec/strings.wast

@@ -476,24 +476,6 @@
       (string.const "five!")
     )
   )
-
-  ;; CHECK:      [fuzz-exec] calling extern
-  ;; CHECK-NEXT: [fuzz-exec] note result: extern => string("string")
-  (func $extern (export "extern") (result externref)
-    (extern.convert_any
-      (string.const "string")
-    )
-  )
-
-  ;; CHECK:      [fuzz-exec] calling extern-intern
-  ;; CHECK-NEXT: [fuzz-exec] note result: extern-intern => string("string")
-  (func $extern-intern (export "extern-intern") (result anyref)
-    (any.convert_extern
-      (extern.convert_any
-        (string.const "string")
-      )
-    )
-  )
 )
 ;; CHECK:      [fuzz-exec] calling new_wtf16_array
 ;; CHECK-NEXT: [fuzz-exec] note result: new_wtf16_array => string("ello")
@@ -623,11 +605,6 @@
 ;; CHECK:      [fuzz-exec] calling string.measure
 ;; CHECK-NEXT: [fuzz-exec] note result: string.measure => 5
 
-;; CHECK:      [fuzz-exec] calling extern
-;; CHECK-NEXT: [fuzz-exec] note result: extern => string("string")
-
-;; CHECK:      [fuzz-exec] calling extern-intern
-;; CHECK-NEXT: [fuzz-exec] note result: extern-intern => string("string")
 ;; CHECK-NEXT: [fuzz-exec] comparing compare.1
 ;; CHECK-NEXT: [fuzz-exec] comparing compare.10
 ;; CHECK-NEXT: [fuzz-exec] comparing compare.2
@@ -648,8 +625,6 @@
 ;; CHECK-NEXT: [fuzz-exec] comparing eq.3
 ;; CHECK-NEXT: [fuzz-exec] comparing eq.4
 ;; CHECK-NEXT: [fuzz-exec] comparing eq.5
-;; CHECK-NEXT: [fuzz-exec] comparing extern
-;; CHECK-NEXT: [fuzz-exec] comparing extern-intern
 ;; CHECK-NEXT: [fuzz-exec] comparing get_codeunit
 ;; CHECK-NEXT: [fuzz-exec] comparing invalid_code_point
 ;; CHECK-NEXT: [fuzz-exec] comparing isolated_high_code_point

test/lit/passes/precompute-strings.wast

@@ -12,7 +12,7 @@
 
  ;; CHECK:      (type $2 (func (result (ref string))))
 
- ;; CHECK:      (type $3 (func (result anyref)))
+ ;; CHECK:      (type $3 (func (result externref)))
 
  ;; CHECK:      (type $4 (func (result (ref any))))
 
@@ -262,7 +262,7 @@
  )
 
 
- ;; CHECK:      (func $string.new-mutable (type $3) (result anyref)
+ ;; CHECK:      (func $string.new-mutable (type $3) (result externref)
  ;; CHECK-NEXT:  (string.new_wtf16_array
  ;; CHECK-NEXT:   (array.new_fixed $array16 4
  ;; CHECK-NEXT:    (i32.const 65)
@@ -274,7 +274,7 @@
  ;; CHECK-NEXT:   (i32.const 4)
  ;; CHECK-NEXT:  )
  ;; CHECK-NEXT: )
- (func $string.new-mutable (result anyref)
+ (func $string.new-mutable (result externref)
   ;; We do not precompute this because the array is mutable, and we do not yet
   ;; do an analysis to see that it does not "escape" into places that modify it.
   (string.new_wtf16_array
@@ -289,10 +289,10 @@
   )
  )
 
- ;; CHECK:      (func $string.new-immutable (type $3) (result anyref)
+ ;; CHECK:      (func $string.new-immutable (type $3) (result externref)
  ;; CHECK-NEXT:  (string.const "ABCD")
  ;; CHECK-NEXT: )
- (func $string.new-immutable (result anyref)
+ (func $string.new-immutable (result externref)
   ;; This array is immutable and we can optimize here.
   (string.new_wtf16_array
    (array.new_fixed $array16-imm 4

test/lit/passes/string-lowering-instructions.wast

@@ -236,7 +236,8 @@
     (if (result stringref)
       (i32.const 0)
       (then
-        (ref.null none) ;; this will turn into noextern
+        (ref.null noextern) ;; The change from stringref to externref does not
+                            ;; cause problems here, nothing needs to change.
       )
       (else
         (local.get $ref)
@@ -263,7 +264,7 @@
         (local.get $ref)
       )
       (else
-        (ref.null none)
+        (ref.null noextern)
       )
     )
   )
@@ -378,10 +379,13 @@
   ;; CHECK-NEXT:  )
   ;; CHECK-NEXT: )
   (func $struct-of-string
-    ;; Test lowering of struct fields from stringref to externref.
+    ;; Test lowering of struct fields from stringref to externref. This was more
+    ;; useful of a test when stringref was a subtype of anyref, but it is still
+    ;; useful to verify nothing here goes wrong. (Now we convert stringref to
+    ;; externref, a supertype, which is much simpler - same bottom type, etc.)
     (drop
       (struct.new $struct-of-string
-        (ref.null none) ;; This null must be fixed to be ext.
+        (ref.null noextern) ;; This null is already of the right type.
         (i32.const 10)
         (ref.null none) ;; Nothing to do here (field remains anyref).
       )