[model] Make null-guarded lowerings of cascades type safe

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

Author: chloestefantsova

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

@@ -393,11 +393,48 @@ class NullAwareGuard {
     _inferrer.flowAnalysis.nullAwareAccess_end();
     Expression equalsNull = _inferrer.createEqualsNull(
         _nullAwareFileOffset, createVariableGet(_nullAwareVariable));
+
+    // In case null guards are applied to non-nullable receivers, we still
+    // generate a null-testing conditional expression; although, it is
+    // unnecessary. Moreover, those expressions appear to be not null safe,
+    // since their static type is not nullable, and one of their branches is the
+    // `null` literal. The following is an example of such lowering for the
+    // expression `s?..length`, where `s` is of static type `String`.
+    //
+    //     let final core::String #t1 = s in #t1 == null ?{core::String}
+    //         null :
+    //         block { #t1.{core::String::length}{core::int}; } =>#t1
+    //
+    // Note the static type of the condition expression being the non-nullable
+    // `core::String`, and the then-branch being `null`. In such cases, we
+    // implement the following workaround: replace the `null` literal with the
+    // expression tested for being `null`. With the workaround in place, the
+    // expression `s?..length` is lowered as follows:
+    //
+    //     let final core::String #t1 = s in #t1 == null ?{core::String}
+    //         #t1 :
+    //         block { #t1.{core::String::length}{core::int}; } =>#t1
+    //
+    // This achieves the following:
+    //
+    //   * The conditional expression becomes type-safe.
+    //   * Semantically the tested expression and the null literal evaluate to
+    //     the same result, to the `null` value, so the runtime properties of
+    //     the code don't change.
+    //   * In practice, the `null` literal is dead code for non-nullable
+    //     receivers anyway, so the altered part of the expression won't ever be
+    //     executed.
+    //
+    // TODO(johnniwinther,cstefantsova): Don't generate null-testing expressions
+    // for non-nullable receivers in cascades.
+    Expression typeSafeIfNullBranch =
+        inferredType.nullability == Nullability.nullable
+            ? new NullLiteral()
+            : createVariableGet(_nullAwareVariable);
+    typeSafeIfNullBranch.fileOffset = _nullAwareFileOffset;
+
     ConditionalExpression condition = new ConditionalExpression(
-        equalsNull,
-        new NullLiteral()..fileOffset = _nullAwareFileOffset,
-        nullAwareAction,
-        inferredType)
+        equalsNull, typeSafeIfNullBranch, nullAwareAction, inferredType)
       ..fileOffset = _nullAwareFileOffset;
     return new Let(_nullAwareVariable, condition)
       ..fileOffset = _nullAwareFileOffset;

pkg/front_end/test/spell_checking_list_common.txt

@@ -52,6 +52,7 @@ accurate
 accurately
 achieve
 achieved
+achieves
 act
 acting
 action

pkg/front_end/testcases/nnbd/strictly_non_nullable_warnings.dart.strong.expect

@@ -31,7 +31,7 @@ static extension-member method E|[](lowered final core::String #this, core::int
   return 42;
 static method warning(core::String s, core::List<core::String> l, core::Map<core::String, core::int> m) → dynamic {
   let final core::String #t2 = s in #t2 == null ?{core::int?} null : #t2.{core::String::length}{core::int};
-  let final core::String #t3 = s in #t3 == null ?{core::String} null : block {
+  let final core::String #t3 = s in #t3 == null ?{core::String} #t3 : block {
     #t3.{core::String::length}{core::int};
   } =>#t3;
   let final core::String #t4 = s in #t4 == null ?{core::String} "foo" : #t4;
@@ -75,10 +75,10 @@ static method warning(core::String s, core::List<core::String> l, core::Map<core
   let final core::String #t20 = s in let final core::int #t21 = 42 in self::E|[](#t20, #t21) == null ?{core::int?} self::E|[]=(#t20, #t21, 42) : null;
   let final core::List<core::String> #t22 = l in let final core::int #t23 = 42 in #t22.{core::List::[]}(#t23){(core::int) → core::String} == null ?{core::String?} #t22.{core::List::[]=}(#t23, "foo"){(core::int, core::String) → void} : null;
   let final core::List<core::String> #t24 = l in #t24.{core::List::length}{core::int} == null ?{core::int?} #t24.{core::List::length} = 42 : null;
-  let final core::List<core::String> #t25 = l in #t25 == null ?{core::List<core::String>} null : block {
+  let final core::List<core::String> #t25 = l in #t25 == null ?{core::List<core::String>} #t25 : block {
     #t25.{core::List::length} = 42;
   } =>#t25;
-  let final core::List<core::String> #t26 = l in #t26 == null ?{core::List<core::String>} null : block {
+  let final core::List<core::String> #t26 = l in #t26 == null ?{core::List<core::String>} #t26 : block {
     let final core::List<core::String> #t27 = #t26 in #t27.{core::List::length}{core::int} == null ?{core::int?} #t27.{core::List::length} = 42 : null;
   } =>#t26;
 }

