[flow analysis] Allow non-cascaded field accesses to participate in field promotion.

The way this is accomplished is that in
`_FlowAnalysisImpl.nullAwareAccess_rightBegin`, any expression
reference associated with the target of the null-aware access is
restored, and the corresponding SSA node is associated with the guard
variable (if any). These changes ensure that if the null-aware access
is a property get, the subsequent call to `propertyGet` will pick up
the appropriate SSA node, so it will be able to locate the promotion
key for the property.

This functionality is only enabled when the language feature
`sound-flow-analysis` is enabled.

To prevent test regressions, a few related changes need to be made at
the same time:

- `_FlowAnalysisImpl.nullAwareAccess_end` is changed so that it clears
  any expression info or expression reference that was associated with
  the null-aware access expression. This prevents flow analysis
  information from being erroneously propagated out of a null-aware
  expression, which would have led to assertion failures when
  analyzing null-aware expressions inside of conditional
  expressions. This wasn't previously a problem because the expression
  reference used to be consumed by
  `_FlowAnalysisImpl.nullAwareAccess_rightBegin`, preventing further
  expression references and expression infos from being recorded
  further along in the null-aware access.

- The test framework in `mini_ast.dart` is fixed so that `!` is
  considered to participate in null shorting. This was a bug in the
  test framework that wasn't previously caught because it happened not
  to produce any test failures.

- The analyzer's method `PostfixExpressionResolver._resolveNullCheck`
  is changed so that it calls `nonNullAssert_end` before terminating
  null-aware access. Previously, the order was swapped, causing
  `nullAwareAccess_end` to be called before `nonNullAssert_end` when
  analyzing expressions like `a?.b!`. This used to be benign, but now
  that non-cascaded field accesses participate in field promotion,
  flow analysis needs the methods to be called in the correct order.

Fixes dart-lang/language#4344.

Bug: dart-lang/language#4344
Change-Id: I523be1b4be1af3f68654a745187a546728c878fe
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/427820
Commit-Queue: Paul Berry <[email protected]>
Reviewed-by: Konstantin Shcheglov <[email protected]>

Author: stereotype441

pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis.dart

@@ -5926,6 +5926,11 @@ class _FlowAnalysisImpl<
     _NullAwareAccessContext<Type> context =
         _stack.removeLast() as _NullAwareAccessContext<Type>;
     _current = _join(_current, context._previous).unsplit();
+    // If any expression info or expression reference was stored for the
+    // null-aware expression, it was only valid in the case where the target
+    // expression was not null. So it needs to be cleared now.
+    _expressionInfo = null;
+    _expressionReference = null;
   }
 
   @override
@@ -5955,6 +5960,15 @@ class _FlowAnalysisImpl<
         break;
     }
     _stack.add(new _NullAwareAccessContext<Type>(shortcutControlPath));
+    SsaNode<Type>? targetSsaNode;
+    if (typeAnalyzerOptions.soundFlowAnalysisEnabled) {
+      // Store back the target reference so that it can be used for field
+      // promotion.
+      if (target != null && targetReference != null) {
+        _storeExpressionReference(target, targetReference);
+        targetSsaNode = targetReference.ssaNode;
+      }
+    }
     if (guardVariable != null) {
       // Promote the guard variable as well.
       int promotionKey = promotionKeyStore.keyForVariable(guardVariable);
@@ -5967,7 +5981,7 @@ class _FlowAnalysisImpl<
           tested: const [],
           assigned: true,
           unassigned: false,
-          ssaNode: new SsaNode(null),
+          ssaNode: targetSsaNode ?? new SsaNode(null),
         ),
       );
     }

pkg/_fe_analyzer_shared/test/flow_analysis/flow_analysis_test.dart

@@ -11885,6 +11885,109 @@ main() {
         ]);
       });
     });
