[NFC] Add some comments about flow in SubtypingDiscoverer and Unsubtyping (#6359)

I audited all of SubtypingDiscoverer for flow/non-flow constraints and added
some comments to clarify things for our future selves if we ever need to
generalize it.

2 files changed, 18 insertions, 0 deletions

diff --git a/src/ir/subtype-exprs.h b/src/ir/subtype-exprs.h
index 1c922521f..ab0ace53d 100644
--- a/src/ir/subtype-exprs.h
+++ b/src/ir/subtype-exprs.h
@@ -161,6 +161,8 @@ struct SubtypingDiscoverer : public OverriddenVisitor<SubType> {
       // sources, call_indirect target types may be supertypes of their source
       // table types. In this case, the cast will always succeed, but only if we
       // keep the types related.
+      // TODO: No value flows here, so we could use |noteNonFlowSubtype|, but
+      //       this is a trivial situation that is not worth optimizing.
       self()->noteSubtype(tableType, curr->heapType);
     } else if (HeapType::isSubType(curr->heapType, tableType)) {
       self()->noteCast(tableType, curr->heapType);
@@ -256,6 +258,8 @@ struct SubtypingDiscoverer : public OverriddenVisitor<SubType> {
   void visitTupleExtract(TupleExtract* curr) {}
   void visitRefI31(RefI31* curr) {}
   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));
   }
   void visitCallRef(CallRef* curr) {
@@ -271,6 +275,9 @@ struct SubtypingDiscoverer : public OverriddenVisitor<SubType> {
     // generalize, should they generalize the target more or the parameters
     // more? etc.), so we do the simple thing here for now of requiring the
     // target type not generalize.
+    //
+    // Note that this could be |noteNonFlowSubtype| but since we are comparing
+    // a type to itself here, that does not matter.
     self()->noteSubtype(curr->target, curr->target->type);
 
     if (curr->target->type.isSignature()) {
diff --git a/src/passes/Unsubtyping.cpp b/src/passes/Unsubtyping.cpp
index 897511c35..ccad4ed3d 100644
--- a/src/passes/Unsubtyping.cpp
+++ b/src/passes/Unsubtyping.cpp
@@ -201,6 +201,17 @@ struct Unsubtyping
     // basic type then we can simply ignore this: we only remove subtyping
     // between user types, so subtyping wrt basic types is unchanged, and so
     // this constraint will never be a problem.
+    //
+    // This is sort of a hack because in general to be precise we should not
+    // just consider basic types here - in general, we should note for each
+    // constraint whether it is a flow-based one or not, and only take the
+    // flow-based ones into account when looking at the impact of casts.
+    // However, in practice this is enough as the only non-trivial case of
+    // |noteNonFlowSubtype| is for RefEq, which uses a basic type (eqref). Other
+    // cases of non-flow subtyping end up trivial, e.g., the target of a
+    // CallRef is compared to itself (and we ignore constraints of A :> A).
+    // However, if we change how |noteNonFlowSubtype| is used in
+    // SubtypingDiscoverer then we may need to generalize this.
     if (super.isRef() && super.getHeapType().isBasic()) {
       return;
     }