[NFC] Use SubtypingDiscoverer in StringLowering (#6325)

This replaces horrible hacks to find which nulls need to switch (from none to
noext) with general code using SubtypingDiscoverer. That helper is aware of
where each expression is written, so we can find those nulls trivially.

This is NFC on existing usage but should fix any remaining bugs with null
constants.

1 files changed, 49 insertions, 49 deletions

diff --git a/src/passes/StringLowering.cpp b/src/passes/StringLowering.cpp
index c70691ea1..c04695c50 100644
--- a/src/passes/StringLowering.cpp
+++ b/src/passes/StringLowering.cpp
@@ -34,6 +34,7 @@
 
 #include "ir/module-utils.h"
 #include "ir/names.h"
+#include "ir/subtype-exprs.h"
 #include "ir/type-updating.h"
 #include "ir/utils.h"
 #include "pass.h"
@@ -199,6 +200,12 @@ struct StringLowering : public StringGathering {
     // Replace string.* etc. operations with imported ones.
     replaceInstructions(module);
 
+    // Replace ref.null types as needed.
+    replaceNulls(module);
+
+    // ReFinalize to apply all the above changes.
+    ReFinalize().run(getPassRunner(), module);
+
     // Disable the feature here after we lowered everything away.
     module->features.disable(FeatureSet::Strings);
   }
@@ -433,65 +440,58 @@ struct StringLowering : public StringGathering {
             WASM_UNREACHABLE("TODO: all string.slice*");
         }
       }
+    };
 
-      // Additional hacks: We fix up a none that should be noext. Before the
-      // lowering we can use none for stringref, but after we must use noext as
-      // the two do not share a bottom type.
-      //
-      // The code here and in the visitors below is course wildly insufficient
-      // (we need selects and blocks and all other joins, and not just nulls,
-      // etc.) but in practice this is enough for now. TODO extend as needed
-      void ensureNullIsExt(Expression* curr) {
-        if (auto* null = curr->dynCast<RefNull>()) {
-          null->finalize(HeapType::noext);
-        }
-      }
+    Replacer replacer(*this);
+    replacer.run(getPassRunner(), module);
+    replacer.walkModuleCode(module);
+  }
 
-      bool isExt(Type type) {
-        return type.isRef() && type.getHeapType() == HeapType::ext;
+  // A ref.null of none needs to be noext if it is going to a location of type
+  // stringref.
+  void replaceNulls(Module* module) {
+    // Use SubtypingDiscoverer to find when a ref.null of none flows into a
+    // place that has been changed from stringref to externref.
+    struct NullFixer
+      : public WalkerPass<
+          ControlFlowWalker<NullFixer, SubtypingDiscoverer<NullFixer>>> {
+      // Hooks for SubtypingDiscoverer.
+      void noteSubtype(Type, Type) {
+        // Nothing to do for pure types.
       }
-
-      void visitIf(If* curr) {
-        // If the if outputs an ext, fix up the arms to contain proper nulls for
-        // that type.
-        if (isExt(curr->type)) {
-          ensureNullIsExt(curr->ifTrue);
-          ensureNullIsExt(curr->ifFalse);
-        }
+      void noteSubtype(HeapType, HeapType) {
+        // Nothing to do for pure types.
       }
-
-      void visitCall(Call* curr) {
-        auto targetSig =
-          getModule()->getFunction(curr->target)->type.getSignature();
-        for (Index i = 0; i < curr->operands.size(); i++) {
-          if (isExt(targetSig.params[i])) {
-            ensureNullIsExt(curr->operands[i]);
-          }
-        }
+      void noteSubtype(Type, Expression*) {
+        // Nothing to do for a subtype of an expression.
       }
-
-      void visitStructNew(StructNew* curr) {
-        if (curr->type == Type::unreachable || curr->operands.empty()) {
-          return;
-        }
-
-        // If we write a none into an ext field, fix that.
-        auto& fields = curr->type.getHeapType().getStruct().fields;
-        assert(curr->operands.size() == fields.size());
-        for (Index i = 0; i < fields.size(); i++) {
-          if (isExt(fields[i].type)) {
-            ensureNullIsExt(curr->operands[i]);
+      void noteSubtype(Expression* a, Type b) {
+        // This is the case we care about: if |a| is a null that must be a
+        // subtype of ext then we fix that up.
+        if (b.isRef() && b.getHeapType().getTop() == HeapType::ext) {
+          if (auto* null = a->dynCast<RefNull>()) {
+            null->finalize(HeapType::noext);
           }
         }
       }
+      void noteSubtype(Expression* a, Expression* b) {
+        // Only the type matters of the place we assign to.
+        noteSubtype(a, b->type);
+      }
+      void noteCast(HeapType, HeapType) {
+        // Casts do not concern us.
+      }
+      void noteCast(Expression*, Type) {
+        // Casts do not concern us.
+      }
+      void noteCast(Expression*, Expression*) {
+        // Casts do not concern us.
+      }
     };
 
-    Replacer replacer(*this);
-    replacer.run(getPassRunner(), module);
-    replacer.walkModuleCode(module);
-
-    // ReFinalize to apply changes to parents.
-    ReFinalize().run(getPassRunner(), module);
+    NullFixer fixer;
+    fixer.run(getPassRunner(), module);
+    fixer.walkModuleCode(module);
   }
 };