[cfe] Fix bottomType as Never in closures of type schemas

In pre-NNBD Dart the greatest and lowest closures of a type schema were
using `Null` as the bottom type and `dynamic` as the top type. In
NNBD code, the top type should be `Object?', and the bottom type should
be `Never`. Since pre-NNBD code is being deprecated, the code
that allows to choose between the two pairs of extreme types should be
removed, and `Object?` and `Never` should be used as the only options.

For historical reasons, replacing `Object?` with `dynamic` in the
algorithms for the greatest and the lowest closures leads to breakages
in the existing NNBD code. However, the bottom type is assumed to be
`Never` everywhere. This CL removes the ability to parametrise over
the bottom type, choosing `Never` as the only option. The discrepancy
between `Object?` and `dynamic` will be addressed later.

Change-Id: Ia58ccaca37761c5de8e005f11e79b23d02a0e416
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/395963
Reviewed-by: Johnni Winther <[email protected]>
Commit-Queue: Chloe Stefantsova <[email protected]>

Author: chloestefantsova

pkg/front_end/lib/src/type_inference/inference_visitor_base.dart

@@ -187,11 +187,11 @@ abstract class InferenceVisitorBase implements InferenceVisitor {
   StaticTypeContext get staticTypeContext => _inferrer.staticTypeContext;
 
   DartType computeGreatestClosure(DartType type) {
-    return greatestClosure(type, const DynamicType(), bottomType);
+    return greatestClosure(type, topType: const DynamicType());
   }
 
   DartType computeGreatestClosure2(DartType type) {
-    return greatestClosure(type, coreTypes.objectNullableRawType, bottomType);
+    return greatestClosure(type, topType: coreTypes.objectNullableRawType);
   }
 
   DartType computeNullable(DartType type) {

pkg/front_end/lib/src/type_inference/standard_bounds.dart

@@ -2,7 +2,7 @@
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
-import 'package:kernel/ast.dart' show DartType, NeverType;
+import 'package:kernel/ast.dart' show DartType;
 import 'package:kernel/src/standard_bounds.dart';
 
 import 'type_schema.dart' show UnknownType;
@@ -13,16 +13,15 @@ mixin TypeSchemaStandardBounds on StandardBounds {
   DartType greatestClosureForLowerBound(DartType typeSchema) {
     //  - We replace all uses of `T1 <: T2` in the `DOWN` algorithm by `S1 <:
     //  S2` where `Si` is the greatest closure of `Ti` with respect to `_`.
-    return greatestClosure(typeSchema, coreTypes.objectNullableRawType,
-        const NeverType.nonNullable());
+    return greatestClosure(typeSchema,
+        topType: coreTypes.objectNullableRawType);
   }
 
   @override
   DartType leastClosureForUpperBound(DartType typeSchema) {
     //  - We replace all uses of `T1 <: T2` in the `UP` algorithm by `S1 <: S2`
     //  where `Si` is the least closure of `Ti` with respect to `_`.
-    return leastClosure(typeSchema, coreTypes.objectNullableRawType,
-        const NeverType.nonNullable());
+    return leastClosure(typeSchema, topType: coreTypes.objectNullableRawType);
   }
 
   @override

pkg/front_end/lib/src/type_inference/type_inference_engine.dart

@@ -540,8 +540,7 @@ class OperationsCfe
       SharedTypeSchemaView<DartType> schema) {
     return new SharedTypeView(type_schema_elimination.greatestClosure(
         schema.unwrapTypeSchemaView(),
-        const DynamicType(),
-        const NeverType.nonNullable()));
+        topType: const DynamicType()));
   }
 
   @override

pkg/front_end/lib/src/type_inference/type_schema_elimination.dart

@@ -20,9 +20,9 @@ import 'type_schema.dart' show UnknownType;
 ///
 /// Note that the greatest closure of a type schema is always a supertype of any
 /// type which matches the schema.
