[analyzer] Allow downwards inference and upper bound heuristic wrt variance.

kallentu · commit-bot@chromium.org · commit 029e06b8d9eb · 2019-11-22T01:31:03.000Z
variance_downwards_inference_test now passes. The other part of this CL addresses dart-lang/language#658 The code has similar logic to https://dart-review.googlesource.com/c/sdk/+/125552. Change-Id: Ib2bc7e9c11574ccd9b16fe66472bba8c16dabd29 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/125486 Reviewed-by: Konstantin Shcheglov <scheglov@google.com> Reviewed-by: Leaf Petersen <leafp@google.com> Commit-Queue: Kallen Tu <kallentu@google.com>
diff --git a/pkg/analyzer/lib/src/generated/type_system.dart b/pkg/analyzer/lib/src/generated/type_system.dart
@@ -2009,13 +2009,11 @@ class GenericInferrer {
     // degradation for f-bounded type parameters.
     var inferredTypes = new List<DartType>.filled(
         typeFormals.length, UnknownInferredType.instance);
-    var _inferTypeParameter = downwardsInferPhase
-        ? _inferTypeParameterFromContext
-        : _inferTypeParameterFromAll;
 
     for (int i = 0; i < typeFormals.length; i++) {
-      TypeParameterElement typeParam = typeFormals[i];
-
+      // TODO (kallentu) : Clean up TypeParameterElementImpl casting once
+      // variance is added to the interface.
+      TypeParameterElementImpl typeParam = typeFormals[i];
       _TypeConstraint extendsClause;
       if (considerExtendsClause && typeParam.bound != null) {
         extendsClause = new _TypeConstraint.fromExtends(
@@ -2024,8 +2022,12 @@ class GenericInferrer {
                 .substituteType(typeParam.bound));
       }
 
-      inferredTypes[i] =
-          _inferTypeParameter(constraints[typeParam], extendsClause);
+      inferredTypes[i] = downwardsInferPhase || !typeParam.isLegacyCovariant
+          ? _inferTypeParameterFromContext(
+              constraints[typeParam], extendsClause,
+              isContravariant: typeParam.variance.isContravariant)
+          : _inferTypeParameterFromAll(constraints[typeParam], extendsClause,
+              isContravariant: typeParam.variance.isContravariant);
     }
 
     // If the downwards infer phase has failed, we'll catch this in the upwards
@@ -2166,8 +2168,12 @@ class GenericInferrer {
   /// * `int <: T`
   ///
   /// ... and no upper bound. Therefore the lower bound is the best choice.
+  ///
+  /// If [isContravariant] is `true`, then we are solving for a contravariant
+  /// type parameter which means we choose the upper bound rather than the
+  /// lower bound for normally covariant type parameters.
   DartType _chooseTypeFromConstraints(Iterable<_TypeConstraint> constraints,
-      {bool toKnownType: false}) {
+      {bool toKnownType: false, @required bool isContravariant}) {
     DartType lower = UnknownInferredType.instance;
     DartType upper = UnknownInferredType.instance;
     for (var constraint in constraints) {
@@ -2193,20 +2199,37 @@ class GenericInferrer {
     // Prefer the known bound, if any.
     // Otherwise take whatever bound has partial information, e.g. `Iterable<?>`
     //
-    // For both of those, prefer the lower bound (arbitrary heuristic).
-    if (UnknownInferredType.isKnown(lower)) {
-      return lower;
-    }
-    if (UnknownInferredType.isKnown(upper)) {
+    // For both of those, prefer the lower bound (arbitrary heuristic) or upper
+    // bound if [isContravariant] is `true`
+    if (isContravariant) {
+      if (UnknownInferredType.isKnown(upper)) {
+        return upper;
+      }
+      if (UnknownInferredType.isKnown(lower)) {
+        return lower;
+      }
+      if (!identical(UnknownInferredType.instance, upper)) {
+        return toKnownType ? _typeSystem.upperBoundForType(upper) : upper;
+      }
+      if (!identical(UnknownInferredType.instance, lower)) {
+        return toKnownType ? _typeSystem.lowerBoundForType(lower) : lower;
+      }
       return upper;
+    } else {
+      if (UnknownInferredType.isKnown(lower)) {
+        return lower;
+      }
+      if (UnknownInferredType.isKnown(upper)) {
+        return upper;
+      }
+      if (!identical(UnknownInferredType.instance, lower)) {
+        return toKnownType ? _typeSystem.lowerBoundForType(lower) : lower;
+      }
+      if (!identical(UnknownInferredType.instance, upper)) {
+        return toKnownType ? _typeSystem.upperBoundForType(upper) : upper;
+      }
+      return lower;
     }
-    if (!identical(UnknownInferredType.instance, lower)) {
-      return toKnownType ? _typeSystem.lowerBoundForType(lower) : lower;
-    }
-    if (!identical(UnknownInferredType.instance, upper)) {
-      return toKnownType ? _typeSystem.upperBoundForType(upper) : upper;
-    }
-    return lower;
   }
 
   String _formatError(TypeParameterElement typeParam, DartType inferred,
@@ -2280,11 +2303,13 @@ class GenericInferrer {
   }
 
   DartType _inferTypeParameterFromAll(
-      List<_TypeConstraint> constraints, _TypeConstraint extendsClause) {
+      List<_TypeConstraint> constraints, _TypeConstraint extendsClause,
+      {@required bool isContravariant}) {
     // See if we already fixed this type from downwards inference.
     // If so, then we aren't allowed to change it based on argument types.
     DartType t = _inferTypeParameterFromContext(
-        constraints.where((c) => c.isDownwards), extendsClause);
+        constraints.where((c) => c.isDownwards), extendsClause,
+        isContravariant: isContravariant);
     if (UnknownInferredType.isKnown(t)) {
       // Remove constraints that aren't downward ones; we'll ignore these for
       // error reporting, because inference already succeeded.
@@ -2296,13 +2321,16 @@ class GenericInferrer {
       constraints = constraints.toList()..add(extendsClause);
     }
 
-    var choice = _chooseTypeFromConstraints(constraints, toKnownType: true);
+    var choice = _chooseTypeFromConstraints(constraints,
+        toKnownType: true, isContravariant: isContravariant);
     return choice;
   }
 
   DartType _inferTypeParameterFromContext(
-      Iterable<_TypeConstraint> constraints, _TypeConstraint extendsClause) {
-    DartType t = _chooseTypeFromConstraints(constraints);
+      Iterable<_TypeConstraint> constraints, _TypeConstraint extendsClause,
+      {@required bool isContravariant}) {
+    DartType t = _chooseTypeFromConstraints(constraints,
+        isContravariant: isContravariant);
     if (UnknownInferredType.isUnknown(t)) {
       return t;
     }
@@ -2316,7 +2344,8 @@ class GenericInferrer {
     // If we consider the `T extends num` we conclude `<num>`, which works.
     if (extendsClause != null) {
       constraints = constraints.toList()..add(extendsClause);
-      return _chooseTypeFromConstraints(constraints);
+      return _chooseTypeFromConstraints(constraints,
+          isContravariant: isContravariant);
     }
     return t;
   }
diff --git a/pkg/analyzer/test/generated/type_system_test.dart b/pkg/analyzer/test/generated/type_system_test.dart
@@ -1191,6 +1191,50 @@ class GenericFunctionInferenceTest extends AbstractTypeSystemTest {
     expect(_inferCall(f, [intType]), [intType]);
   }
 
+  void test_parameter_contravariantUseUpperBound() {
+    // <T>(T x, void Function(T) y) -> T
+    // Generates constraints int <: T <: num.
+    // Since T is contravariant, choose num.
+    var T = typeParameter('T', variance: Variance.contravariant);
+    var tFunction = functionTypeStar(
+        parameters: [requiredParameter(type: typeParameterTypeStar(T))],
+        returnType: voidType);
+    var numFunction = functionTypeStar(
+        parameters: [requiredParameter(type: numType)], returnType: voidType);
+    var function = functionTypeStar(
+      typeFormals: [T],
+      parameters: [
+        requiredParameter(type: typeParameterTypeStar(T)),
+        requiredParameter(type: tFunction)
+      ],
+      returnType: typeParameterTypeStar(T),
+    );
+
+    expect(_inferCall(function, [intType, numFunction]), [numType]);
+  }
+
+  void test_parameter_covariantUseLowerBound() {
+    // <T>(T x, void Function(T) y) -> T
+    // Generates constraints int <: T <: num.
+    // Since T is covariant, choose int.
+    var T = typeParameter('T', variance: Variance.covariant);
+    var tFunction = functionTypeStar(
+        parameters: [requiredParameter(type: typeParameterTypeStar(T))],
+        returnType: voidType);
+    var numFunction = functionTypeStar(
+        parameters: [requiredParameter(type: numType)], returnType: voidType);
+    var function = functionTypeStar(
+      typeFormals: [T],
+      parameters: [
+        requiredParameter(type: typeParameterTypeStar(T)),
+        requiredParameter(type: tFunction)
+      ],
+      returnType: typeParameterTypeStar(T),
+    );
+
+    expect(_inferCall(function, [intType, numFunction]), [intType]);
+  }
+
   void test_returnFunctionWithGenericParameter() {
     // <T>(T -> T) -> (T -> void)
     var T = typeParameter('T');
diff --git a/tests/language_2/variance/variance_in_inference_error_test.dart b/tests/language_2/variance/variance_in_inference_error_test.dart
@@ -87,15 +87,15 @@ main() {
   // Since T is contravariant, we choose Upper as the solution.
   var inferredContraUpper = inferContraBound(ContraBound(Lower(), (Upper x) {}));
   lower = inferredContraUpper;
-  //      ^
-  // [analyzer] unspecified
+  //      ^^^^^^^^^^^^^^^^^^^
+  // [analyzer] STATIC_TYPE_WARNING.INVALID_ASSIGNMENT
   // [cfe] A value of type 'Exactly<Upper>' can't be assigned to a variable of type 'Exactly<Lower>'.
 
   // Inference for Contrabound(...) produces Lower <: T <: Middle.
   // Since T is contravariant, we choose Middle as the solution.
   var inferredContraMiddle = inferContraBound(ContraBound(Lower(), (Middle x) {}));
   lower = inferredContraMiddle;
-  //      ^
-  // [analyzer] unspecified
+  //      ^^^^^^^^^^^^^^^^^^^^
+  // [analyzer] STATIC_TYPE_WARNING.INVALID_ASSIGNMENT
   // [cfe] A value of type 'Exactly<Middle>' can't be assigned to a variable of type 'Exactly<Lower>'.
 }