+
+    group('Null aware field access:', () {
+      group('Non-cascaded:', () {
+        test('When disabled, does not see previous promotions', () {
+          h.disableSoundFlowAnalysis();
+          h.addMember('A', '_i', 'int?', promotable: true);
+          var a = Var('a');
+          h.run([
+            declare(a, initializer: expr('A')),
+            a.property('_i').nonNullAssert,
+            // `a._i` is promoted now.
+            a.property('_i').checkType('int'),
+            // But `a?._i` is not.
+            a.property('_i', isNullAware: true).checkType('int?'),
+          ]);
+        });
+
+        test('When enabled, sees previous promotions', () {
+          h.addMember('A', '_i', 'int?', promotable: true);
+          var a = Var('a');
+          h.run([
+            declare(a, initializer: expr('A')),
+            a.property('_i').nonNullAssert,
+            // `a._i` is promoted now.
+            a.property('_i').checkType('int'),
+            // And so is `a?._i`.
+            a.property('_i', isNullAware: true).checkType('int'),
+          ]);
+        });
+
+        test('When disabled, cannot promote', () {
+          h.disableSoundFlowAnalysis();
+          h.addMember('A', '_i', 'int?', promotable: true);
+          var a = Var('a');
+          h.run([
+            declare(a, initializer: expr('A')),
+            a.property('_i', isNullAware: true).nonNullAssert,
+            // `a._i` is not promoted.
+            a.property('_i').checkType('int?'),
+            // But had the field access not been null aware, it would have been
+            // promoted.
+            a.property('_i').nonNullAssert,
+            a.property('_i').checkType('int'),
+          ]);
+        });
+
+        test('When enabled, can promote', () {
+          h.addMember('A', '_i', 'int?', promotable: true);
+          var a = Var('a');
+          h.run([
+            declare(a, initializer: expr('A')),
+            a.property('_i').checkType('int?'),
+            a.property('_i', isNullAware: true).nonNullAssert,
+            // `a._i` is promoted.
+            a.property('_i').checkType('int'),
+          ]);
+        });
+
+        test('In conditional expression', () {
+          h.addMember('A', '_i', 'int?', promotable: true);
+          var a = Var('a');
+          h.run([
+            declare(a, initializer: expr('A')),
+            expr(
+              'bool',
+            ).conditional(nullLiteral, a.property('_i', isNullAware: true)),
+          ]);
+        });
+      });
+
+      group('Cascaded:', () {
+        test('When disabled, sees previous promotions', () {
+          h.disableSoundFlowAnalysis();
+          h.addMember('A', '_i', 'int?', promotable: true);
+          var a = Var('a');
+          h.run([
+            declare(a, initializer: expr('A')),
+            a.property('_i').nonNullAssert,
+            // `a._i` is promoted now.
+            a.property('_i').checkType('int'),
+            // And `a?.._i` is promoted.
+            a.cascade(isNullAware: true, [
+              (placeholder) => placeholder.property('_i').checkType('int'),
+            ]),
+          ]);
+        });
+
+        test('When enabled, sees previous promotions', () {
+          h.addMember('A', '_i', 'int?', promotable: true);
+          var a = Var('a');
+          h.run([
+            declare(a, initializer: expr('A')),
+            a.property('_i').nonNullAssert,
+            // `a._i` is promoted now.
+            a.property('_i').checkType('int'),
+            // And `a?.._i` is promoted.
+            a.cascade(isNullAware: true, [
+              (placeholder) => placeholder.property('_i').checkType('int'),
+            ]),
+          ]);
+        });
+      });
+    });
   });
 }
 

pkg/_fe_analyzer_shared/test/mini_ast.dart