-DartType greatestClosure(
-    DartType schema, DartType topType, DartType bottomType) {
-  return _TypeSchemaEliminationVisitor.run(false, schema, topType, bottomType);
+DartType greatestClosure(DartType schema, {required DartType topType}) {
+  return _TypeSchemaEliminationVisitor.run(schema,
+      computeLeastClosure: false, topType: topType);
 }
 
 /// Returns the least closure of the given type [schema] with respect to `?`.
@@ -38,8 +38,9 @@ DartType greatestClosure(
 ///
 /// Note that the least closure of a type schema is always a subtype of any type
 /// which matches the schema.
-DartType leastClosure(DartType schema, DartType topType, DartType bottomType) {
-  return _TypeSchemaEliminationVisitor.run(true, schema, topType, bottomType);
+DartType leastClosure(DartType schema, {required DartType topType}) {
+  return _TypeSchemaEliminationVisitor.run(schema,
+      computeLeastClosure: true, topType: topType);
 }
 
 /// Visitor that computes least and greatest closures of a type schema.
@@ -48,16 +49,15 @@ DartType leastClosure(DartType schema, DartType topType, DartType bottomType) {
 /// type, otherwise it returns the result of substituting `?` with `Null` or
 /// `Object`, as appropriate.
 class _TypeSchemaEliminationVisitor extends ReplacementVisitor {
-  final DartType topType;
-  final DartType bottomType;
+  final DartType _topType;
 
-  _TypeSchemaEliminationVisitor(this.topType, this.bottomType);
+  _TypeSchemaEliminationVisitor(this._topType);
 
   @override
   DartType? visitAuxiliaryType(AuxiliaryType node, Variance variance) {
-    bool isLeastClosure = variance == Variance.covariant;
+    bool computeLeastClosure = variance == Variance.covariant;
     if (node is UnknownType) {
-      return isLeastClosure ? bottomType : topType;
+      return computeLeastClosure ? const NeverType.nonNullable() : _topType;
     }
     // Coverage-ignore-block(suite): Not run.
     throw new UnsupportedError(
@@ -67,21 +67,18 @@ class _TypeSchemaEliminationVisitor extends ReplacementVisitor {
   /// Runs an instance of the visitor on the given [schema] and returns the
   /// resulting type.  If the schema contains no instances of `?`, the original
   /// schema object is returned to avoid unnecessary allocation.
-  static DartType run(bool isLeastClosure, DartType schema, DartType topType,
-      DartType bottomType) {
+  static DartType run(DartType schema,
+      {required bool computeLeastClosure, required DartType topType}) {
     assert(topType == const DynamicType() ||
         topType is InterfaceType &&
             topType.nullability == Nullability.nullable &&
             topType.classNode.enclosingLibrary.importUri.isScheme("dart") &&
             topType.classNode.enclosingLibrary.importUri.path == "core" &&
             topType.classNode.name == "Object");
-    assert(bottomType == const NeverType.nonNullable() ||
-        // Coverage-ignore(suite): Not run.
-        bottomType is NullType);
     _TypeSchemaEliminationVisitor visitor =
-        new _TypeSchemaEliminationVisitor(topType, bottomType);
-    DartType? result = schema.accept1(
-        visitor, isLeastClosure ? Variance.covariant : Variance.contravariant);
+        new _TypeSchemaEliminationVisitor(topType);
+    DartType? result = schema.accept1(visitor,
+        computeLeastClosure ? Variance.covariant : Variance.contravariant);
     return result ?? schema;
   }
 }

pkg/front_end/lib/src/type_inference/type_schema_environment.dart

@@ -392,12 +392,10 @@ class TypeSchemaEnvironment extends HierarchyBasedTypeEnvironment
   /// 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 solveTypeConstraint(
-      MergedTypeConstraint constraint, DartType topType, DartType bottomType,
-      {bool grounded = false, bool isContravariant = false}) {
-    assert(bottomType == const NeverType.nonNullable() ||
-        // Coverage-ignore(suite): Not run.
-        bottomType == const NullType());
+  DartType solveTypeConstraint(MergedTypeConstraint constraint,
+      {required DartType topType,
+      bool grounded = false,
+      bool isContravariant = false}) {
     if (!isContravariant) {
       // Prefer the known bound, if any.
       if (isKnown(constraint.lower.unwrapTypeSchemaView())) {
@@ -411,13 +409,13 @@ class TypeSchemaEnvironment extends HierarchyBasedTypeEnvironment
       // e.g. `Iterable<?>`
       if (constraint.lower is! SharedUnknownTypeSchemaView<DartType>) {
         return grounded
-            ? leastClosure(
-                constraint.lower.unwrapTypeSchemaView(), topType, bottomType)
+            ? leastClosure(constraint.lower.unwrapTypeSchemaView(),
+                topType: topType)
             : constraint.lower.unwrapTypeSchemaView();
       } else if (constraint.upper is! UnknownType) {
         return grounded
-            ? greatestClosure(
-                constraint.upper.unwrapTypeSchemaView(), topType, bottomType)
+            ? greatestClosure(constraint.upper.unwrapTypeSchemaView(),
+                topType: topType)
             : constraint.upper.unwrapTypeSchemaView();
       } else {
         return const UnknownType();
@@ -437,13 +435,13 @@ class TypeSchemaEnvironment extends HierarchyBasedTypeEnvironment
       if (constraint.upper is! UnknownType) {
         // Coverage-ignore-block(suite): Not run.
         return grounded
-            ? greatestClosure(
-                constraint.upper.unwrapTypeSchemaView(), topType, bottomType)
+            ? greatestClosure(constraint.upper.unwrapTypeSchemaView(),
+                topType: topType)
             : constraint.upper.unwrapTypeSchemaView();
       } else if (constraint.lower is! UnknownType) {
         return grounded
-            ? leastClosure(
-                constraint.lower.unwrapTypeSchemaView(), topType, bottomType)
+            ? leastClosure(constraint.lower.unwrapTypeSchemaView(),
+                topType: topType)
             :
             // Coverage-ignore(suite): Not run.
             constraint.lower.unwrapTypeSchemaView();
@@ -535,9 +533,10 @@ class TypeSchemaEnvironment extends HierarchyBasedTypeEnvironment
           new SharedTypeSchemaView(extendsConstraint), operations);
     }
 
-    return solveTypeConstraint(constraint, coreTypes.objectNullableRawType,
-        const NeverType.nonNullable(),
-        grounded: true, isContravariant: isContravariant);
+    return solveTypeConstraint(constraint,
+        topType: coreTypes.objectNullableRawType,
+        grounded: true,
+        isContravariant: isContravariant);
   }
 
   void _mergeInConstraintsFromBound(
@@ -577,7 +576,7 @@ class TypeSchemaEnvironment extends HierarchyBasedTypeEnvironment
     }
 
     DartType t = solveTypeConstraint(constraint,
-        coreTypes.objectNullableRawType, const NeverType.nonNullable());
+        topType: coreTypes.objectNullableRawType);
     if (!isKnown(t)) {
       return t;
     }
@@ -604,8 +603,8 @@ class TypeSchemaEnvironment extends HierarchyBasedTypeEnvironment
       constraint = constraint.clone();
       constraint.mergeInTypeSchemaUpper(
           new SharedTypeSchemaView(extendsConstraint), operations);
-      return solveTypeConstraint(constraint, coreTypes.objectNullableRawType,
-          const NeverType.nonNullable());
+      return solveTypeConstraint(constraint,
+          topType: coreTypes.objectNullableRawType);
     }
 
     return t;

pkg/front_end/test/type_inference/type_schema_elimination_nnbd_test.dart

@@ -24,13 +24,13 @@ class TypeSchemaEliminationTest {
   };
 
   DartType greatestClosure(DartType schema) {
-    return typeSchemaElimination.greatestClosure(
-        schema, const DynamicType(), const NeverType.nonNullable());
+    return typeSchemaElimination.greatestClosure(schema,
+        topType: env.coreTypes.objectNullableRawType);
   }
 
   DartType leastClosure(DartType schema) {
-    return typeSchemaElimination.leastClosure(
-        schema, const DynamicType(), const NeverType.nonNullable());
+    return typeSchemaElimination.leastClosure(schema,
+        topType: env.coreTypes.objectNullableRawType);
   }
 
   void testGreatest(String type, String expectedClosure) {
@@ -45,47 +45,47 @@ class TypeSchemaEliminationTest {
   }
 
   void test_greatestClosure_contravariant() {
-    testGreatest("(UNKNOWN) ->* dynamic", "(Never) ->* dynamic");
-    testGreatest("({UNKNOWN foo}) ->* dynamic", "({Never foo}) ->* dynamic");
+    testGreatest("(UNKNOWN) -> dynamic", "(Never) -> dynamic");
+    testGreatest("({UNKNOWN foo}) -> dynamic", "({Never foo}) -> dynamic");
   }
 
   void test_greatestClosure_contravariant_contravariant() {
-    testGreatest("((UNKNOWN) ->* dynamic) ->* dynamic",
-        "((dynamic) ->* dynamic) ->* dynamic");
+    testGreatest("((UNKNOWN) -> dynamic) -> dynamic",
+        "((Object?) -> dynamic) -> dynamic");
   }
 
   void test_greatestClosure_covariant() {
-    testGreatest("() ->* UNKNOWN", "() ->* dynamic");
-    testGreatest("List<UNKNOWN>*", "List<dynamic>*");
+    testGreatest("() -> UNKNOWN", "() -> Object?");
+    testGreatest("List<UNKNOWN>", "List<Object?>");
   }
 
   void test_greatestClosure_function_multipleUnknown() {
-    testGreatest("(UNKNOWN, UNKNOWN, {UNKNOWN a, UNKNOWN b}) ->* UNKNOWN",
-        "(Never, Never, {Never a, Never b}) ->* dynamic");
+    testGreatest("(UNKNOWN, UNKNOWN, {UNKNOWN a, UNKNOWN b}) -> UNKNOWN",
+        "(Never, Never, {Never a, Never b}) -> Object?");
   }
 
   void test_greatestClosure_simple() {
-    testGreatest("UNKNOWN", "dynamic");
+    testGreatest("UNKNOWN", "Object?");
   }
 
   void test_leastClosure_contravariant() {
-    testLeast("(UNKNOWN) ->* dynamic", "(dynamic) ->* dynamic");
-    testLeast("({UNKNOWN foo}) ->* dynamic", "({dynamic foo}) ->* dynamic");
+    testLeast("(UNKNOWN) -> dynamic", "(Object?) -> dynamic");
+    testLeast("({UNKNOWN foo}) -> dynamic", "({Object? foo}) -> dynamic");
   }
 
   void test_leastClosure_contravariant_contravariant() {
-    testLeast("((UNKNOWN) ->* dynamic) ->* dynamic",
-        "((Never) ->* dynamic) ->* dynamic");
+    testLeast(
+        "((UNKNOWN) -> dynamic) -> dynamic", "((Never) -> dynamic) -> dynamic");
   }
 
   void test_leastClosure_covariant() {
-    testLeast("() ->* UNKNOWN", "() ->* Never");
-    testLeast("List<UNKNOWN>*", "List<Never>*");
+    testLeast("() -> UNKNOWN", "() -> Never");
+    testLeast("List<UNKNOWN>", "List<Never>");
   }
 
   void test_leastClosure_function_multipleUnknown() {
-    testLeast("(UNKNOWN, UNKNOWN, {UNKNOWN a, UNKNOWN b}) ->* UNKNOWN",
-        "(dynamic, dynamic, {dynamic a, dynamic b}) ->* Never");
+    testLeast("(UNKNOWN, UNKNOWN, {UNKNOWN a, UNKNOWN b}) -> UNKNOWN",
+        "(Object?, Object?, {Object? a, Object? b}) -> Never");
   }
 
   void test_leastClosure_simple() {

pkg/front_end/test/type_inference/type_schema_elimination_test.dart

@@ -24,13 +24,13 @@ class TypeSchemaEliminationTest {
   };
 
   DartType greatestClosure(DartType schema) {
-    return typeSchemaElimination.greatestClosure(
-        schema, new DynamicType(), new NullType());
+    return typeSchemaElimination.greatestClosure(schema,
+        topType: new DynamicType());
   }
 
   DartType leastClosure(DartType schema) {
-    return typeSchemaElimination.leastClosure(
-        schema, new DynamicType(), new NullType());
+    return typeSchemaElimination.leastClosure(schema,
+        topType: new DynamicType());
   }
 
   void testGreatest(String type, String expectedClosure) {
@@ -45,8 +45,8 @@ class TypeSchemaEliminationTest {
   }
 
   void test_greatestClosure_contravariant() {
-    testGreatest("(UNKNOWN) ->* dynamic", "(Null) ->* dynamic");
-    testGreatest("({UNKNOWN foo}) ->* dynamic", "({Null foo}) ->* dynamic");
+    testGreatest("(UNKNOWN) ->* dynamic", "(Never) ->* dynamic");
+    testGreatest("({UNKNOWN foo}) ->* dynamic", "({Never foo}) ->* dynamic");
   }
 
   void test_greatestClosure_contravariant_contravariant() {
@@ -61,7 +61,7 @@ class TypeSchemaEliminationTest {
 
   void test_greatestClosure_function_multipleUnknown() {
     testGreatest("(UNKNOWN, UNKNOWN, {UNKNOWN a, UNKNOWN b}) ->* UNKNOWN",
-        "(Null, Null, {Null a, Null b}) ->* dynamic");
+        "(Never, Never, {Never a, Never b}) ->* dynamic");
   }
 
   void test_greatestClosure_simple() {
@@ -75,20 +75,20 @@ class TypeSchemaEliminationTest {
 
   void test_leastClosure_contravariant_contravariant() {
     testLeast("((UNKNOWN) ->* UNKNOWN) ->* dynamic",
-        "((Null) ->* dynamic) ->* dynamic");
+        "((Never) ->* dynamic) ->* dynamic");
   }
 
   void test_leastClosure_covariant() {
-    testLeast("() ->* UNKNOWN", "() ->* Null");
-    testLeast("List<UNKNOWN>*", "List<Null>*");
+    testLeast("() ->* UNKNOWN", "() ->* Never");
+    testLeast("List<UNKNOWN>*", "List<Never>*");
   }
 
   void test_leastClosure_function_multipleUnknown() {
     testLeast("(UNKNOWN, UNKNOWN, {UNKNOWN a, UNKNOWN b}) ->* UNKNOWN",
-        "(dynamic, dynamic, {dynamic a, dynamic b}) ->* Null");
+        "(dynamic, dynamic, {dynamic a, dynamic b}) ->* Never");
   }
 
   void test_leastClosure_simple() {
-    testLeast("UNKNOWN", "Null");
+    testLeast("UNKNOWN", "Never");
   }
 }

pkg/front_end/test/type_inference/type_schema_environment_test_base.dart

@@ -67,11 +67,8 @@ abstract class TypeSchemaEnvironmentTestBase {
   void checkConstraintSolving(String constraint, String expected,
       {required bool grounded}) {
     expect(
-        typeSchemaEnvironment.solveTypeConstraint(
-            parseConstraint(constraint),
-            coreTypes.objectNullableRawType,
-            new NeverType.internal(Nullability.nonNullable),
-            grounded: grounded),
+        typeSchemaEnvironment.solveTypeConstraint(parseConstraint(constraint),
+            topType: coreTypes.objectNullableRawType, grounded: grounded),
         parseType(expected));
   }