fix #26990, incorrect substitution of upper bound

We were handling the "extends" clause too early, when we did not have enough information to correctly substitute it. So we tried to substitute later on, but this breaks cases where we (correctly) inferred using the argument types.

The fix is to move the "extends" clause handling to a place where we have the inferred types for our preceding type variables available.

[email protected]

Review URL: https://codereview.chromium.org/2205993002 .

Author: John Messerly

pkg/analyzer/lib/src/generated/type_system.dart

@@ -163,7 +163,7 @@ class StrongTypeSystemImpl extends TypeSystem {
     // subtypes (or supertypes) as necessary, and track the constraints that
     // are implied by this.
     var inferringTypeSystem =
-        new _StrongInferenceTypeSystem(typeProvider, fnType.typeFormals);
+        new _StrongInferenceTypeSystem(typeProvider, this, fnType.typeFormals);
 
     // Since we're trying to infer the instantiation, we want to ignore type
     // formals as we check the parameters and return type.
@@ -216,7 +216,7 @@ class StrongTypeSystemImpl extends TypeSystem {
     // subtypes (or supertypes) as necessary, and track the constraints that
     // are implied by this.
     var inferringTypeSystem =
-        new _StrongInferenceTypeSystem(typeProvider, fnType.typeFormals);
+        new _StrongInferenceTypeSystem(typeProvider, this, fnType.typeFormals);
 
     // Special case inference for Future.then.
     //
@@ -1271,43 +1271,36 @@ class TypeSystemImpl extends TypeSystem {
 /// Tracks upper and lower type bounds for a set of type parameters.
 class _StrongInferenceTypeSystem extends StrongTypeSystemImpl {
   final TypeProvider _typeProvider;
+
+  /// The outer strong mode type system, used for GLB and LUB, so we don't
+  /// recurse into our constraint solving code.
+  final StrongTypeSystemImpl _typeSystem;
   final Map<TypeParameterType, _TypeParameterBound> _bounds;
 
-  _StrongInferenceTypeSystem(
-      this._typeProvider, Iterable<TypeParameterElement> typeFormals)
-      : _bounds =
-            new Map.fromIterable(typeFormals, key: (t) => t.type, value: (t) {
-          _TypeParameterBound bound = new _TypeParameterBound();
-          if (t.bound != null) bound.upper = t.bound;
-          return bound;
-        });
+  _StrongInferenceTypeSystem(this._typeProvider, this._typeSystem,
+      Iterable<TypeParameterElement> typeFormals)
+      : _bounds = new Map.fromIterable(typeFormals,
+            key: (t) => t.type, value: (t) => new _TypeParameterBound());
 
   /// Given the constraints that were given by calling [isSubtypeOf], find the
   /// instantiation of the generic function that satisfies these constraints.
   FunctionType _infer(FunctionType fnType) {
     List<TypeParameterType> fnTypeParams =
         TypeParameterTypeImpl.getTypes(fnType.typeFormals);
 
-    var inferredTypes = new List<DartType>.from(fnTypeParams, growable: false);
+    // Initialize the inferred type array.
+    //
+    // They all start as `dynamic` to offer reasonable degradation for f-bounded
+    // type parameters.
+    var inferredTypes = new List<DartType>.filled(
+        fnTypeParams.length, DynamicTypeImpl.instance,
+        growable: false);
+
     for (int i = 0; i < fnTypeParams.length; i++) {
       TypeParameterType typeParam = fnTypeParams[i];
-      _TypeParameterBound bound = _bounds[typeParam];
 
-      // Now we've computed lower and upper bounds for each type parameter.
+      // Apply the `extends` clause for the type parameter, if any.
       //
-      // To decide on which type to assign, we look at the return type and see
-      // if the type parameter occurs in covariant or contravariant positions.
-      //
-      // If the type is "passed in" at all, or if our lower bound was bottom,
-      // we choose the upper bound as being the most useful.
-      //
-      // Otherwise we choose the more precise lower bound.
-      _TypeParameterVariance variance =
-          new _TypeParameterVariance.from(typeParam, fnType.returnType);
-
-      inferredTypes[i] =
-          variance.passedIn || bound.lower.isBottom ? bound.upper : bound.lower;
-
       // Assumption: if the current type parameter has an "extends" clause
       // that refers to another type variable we are inferring, it will appear
       // before us or in this list position. For example:
@@ -1323,8 +1316,28 @@ class _StrongInferenceTypeSystem extends StrongTypeSystemImpl {
       // Or if the type parameter's bound depends on itself such as:
       //
       //     <T extends Clonable<T>>
+      DartType declaredUpperBound = typeParam.element.bound;
+      if (declaredUpperBound != null) {
+        // Assert that the type parameter is a subtype of its bound.
+        _inferTypeParameterSubtypeOf(typeParam,
+            declaredUpperBound.substitute2(inferredTypes, fnTypeParams), null);
+      }
+
+      // Now we've computed lower and upper bounds for each type parameter.
+      //
+      // To decide on which type to assign, we look at the return type and see
+      // if the type parameter occurs in covariant or contravariant positions.
+      //
+      // If the type is "passed in" at all, or if our lower bound was bottom,
+      // we choose the upper bound as being the most useful.
+      //
+      // Otherwise we choose the more precise lower bound.
+      _TypeParameterVariance variance =
+          new _TypeParameterVariance.from(typeParam, fnType.returnType);
+
+      _TypeParameterBound bound = _bounds[typeParam];
       inferredTypes[i] =
-          inferredTypes[i].substitute2(inferredTypes, fnTypeParams);
+          variance.passedIn || bound.lower.isBottom ? bound.upper : bound.lower;
 
       // See if the constraints on the type variable are satisfied.
       //
@@ -1358,7 +1371,8 @@ class _StrongInferenceTypeSystem extends StrongTypeSystemImpl {
         // We already know T1 <: U, for some U.
         // So update U to reflect the new constraint T1 <: GLB(U, T2)
         //
-        bound.upper = getGreatestLowerBound(_typeProvider, bound.upper, t2);
+        bound.upper =
+            _typeSystem.getGreatestLowerBound(_typeProvider, bound.upper, t2);
         // Optimistically assume we will be able to satisfy the constraint.
         return true;
       }
@@ -1372,7 +1386,8 @@ class _StrongInferenceTypeSystem extends StrongTypeSystemImpl {
         // We already know L <: T2, for some L.
         // So update L to reflect the new constraint LUB(L, T1) <: T2
         //
-        bound.lower = getLeastUpperBound(_typeProvider, bound.lower, t1);
+        bound.lower =
+            _typeSystem.getLeastUpperBound(_typeProvider, bound.lower, t1);
         // Optimistically assume we will be able to satisfy the constraint.
         return true;
       }

pkg/analyzer/test/src/summary/resynthesize_ast_test.dart

@@ -155,6 +155,12 @@ class AstInferredTypeTest extends AbstractResynthesizeTest
     super.test_circularReference_viaClosures_initializerTypes();
   }
 
+  @override
+  @failingTest
+  void test_constructors_inferenceFBounded() {
+    super.test_constructors_inferenceFBounded();
+  }
+
   @override
   @failingTest
   void test_constructors_inferFromArguments() {

pkg/analyzer/test/src/task/strong/inferred_type_test.dart

@@ -588,6 +588,24 @@ class C2 implements A, B {
 ''');
   }
 
+  void test_constructors_inferenceFBounded() {
+    // Regression for https://github.com/dart-lang/sdk/issues/26990
+    var unit = checkFile('''
+class Clonable<T> {}
+
+class Pair<T extends Clonable<T>, U extends Clonable<U>> {
+  T t;
+  U u;
+  Pair(this.t, this.u);
+  Pair._();
+  Pair<U, T> get reversed => /*info:INFERRED_TYPE_ALLOCATION*/new Pair(u, t);
+}
+final x = /*info:INFERRED_TYPE_ALLOCATION*/new Pair._();
+    ''');
+    var x = unit.topLevelVariables[0];
+    expect(x.type.toString(), 'Pair<Clonable<dynamic>, Clonable<dynamic>>');
+  }
+
   void test_constructors_inferFromArguments() {
     var unit = checkFile('''
 class C<T> {
@@ -734,6 +752,18 @@ main() {
     expect(unit.topLevelVariables[0].type.toString(), 'C<int>');
   }
 
+  void test_constructors_reverseTypeParameters() {
+    // Regression for https://github.com/dart-lang/sdk/issues/26990
+    checkFile('''
+class Pair<T, U> {
+  T t;
+  U u;
+  Pair(this.t, this.u);
+  Pair<U, T> get reversed => /*info:INFERRED_TYPE_ALLOCATION*/new Pair(u, t);
+}
+    ''');
+  }
+
   void test_doNotInferOverriddenFieldsThatExplicitlySayDynamic_infer() {
     checkFile('''
 class A {