[VM/nnbd] Consider nullability of type parameters in function type tests.

This also fixes issue #40259.

Change-Id: I2e603e927e98270bb24b726444490993e6360f97
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/134572
Commit-Queue: Régis Crelier <regis@google.com>
Reviewed-by: Liam Appelbe <liama@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>

runtime/lib/object.cc

@@ -110,7 +110,7 @@ DEFINE_NATIVE_ENTRY(Object_haveSameRuntimeType, 0, 2) {
     const AbstractType& right_type =
         AbstractType::Handle(right.GetType(Heap::kNew));
     return Bool::Get(
-               left_type.IsEquivalent(right_type, /* syntactically = */ true))
+               left_type.IsEquivalent(right_type, TypeEquality::kSyntactical))
         .raw();
   }
 
@@ -129,7 +129,7 @@ DEFINE_NATIVE_ENTRY(Object_haveSameRuntimeType, 0, 2) {
   const intptr_t num_type_params = cls.NumTypeParameters();
   return Bool::Get(left_type_arguments.IsSubvectorEquivalent(
                        right_type_arguments, num_type_args - num_type_params,
-                       num_type_params, /* syntactically = */ true))
+                       num_type_params, TypeEquality::kSyntactical))
       .raw();
 }
 
@@ -205,7 +205,7 @@ DEFINE_NATIVE_ENTRY(Type_equality, 0, 2) {
   if (type.raw() == other.raw()) {
     return Bool::True().raw();
   }
-  return Bool::Get(type.IsEquivalent(other, /* syntactically = */ true)).raw();
+  return Bool::Get(type.IsEquivalent(other, TypeEquality::kSyntactical)).raw();
 }
 
 DEFINE_NATIVE_ENTRY(Internal_inquireIs64Bit, 0, 0) {

runtime/vm/class_finalizer.cc

@@ -378,9 +378,11 @@ void ClassFinalizer::CheckRecursiveType(const Class& cls,
     if ((pending_type.raw() != type.raw()) && pending_type.IsType() &&
         (pending_type.type_class() == type_cls.raw())) {
       pending_arguments = pending_type.arguments();
+      // By using TypeEquality::kIgnoreNullability, we throw a wider net and
+      // may reject more problematic declarations.
       if (!pending_arguments.IsSubvectorEquivalent(
               arguments, first_type_param, num_type_params,
-              /* syntactically = */ true) &&
+              TypeEquality::kIgnoreNullability) &&
           !pending_arguments.IsSubvectorInstantiated(first_type_param,
                                                      num_type_params)) {
         const TypeArguments& instantiated_arguments = TypeArguments::Handle(
@@ -394,9 +396,11 @@ void ClassFinalizer::CheckRecursiveType(const Class& cls,
                           NNBDMode::kLegacyLib, Object::null_type_arguments(),
                           Object::null_type_arguments(), kNoneFree, NULL,
                           Heap::kNew));
+        // By using TypeEquality::kIgnoreNullability, we throw a wider net and
+        // may reject more problematic declarations.
         if (!instantiated_pending_arguments.IsSubvectorEquivalent(
                 instantiated_arguments, first_type_param, num_type_params,
-                /* syntactically = */ true)) {
+                TypeEquality::kIgnoreNullability)) {
           const String& type_name = String::Handle(zone, type.Name());
           ReportError(cls, type.token_pos(), "illegal recursive type '%s'",
                       type_name.ToCString());

runtime/vm/compiler/backend/il.cc

@@ -387,7 +387,9 @@ bool HierarchyInfo::CanUseSubtypeRangeCheckFor(const AbstractType& type) {
     // arguments are not "dynamic" but instantiated-to-bounds.
     const Type& rare_type =
         Type::Handle(zone, Type::RawCast(type_class.RareType()));
-    if (!rare_type.IsEquivalent(type, /* syntactically = */ true)) {
+    // TODO(regis): Revisit the usage of TypeEquality::kSyntactical when
+    // implementing strong mode.
+    if (!rare_type.IsEquivalent(type, TypeEquality::kSyntactical)) {
       return false;
     }
   }

runtime/vm/compiler/frontend/flow_graph_builder.cc

@@ -380,7 +380,9 @@ bool SimpleInstanceOfType(const AbstractType& type) {
     // type arguments, then we can still use the _simpleInstanceOf
     // implementation (see also runtime/lib/object.cc:Object_SimpleInstanceOf).
     const auto& rare_type = AbstractType::Handle(type_class.RareType());
-    return rare_type.IsEquivalent(type, /* syntactically = */ true);
+    // TODO(regis): Revisit the usage of TypeEquality::kSyntactical when
+    // implementing strong mode.
+    return rare_type.IsEquivalent(type, TypeEquality::kSyntactical);
   }
 
   // Finally a simple class for instance of checking.

runtime/vm/object.cc

@@ -5625,7 +5625,7 @@ void TypeArguments::PrintSubvectorName(intptr_t from_index,
 bool TypeArguments::IsSubvectorEquivalent(const TypeArguments& other,
                                           intptr_t from_index,
                                           intptr_t len,
-                                          bool syntactically,
+                                          TypeEquality kind,
                                           TrailPtr trail) const {
   if (this->raw() == other.raw()) {
     return true;
@@ -5643,7 +5643,7 @@ bool TypeArguments::IsSubvectorEquivalent(const TypeArguments& other,
     type = TypeAt(i);
     other_type = other.TypeAt(i);
     // Still unfinalized vectors should not be considered equivalent.
-    if (type.IsNull() || !type.IsEquivalent(other_type, syntactically, trail)) {
+    if (type.IsNull() || !type.IsEquivalent(other_type, kind, trail)) {
       return false;
     }
   }
@@ -7721,7 +7721,8 @@ bool Function::IsContravariantParameter(NNBDMode mode,
   return other_param_type.IsSubtypeOf(mode, param_type, space);
 }
 
-bool Function::HasSameTypeParametersAndBounds(const Function& other) const {
+bool Function::HasSameTypeParametersAndBounds(const Function& other,
+                                              TypeEquality kind) const {
   Thread* thread = Thread::Current();
   Zone* zone = thread->zone();
 
@@ -7747,10 +7748,7 @@ bool Function::HasSameTypeParametersAndBounds(const Function& other) const {
       ASSERT(bound.IsFinalized());
       other_bound = other_type_param.bound();
       ASSERT(other_bound.IsFinalized());
-      // TODO(dartbug.com/40259): Treat top types as equivalent and disregard
-      // nullability in weak mode.
-      const bool syntactically = !FLAG_strong_non_nullable_type_checks;
-      if (!bound.IsEquivalent(other_bound, syntactically)) {
+      if (!bound.IsEquivalent(other_bound, kind)) {
         return false;
       }
     }
@@ -7785,7 +7783,8 @@ bool Function::IsSubtypeOf(NNBDMode mode,
     return false;
   }
   // Check the type parameters and bounds of generic functions.
-  if (!HasSameTypeParametersAndBounds(other)) {
+  if (!HasSameTypeParametersAndBounds(other,
+                                      TypeEquality::kSubtypeNullability)) {
     return false;
   }
   Thread* thread = Thread::Current();
@@ -17982,7 +17981,7 @@ void AbstractType::SetIsBeingFinalized() const {
 }
 
 bool AbstractType::IsEquivalent(const Instance& other,
-                                bool syntactically,
+                                TypeEquality kind,
                                 TrailPtr trail) const {
   // AbstractType is an abstract class.
   UNREACHABLE();
@@ -18422,7 +18421,8 @@ bool AbstractType::IsSubtypeOf(NNBDMode mode,
     const TypeParameter& type_param = TypeParameter::Cast(*this);
     if (other.IsTypeParameter()) {
       const TypeParameter& other_type_param = TypeParameter::Cast(other);
-      if (type_param.Equals(other_type_param)) {
+      if (type_param.IsEquivalent(other_type_param,
+                                  TypeEquality::kSubtypeNullability)) {
         return true;
       }
       if (type_param.IsFunctionTypeParameter() &&
@@ -18441,6 +18441,10 @@ bool AbstractType::IsSubtypeOf(NNBDMode mode,
         const int other_offset = other_sig_fun.NumParentTypeParameters();
         if (type_param.index() - offset ==
             other_type_param.index() - other_offset) {
+          if (FLAG_strong_non_nullable_type_checks && type_param.IsNullable() &&
+              other_type_param.IsNonNullable()) {
+            return false;
+          }
           return true;
         }
       }
@@ -18843,7 +18847,7 @@ RawAbstractType* Type::InstantiateFrom(
 }
 
 bool Type::IsEquivalent(const Instance& other,
-                        bool syntactically,
+                        TypeEquality kind,
                         TrailPtr trail) const {
   ASSERT(!IsNull());
   if (raw() == other.raw()) {
@@ -18854,7 +18858,7 @@ bool Type::IsEquivalent(const Instance& other,
     const AbstractType& other_ref_type =
         AbstractType::Handle(TypeRef::Cast(other).type());
     ASSERT(!other_ref_type.IsTypeRef());
-    return IsEquivalent(other_ref_type, syntactically, trail);
+    return IsEquivalent(other_ref_type, kind, trail);
   }
   if (!other.IsType()) {
     return false;
@@ -18866,18 +18870,30 @@ bool Type::IsEquivalent(const Instance& other,
   if (type_class_id() != other_type.type_class_id()) {
     return false;
   }
-  Nullability this_type_nullability = nullability();
-  Nullability other_type_nullability = other_type.nullability();
-  if (syntactically) {
-    if (this_type_nullability == Nullability::kLegacy) {
-      this_type_nullability = Nullability::kNonNullable;
+  if (kind != TypeEquality::kIgnoreNullability) {
+    Nullability this_type_nullability = nullability();
+    Nullability other_type_nullability = other_type.nullability();
+    if (kind == TypeEquality::kSubtypeNullability) {
+      if (FLAG_strong_non_nullable_type_checks &&
+          this_type_nullability == Nullability::kNullable &&
+          other_type_nullability == Nullability::kNonNullable) {
+        return false;
+      }
+    } else {
+      if (kind == TypeEquality::kSyntactical) {
+        if (this_type_nullability == Nullability::kLegacy) {
+          this_type_nullability = Nullability::kNonNullable;
+        }
+        if (other_type_nullability == Nullability::kLegacy) {
+          other_type_nullability = Nullability::kNonNullable;
+        }
+      } else {
+        ASSERT(kind == TypeEquality::kCanonical);
+      }
+      if (this_type_nullability != other_type_nullability) {
+        return false;
+      }
     }
-    if (other_type_nullability == Nullability::kLegacy) {
-      other_type_nullability = Nullability::kNonNullable;
-    }
-  }
-  if (this_type_nullability != other_type_nullability) {
-    return false;
   }
   if (!IsFinalized() || !other_type.IsFinalized()) {
     return false;  // Too early to decide if equal.
@@ -18909,8 +18925,8 @@ bool Type::IsEquivalent(const Instance& other,
           return false;
         }
       } else if (!type_args.IsSubvectorEquivalent(other_type_args, from_index,
-                                                  num_type_params,
-                                                  syntactically, trail)) {
+                                                  num_type_params, kind,
+                                                  trail)) {
         return false;
       }
 #ifdef DEBUG
@@ -18926,7 +18942,7 @@ bool Type::IsEquivalent(const Instance& other,
         for (intptr_t i = 0; i < from_index; i++) {
           type_arg = type_args.TypeAt(i);
           other_type_arg = other_type_args.TypeAt(i);
-          ASSERT(type_arg.IsEquivalent(other_type_arg, syntactically, trail));
+          ASSERT(type_arg.IsEquivalent(other_type_arg, kind, trail));
         }
       }
 #endif
@@ -18947,7 +18963,7 @@ bool Type::IsEquivalent(const Instance& other,
 
   // Compare function type parameters and their bounds.
   // Check the type parameters and bounds of generic functions.
-  if (!sig_fun.HasSameTypeParametersAndBounds(other_sig_fun)) {
+  if (!sig_fun.HasSameTypeParametersAndBounds(other_sig_fun, kind)) {
     return false;
   }
 
@@ -18989,7 +19005,7 @@ bool Type::IsEquivalent(const Instance& other,
   for (intptr_t i = 0; i < num_params; i++) {
     param_type = sig_fun.ParameterTypeAt(i);
     other_param_type = other_sig_fun.ParameterTypeAt(i);
-    if (!param_type.IsEquivalent(other_param_type, syntactically)) {
+    if (!param_type.IsEquivalent(other_param_type, kind)) {
       return false;
     }
   }
@@ -19354,7 +19370,7 @@ bool TypeRef::IsInstantiated(Genericity genericity,
 }
 
 bool TypeRef::IsEquivalent(const Instance& other,
-                           bool syntactically,
+                           TypeEquality kind,
                            TrailPtr trail) const {
   if (raw() == other.raw()) {
     return true;
@@ -19366,8 +19382,7 @@ bool TypeRef::IsEquivalent(const Instance& other,
     return true;
   }
   const AbstractType& ref_type = AbstractType::Handle(type());
-  return !ref_type.IsNull() &&
-         ref_type.IsEquivalent(other, syntactically, trail);
+  return !ref_type.IsNull() && ref_type.IsEquivalent(other, kind, trail);
 }
 
 RawTypeRef* TypeRef::InstantiateFrom(
@@ -19535,7 +19550,7 @@ bool TypeParameter::IsInstantiated(Genericity genericity,
 }
 
 bool TypeParameter::IsEquivalent(const Instance& other,
-                                 bool syntactically,
+                                 TypeEquality kind,
                                  TrailPtr trail) const {
   if (raw() == other.raw()) {
     return true;
@@ -19545,7 +19560,7 @@ bool TypeParameter::IsEquivalent(const Instance& other,
     const AbstractType& other_ref_type =
         AbstractType::Handle(TypeRef::Cast(other).type());
     ASSERT(!other_ref_type.IsTypeRef());
-    return IsEquivalent(other_ref_type, syntactically, trail);
+    return IsEquivalent(other_ref_type, kind, trail);
   }
   if (!other.IsTypeParameter()) {
     return false;
@@ -19558,18 +19573,32 @@ bool TypeParameter::IsEquivalent(const Instance& other,
   if (parameterized_function() != other_type_param.parameterized_function()) {
     return false;
   }
-  Nullability this_type_param_nullability = nullability();
-  Nullability other_type_param_nullability = other_type_param.nullability();
-  if (syntactically) {
-    if (this_type_param_nullability == Nullability::kLegacy) {
-      this_type_param_nullability = Nullability::kNonNullable;
+  if (kind != TypeEquality::kIgnoreNullability) {
+    Nullability this_type_param_nullability = nullability();
+    Nullability other_type_param_nullability = other_type_param.nullability();
+    if (kind == TypeEquality::kSubtypeNullability) {
+      if (FLAG_strong_non_nullable_type_checks &&
+          this_type_param_nullability == Nullability::kNullable &&
+          other_type_param_nullability == Nullability::kNonNullable) {
+        return false;
+      }
+    } else {
+      if (kind == TypeEquality::kSyntactical) {
+        if (this_type_param_nullability == Nullability::kLegacy ||
+            this_type_param_nullability == Nullability::kUndetermined) {
+          this_type_param_nullability = Nullability::kNonNullable;
+        }
+        if (other_type_param_nullability == Nullability::kLegacy ||
+            other_type_param_nullability == Nullability::kUndetermined) {
+          other_type_param_nullability = Nullability::kNonNullable;
+        }
+      } else {
+        ASSERT(kind == TypeEquality::kCanonical);
+      }
+      if (this_type_param_nullability != other_type_param_nullability) {
+        return false;
+      }
     }
-    if (other_type_param_nullability == Nullability::kLegacy) {
-      other_type_param_nullability = Nullability::kNonNullable;
-    }
-  }
-  if (this_type_param_nullability != other_type_param_nullability) {
-    return false;
   }
   if (IsFinalized() == other_type_param.IsFinalized()) {
     return (index() == other_type_param.index());

runtime/vm/object.h

@@ -874,6 +874,14 @@ enum class Nullability : int8_t {
   kLegacy = 3,
 };
 
+// Equality kind between types.
+enum class TypeEquality {
+  kCanonical = 0,
+  kSyntactical = 1,
+  kSubtypeNullability = 2,
+  kIgnoreNullability = 3,
+};
+
 // The NNBDMode is passed to routines performing type reification and/or subtype
 // tests. The mode reflects the opted-in status of the library performing type
 // reification and/or subtype tests.
@@ -2514,7 +2522,8 @@ class Function : public Object {
 
   // Returns true if this function has the same number of type parameters with
   // equal bounds as the other function. Type parameter names are ignored.
-  bool HasSameTypeParametersAndBounds(const Function& other) const;
+  bool HasSameTypeParametersAndBounds(const Function& other,
+                                      TypeEquality kind) const;
 
   // Return the number of type parameters declared in parent generic functions.
   intptr_t NumParentTypeParameters() const;
@@ -7031,19 +7040,19 @@ class TypeArguments : public Instance {
   // Check if the vectors are equal (they may be null).
   bool Equals(const TypeArguments& other) const {
     return IsSubvectorEquivalent(other, 0, IsNull() ? 0 : Length(),
-                                 /* syntactically = */ false);
+                                 TypeEquality::kCanonical);
   }
 
   bool IsEquivalent(const TypeArguments& other,
-                    bool syntactically,
+                    TypeEquality kind,
                     TrailPtr trail = NULL) const {
-    return IsSubvectorEquivalent(other, 0, IsNull() ? 0 : Length(),
-                                 syntactically, trail);
+    return IsSubvectorEquivalent(other, 0, IsNull() ? 0 : Length(), kind,
+                                 trail);
   }
   bool IsSubvectorEquivalent(const TypeArguments& other,
                              intptr_t from_index,
                              intptr_t len,
-                             bool syntactically,
+                             TypeEquality kind,
                              TrailPtr trail = NULL) const;
 
   // Check if the vector is instantiated (it must not be null).
@@ -7233,10 +7242,10 @@ class AbstractType : public Instance {
   }
   virtual uint32_t CanonicalizeHash() const { return Hash(); }
   virtual bool Equals(const Instance& other) const {
-    return IsEquivalent(other, /* syntactically = */ false);
+    return IsEquivalent(other, TypeEquality::kCanonical);
   }
   virtual bool IsEquivalent(const Instance& other,
-                            bool syntactically,
+                            TypeEquality kind,
                             TrailPtr trail = NULL) const;
   virtual bool IsRecursive() const;
 
@@ -7482,7 +7491,7 @@ class Type : public AbstractType {
                               intptr_t num_free_fun_type_params = kAllFree,
                               TrailPtr trail = NULL) const;
   virtual bool IsEquivalent(const Instance& other,
-                            bool syntactically,
+                            TypeEquality kind,
                             TrailPtr trail = NULL) const;
   virtual bool IsRecursive() const;
 
@@ -7647,7 +7656,7 @@ class TypeRef : public AbstractType {
                               intptr_t num_free_fun_type_params = kAllFree,
                               TrailPtr trail = NULL) const;
   virtual bool IsEquivalent(const Instance& other,
-                            bool syntactically,
+                            TypeEquality kind,
                             TrailPtr trail = NULL) const;
   virtual bool IsRecursive() const { return true; }
   virtual bool IsFunctionType() const {
@@ -7731,7 +7740,7 @@ class TypeParameter : public AbstractType {
                               intptr_t num_free_fun_type_params = kAllFree,
                               TrailPtr trail = NULL) const;
   virtual bool IsEquivalent(const Instance& other,
-                            bool syntactically,
+                            TypeEquality kind,
                             TrailPtr trail = NULL) const;
   virtual bool IsRecursive() const { return false; }
   virtual RawAbstractType* InstantiateFrom(