@@ -6520,7 +6520,11 @@ class _MiniAstTypeAnalyzer
     Expression node,
     Expression expression,
   ) {
-    var type = analyzeExpression(expression, operations.unknownType);
+    var type = analyzeExpression(
+      expression,
+      operations.unknownType,
+      continueNullShorting: true,
+    );
     flow.nonNullAssert_end(expression);
     return new ExpressionTypeAnalysisResult(
       type: flow.operations.promoteToNonNull(type),

pkg/analyzer/lib/src/dart/resolver/postfix_expression_resolver.dart

@@ -235,7 +235,7 @@ class PostfixExpressionResolver {
     var type = _typeSystem.promoteToNonNull(operandType);
     node.recordStaticType(type, resolver: _resolver);
 
-    _resolver.nullShortingTermination(node);
     _resolver.flowAnalysis.flow?.nonNullAssert_end(operand);
+    _resolver.nullShortingTermination(node);
   }
 }

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

@@ -9637,6 +9637,9 @@ class InferenceVisitorImpl extends InferenceVisitorBase
       if (nonNullableType != variable.type) {
         promotedType = nonNullableType;
       }
+      // It's still necessary to inform flow analysis about the read so that it
+      // can track field promotions.
+      flowAnalysis.variableRead(node, variable);
     } else if (!variable.isLocalFunction) {
       // Don't promote local functions.
       promotedType =

tests/language/inference_update_2/cascaded_field_promotion_unnecessary_null_aware_error_test.dart

@@ -53,11 +53,10 @@ void fieldsPromotableWithinCascade(C c) {
     .._field!.expectStaticType<Exactly<Object>>()
     .._field.expectStaticType<Exactly<Object>>();
 
-  // After the cascade, the promotion is not retained, because of the implicit
-  // control flow join implied by the `?..`. (In principle it would be sound to
-  // preserve the promotion, but it's extra work to do so, and it's not clear
-  // that there would be enough user benefit to justify the work).
-  c?._field.expectStaticType<Exactly<Object?>>();
+  // After the cascade, the promotion is retained, because in the implicit
+  // control flow join implied by the `?..`, the control flow path that skips
+  // the promotion is dead.
+  c?._field.expectStaticType<Exactly<Object>>();
   // [error column 4, length 2]
   // [analyzer] STATIC_WARNING.INVALID_NULL_AWARE_OPERATOR
 }

tests/language/sound_flow_analysis/null_aware_field_access_test.dart

@@ -0,0 +1,83 @@
+// Copyright (c) 2025, 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.
+
+// Tests the interactions between field promotion and various forms of
+// null-aware accesses, when `sound-flow-analysis` is enabled.
+
+// Note that there is (as of this writing) no specification for field promotion;
+// this test reflects the current behavior of the `sound-flow-analysis` feature,
+// as discussed in https://github.com/dart-lang/language/issues/4344.
+
+// SharedOptions=--enable-experiment=sound-flow-analysis
+
+// ignore_for_file: invalid_null_aware_operator
+
+import 'package:expect/static_type_helper.dart';
+
+class A {
+  final int? _i;
+  A(this._i);
+}
+
+// Inside a non-cascaded null-aware field access, prior promotions take effect.
+void nonCascadedPriorPromotion(A a) {
+  if (a._i != null) {
+    // `a._i` is promoted now.
+    a._i.expectStaticType<Exactly<int>>();
+    // And so is `a?._i`.
+    a?._i.expectStaticType<Exactly<int>>();
+  }
+}
+
+// A promotion that occurs inside a non-cascaded null-aware field access
+// produces a continued effect after the field access.
+void nonCascadedPromotionAfter(A a) {
+  a?._i!;
+  // `a._i` is now promoted.
+  a._i.expectStaticType<Exactly<int>>();
+  // And so is `a?._i`.
+  a?._i.expectStaticType<Exactly<int>>();
+}
+
+// Inside a cascaded null-aware field access, prior promotions take effect.
+void cascadedPriorPromotion(A a) {
+  if (a._i != null) {
+    // `a._i` is promoted now.
+    a._i.expectStaticType<Exactly<int>>();
+    // And `a?.._i` is promoted.
+    a?.._i.expectStaticType<Exactly<int>>();
+  }
+}
+
+// A promotion that occurs inside a cascaded null-aware field access produces a
+// continued effect after the cascade (assuming the target is non-nullable).
+void cascadedPromotionAfter(A a) {
+  // `a._i` is promoted inside the cascade
+  a
+    ?.._i!
+    .._i.expectStaticType<Exactly<int>>();
+  // And the promotion is retained afterwards.
+  a._i.expectStaticType<Exactly<int>>();
+}
+
+// A promotion that occurs inside a cascaded null-aware field access produces an
+// effect on the value of the cascade expression (assuming the target is
+// non-nullable).
+void cascadedPromotionValue(A a) {
+  // `a._i` is promoted inside the cascade
+  (a
+        ?.._i!
+        .._i.expectStaticType<Exactly<int>>())
+      // And the promotion is retained in the value of the cascade expression.
+      ._i
+      .expectStaticType<Exactly<int>>();
+}
+
+main() {
+  nonCascadedPriorPromotion(A(0));
+  nonCascadedPromotionAfter(A(0));
+  cascadedPriorPromotion(A(0));
+  cascadedPromotionAfter(A(0));
+  cascadedPromotionValue(A(0));
+}