[vm] Fix incorrect inference of non-nullability from 'is' checks

Fixes #40792
Fixes #40795

Change-Id: I0af455040e05265a025b1a424c1aac30f3476f95
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/137426
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>

pkg/vm/lib/transformations/type_flow/summary_collector.dart

@@ -1071,6 +1071,21 @@ class SummaryCollector extends RecursiveVisitor<TypeExpr> {
     return narrow;
   }
 
+  // Narrow type of [arg] after successful 'is' test against [type].
+  TypeExpr _makeNarrowAfterSuccessfulIsCheck(TypeExpr arg, DartType type) {
+    // 'x is type' can succeed for null if type is
+    //  - a top type (dynamic, void, Object? or Object*)
+    //  - nullable (including Null)
+    //  - a type parameter (it can be instantiated with Null)
+    //  - legacy Never
+    final nullability = type.nullability;
+    final bool canBeNull = _environment.isTop(type) ||
+        nullability == Nullability.nullable ||
+        type is TypeParameterType ||
+        (type is NeverType && nullability == Nullability.legacy);
+    return _makeNarrow(arg, _typesBuilder.fromStaticType(type, canBeNull));
+  }
+
   // Add an artificial use of given expression in order to make it possible to
   // infer its type even if it is not used in a summary.
   void _addUse(TypeExpr arg) {
@@ -1303,8 +1318,8 @@ class SummaryCollector extends RecursiveVisitor<TypeExpr> {
       _addUse(_visit(operand));
       final int varIndex = _variablesInfo.varIndex[operand.variable];
       if (_variableCells[varIndex] == null) {
-        trueState[varIndex] = _makeNarrow(
-            _visit(operand), _typesBuilder.fromStaticType(node.type, false));
+        trueState[varIndex] =
+            _makeNarrowAfterSuccessfulIsCheck(_visit(operand), node.type);
       }
       _variableValues = null;
       return;
@@ -1805,8 +1820,7 @@ class SummaryCollector extends RecursiveVisitor<TypeExpr> {
 
     if ((node.promotedType != null) &&
         (node.promotedType != const DynamicType())) {
-      return _makeNarrow(
-          v, _typesBuilder.fromStaticType(node.promotedType, false));
+      return _makeNarrowAfterSuccessfulIsCheck(v, node.promotedType);
     }
 
     return v;

runtime/vm/compiler/backend/type_propagator.cc

@@ -413,9 +413,11 @@ void FlowGraphTypePropagator::VisitBranch(BranchInstr* instr) {
       type = &(instance_of->type());
       left = instance_of->value()->definition();
     }
-    if (!type->IsDynamicType() && !type->IsObjectType()) {
-      const bool is_nullable = type->IsNullType() ? CompileType::kNullable
-                                                  : CompileType::kNonNullable;
+    if (!type->IsTopType()) {
+      const bool is_nullable = (type->IsNullable() || type->IsTypeParameter() ||
+                                (type->IsNeverType() && type->IsLegacy()))
+                                   ? CompileType::kNullable
+                                   : CompileType::kNonNullable;
       EnsureMoreAccurateRedefinition(
           true_successor, left,
           CompileType::FromAbstractType(*type, is_nullable));

tests/language_2/vm/regress_40792_test.dart

@@ -0,0 +1,129 @@
+// Copyright (c) 2020, the Dart project authors.  Please see the AUTHORS file
+// 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.
+
+// VMOptions=--optimization_counter_threshold=10 --deterministic
+
+// Regression test for https://dartbug.com/40792 and https://dartbug.com/40795.
+// Verifies that non-nullability is not inferred from 'is' tests which
+// accept null.
+
+import "package:expect/expect.dart";
+
+dynamic result;
+
+// Use separate functions so their parameter types can be inferred separately.
+setResult1(x) {
+  result = (x == null) ? 'null' : 'not null';
+}
+
+setResult2(x) {
+  result = (x == null) ? 'null' : 'not null';
+}
+
+setResult3(x) {
+  result = (x == null) ? 'null' : 'not null';
+}
+
+setResult4(x) {
+  result = (x == null) ? 'null' : 'not null';
+}
+
+setResult5(x) {
+  result = (x == null) ? 'null' : 'not null';
+}
+
+setResult6(x) {
+  result = (x == null) ? 'null' : 'not null';
+}
+
+setResult7(x) {
+  result = (x == null) ? 'null' : 'not null';
+}
+
+class A<S, T extends S> {
+  @pragma('vm:never-inline')
+  test1(S x) {
+    if (x is T) {
+      setResult1(x);
+    }
+  }
+
+  @pragma('vm:never-inline')
+  test2(S x) {
+    if (x is T) {
+      setResult2(x);
+    }
+  }
+
+  @pragma('vm:never-inline')
+  test3(S x) {
+    if (x is T) {
+      setResult2(x);
+    }
+  }
+
+  @pragma('vm:never-inline')
+  test4(S x) {
+    if (x is T) {
+      setResult2(x);
+    }
+  }
+}
+
+@pragma('vm:never-inline')
+test5(x) {
+  if (x is Null) {
+    setResult5(x);
+  }
+}
+
+@pragma('vm:never-inline')
+test6(x) {
+  if (x is Object) {
+    setResult6(x);
+  }
+}
+
+@pragma('vm:never-inline')
+test7(x) {
+  if (x is dynamic) {
+    setResult7(x);
+  }
+}
+
+void doTests() {
+  result = 'unexpected';
+  new A<Null, Null>().test1(null);
+  Expect.equals('null', result);
+
+  result = 'unexpected';
+  new A<Object, Object>().test2(null);
+  Expect.equals('null', result);
+
+  result = 'unexpected';
+  new A<dynamic, dynamic>().test3(null);
+  Expect.equals('null', result);
+
+  result = 'unexpected';
+  new A<void, void>().test4(null);
+  Expect.equals('null', result);
+
+  result = 'unexpected';
+  test5(null);
+  Expect.equals('null', result);
+
+  result = 'unexpected';
+  test6(null);
+  Expect.equals('null', result);
+
+  result = 'unexpected';
+  test7(null);
+  Expect.equals('null', result);
+}
+
+main(List<String> args) {
+  for (int i = 0; i < 20; ++i) {
+    doTests();
+  }
+}