pkg/front_end/testcases/nnbd/strictly_non_nullable_warnings.dart.strong.modular.expect

@@ -31,7 +31,7 @@ static extension-member method E|[](lowered final core::String #this, core::int
   return 42;
 static method warning(core::String s, core::List<core::String> l, core::Map<core::String, core::int> m) → dynamic {
   let final core::String #t2 = s in #t2 == null ?{core::int?} null : #t2.{core::String::length}{core::int};
-  let final core::String #t3 = s in #t3 == null ?{core::String} null : block {
+  let final core::String #t3 = s in #t3 == null ?{core::String} #t3 : block {
     #t3.{core::String::length}{core::int};
   } =>#t3;
   let final core::String #t4 = s in #t4 == null ?{core::String} "foo" : #t4;
@@ -75,10 +75,10 @@ static method warning(core::String s, core::List<core::String> l, core::Map<core
   let final core::String #t20 = s in let final core::int #t21 = 42 in self::E|[](#t20, #t21) == null ?{core::int?} self::E|[]=(#t20, #t21, 42) : null;
   let final core::List<core::String> #t22 = l in let final core::int #t23 = 42 in #t22.{core::List::[]}(#t23){(core::int) → core::String} == null ?{core::String?} #t22.{core::List::[]=}(#t23, "foo"){(core::int, core::String) → void} : null;
   let final core::List<core::String> #t24 = l in #t24.{core::List::length}{core::int} == null ?{core::int?} #t24.{core::List::length} = 42 : null;
-  let final core::List<core::String> #t25 = l in #t25 == null ?{core::List<core::String>} null : block {
+  let final core::List<core::String> #t25 = l in #t25 == null ?{core::List<core::String>} #t25 : block {
     #t25.{core::List::length} = 42;
   } =>#t25;
-  let final core::List<core::String> #t26 = l in #t26 == null ?{core::List<core::String>} null : block {
+  let final core::List<core::String> #t26 = l in #t26 == null ?{core::List<core::String>} #t26 : block {
     let final core::List<core::String> #t27 = #t26 in #t27.{core::List::length}{core::int} == null ?{core::int?} #t27.{core::List::length} = 42 : null;
   } =>#t26;
 }

pkg/front_end/testcases/nnbd/strictly_non_nullable_warnings.dart.strong.transformed.expect

@@ -31,7 +31,7 @@ static extension-member method E|[](lowered final core::String #this, core::int
   return 42;
 static method warning(core::String s, core::List<core::String> l, core::Map<core::String, core::int> m) → dynamic {
   let final core::String #t2 = s in #t2 == null ?{core::int?} null : #t2.{core::String::length}{core::int};
-  let final core::String #t3 = s in #t3 == null ?{core::String} null : block {
+  let final core::String #t3 = s in #t3 == null ?{core::String} #t3 : block {
     #t3.{core::String::length}{core::int};
   } =>#t3;
   let final core::String #t4 = s in #t4 == null ?{core::String} "foo" : #t4;
@@ -75,10 +75,10 @@ static method warning(core::String s, core::List<core::String> l, core::Map<core
   let final core::String #t20 = s in let final core::int #t21 = 42 in self::E|[](#t20, #t21) == null ?{core::int?} self::E|[]=(#t20, #t21, 42) : null;
   let final core::List<core::String> #t22 = l in let final core::int #t23 = 42 in #t22.{core::List::[]}(#t23){(core::int) → core::String} == null ?{core::String?} #t22.{core::List::[]=}(#t23, "foo"){(core::int, core::String) → void} : null;
   let final core::List<core::String> #t24 = l in #t24.{core::List::length}{core::int} == null ?{core::int?} #t24.{core::List::length} = 42 : null;
-  let final core::List<core::String> #t25 = l in #t25 == null ?{core::List<core::String>} null : block {
+  let final core::List<core::String> #t25 = l in #t25 == null ?{core::List<core::String>} #t25 : block {
     #t25.{core::List::length} = 42;
   } =>#t25;
-  let final core::List<core::String> #t26 = l in #t26 == null ?{core::List<core::String>} null : block {
+  let final core::List<core::String> #t26 = l in #t26 == null ?{core::List<core::String>} #t26 : block {
     let final core::List<core::String> #t27 = #t26 in #t27.{core::List::length}{core::int} == null ?{core::int?} #t27.{core::List::length} = 42 : null;
   } =>#t26;
 }
@@ -92,4 +92,4 @@ Evaluated: VariableGet @ org-dartlang-testcase:///strictly_non_nullable_warnings
 Evaluated: VariableGet @ org-dartlang-testcase:///strictly_non_nullable_warnings.dart:42:8 -> IntConstant(42)
 Evaluated: VariableGet @ org-dartlang-testcase:///strictly_non_nullable_warnings.dart:43:5 -> IntConstant(42)
 Evaluated: VariableGet @ org-dartlang-testcase:///strictly_non_nullable_warnings.dart:43:5 -> IntConstant(42)
-Extra constant evaluation: evaluated: 175, effectively constant: 6
+Extra constant evaluation: evaluated: 178, effectively constant: 6