Prevent expensive and unnecessary error formatting in the case a bound check is

runtime/vm/object.cc

Index: runtime/vm/object.cc
===================================================================
--- runtime/vm/object.cc	(revision 20937)
+++ runtime/vm/object.cc	(working copy)
@@ -2861,7 +2861,7 @@
     // substituted by the instantiator's type argument vector without checking
     // the upper bound.
     const AbstractType& bound = AbstractType::Handle(type_param.bound());
-    ASSERT(bound.IsFinalized());
+    ASSERT(bound.IsResolved());
     if (!bound.IsObjectType() && !bound.IsDynamicType()) {
       return false;
     }
@@ -9263,8 +9263,8 @@
 bool AbstractType::TypeTest(TypeTestKind test_kind,
                             const AbstractType& other,
                             Error* malformed_error) const {
-  ASSERT(IsFinalized());
-  ASSERT(other.IsFinalized());
+  ASSERT(IsResolved());
+  ASSERT(other.IsResolved());
   // In case the type checked in a type test is malformed, the code generator
   // may compile a throw instead of a run time call performing the type check.
   // However, in checked mode, a function type may include malformed result type
@@ -9301,13 +9301,10 @@
   // The same rule applies when checking the upper bound of a still
   // uninstantiated type at compile time. Returning false will defer the test
   // to run time.
-  // We may think that some cases can be decided at compile time.
+  // There are however some cases can be decided at compile time.
   // For example, with class A<K, V extends K>, new A<T, T> called from within
   // a class B<T> will never require a run time bound check, even if T is
   // uninstantiated at compile time.
-  // However, this is not true, because bounds are ignored in production mode,
-  // and even if we are running in checked mode, we may generate a snapshot
-  // that will be executed in production mode.
   if (IsTypeParameter()) {
     const TypeParameter& type_param = TypeParameter::Cast(*this);
     if (other.IsTypeParameter()) {
@@ -9316,6 +9313,10 @@
         return true;
       }
     }
+    const AbstractType& bound = AbstractType::Handle(type_param.bound());
+    if (bound.IsMoreSpecificThan(other, malformed_error)) {
+      return true;
+    }
     return false;  // TODO(regis): We should return "maybe after instantiation".
   }
   if (other.IsTypeParameter()) {
@@ -9532,7 +9533,7 @@
 RawAbstractType* Type::InstantiateFrom(
     const AbstractTypeArguments& instantiator_type_arguments,
     Error* malformed_error) const {
-  ASSERT(IsFinalized());
+  ASSERT(IsResolved());
   ASSERT(!IsInstantiated());
   // Return the uninstantiated type unchanged if malformed. No copy needed.
   if (IsMalformed()) {
@@ -9564,13 +9565,16 @@
     return false;
   }
   const Type& other_type = Type::Cast(other);
-  ASSERT(IsFinalized() && other_type.IsFinalized());
+  ASSERT(IsResolved() && other_type.IsResolved());
   if (IsMalformed() || other_type.IsMalformed()) {
     return false;
   }
   if (type_class() != other_type.type_class()) {
     return false;
   }
+  if (!IsFinalized() || !other_type.IsFinalized()) {
+    return false;
+  }
   return AbstractTypeArguments::AreEqual(
       AbstractTypeArguments::Handle(arguments()),
       AbstractTypeArguments::Handle(other_type.arguments()));
@@ -9780,49 +9784,60 @@
   if (instantiator_type_arguments.IsNull()) {
     return Type::DynamicType();
   }
-  // Bound checks should never appear in the instantiator type arguments.
-  ASSERT(!AbstractType::Handle(
-      instantiator_type_arguments.TypeAt(index())).IsBoundedType());
-  return instantiator_type_arguments.TypeAt(index());
+  // Bound checks may appear in the instantiator type arguments, as is the case
+  // with a pair of type parameters of the same class referring to each other
+  // via their bounds.
+  AbstractType& type_arg = AbstractType::Handle(
+      instantiator_type_arguments.TypeAt(index()));
+  if (type_arg.IsBoundedType()) {
+    const BoundedType& bounded_type = BoundedType::Cast(type_arg);
+    ASSERT(!bounded_type.IsInstantiated());
+    ASSERT(AbstractType::Handle(bounded_type.bound()).IsInstantiated());
+    type_arg = bounded_type.InstantiateFrom(AbstractTypeArguments::Handle(),
+                                            malformed_error);
+  }
+  return type_arg.raw();
 }
 
 
-void TypeParameter::CheckBound(const AbstractType& bounded_type,
+bool TypeParameter::CheckBound(const AbstractType& bounded_type,
                                const AbstractType& upper_bound,
                                Error* malformed_error) const {
-  ASSERT(malformed_error->IsNull());
+  ASSERT((malformed_error == NULL) || malformed_error->IsNull());
   ASSERT(bounded_type.IsFinalized());
   ASSERT(upper_bound.IsFinalized());
   ASSERT(!bounded_type.IsMalformed());
-  if (bounded_type.IsSubtypeOf(upper_bound, malformed_error) ||
-      !malformed_error->IsNull()) {
-    return;
+  if (bounded_type.IsSubtypeOf(upper_bound, malformed_error)) {
+    return true;
   }
-  // Report the bound error.
-  const String& bounded_type_name = String::Handle(
-      bounded_type.UserVisibleName());
-  const String& upper_bound_name = String::Handle(
-      upper_bound.UserVisibleName());
-  const AbstractType& declared_bound = AbstractType::Handle(bound());
-  const String& declared_bound_name = String::Handle(
-      declared_bound.UserVisibleName());
-  const String& type_param_name = String::Handle(UserVisibleName());
-  const Class& cls = Class::Handle(parameterized_class());
-  const String& class_name = String::Handle(cls.Name());
-  const Script& script = Script::Handle(cls.script());
-  // Since the bound may have been canonicalized, its token index is
-  // meaningless, therefore use the token index of this type parameter.
-  *malformed_error = FormatError(
-      *malformed_error,
-      script,
-      token_pos(),
-      "type parameter '%s' of class '%s' must extend bound '%s', "
-      "but type argument '%s' is not a subtype of '%s'\n",
-      type_param_name.ToCString(),
-      class_name.ToCString(),
-      declared_bound_name.ToCString(),
-      bounded_type_name.ToCString(),
-      upper_bound_name.ToCString());
+  if ((malformed_error != NULL) && malformed_error->IsNull()) {
+    // Report the bound error.
+    const String& bounded_type_name = String::Handle(
+        bounded_type.UserVisibleName());
+    const String& upper_bound_name = String::Handle(
+        upper_bound.UserVisibleName());
+    const AbstractType& declared_bound = AbstractType::Handle(bound());
+    const String& declared_bound_name = String::Handle(
+        declared_bound.UserVisibleName());
+    const String& type_param_name = String::Handle(UserVisibleName());
+    const Class& cls = Class::Handle(parameterized_class());
+    const String& class_name = String::Handle(cls.Name());
+    const Script& script = Script::Handle(cls.script());
+    // Since the bound may have been canonicalized, its token index is
+    // meaningless, therefore use the token index of this type parameter.
+    *malformed_error = FormatError(
+        *malformed_error,
+        script,
+        token_pos(),
+        "type parameter '%s' of class '%s' must extend bound '%s', "
+        "but type argument '%s' is not a subtype of '%s'\n",
+        type_param_name.ToCString(),
+        class_name.ToCString(),
+        declared_bound_name.ToCString(),
+        bounded_type_name.ToCString(),
+        upper_bound_name.ToCString());
+  }
+  return false;
 }