Flow analysis: clarify behavior of ifNullExpression_rightBegin.
Previously, it was unclear what information was stored in
_IfNullExpressionContext._previous. It turned out that it was the
"shortcut state", i.e. the flow state if the LHS of the `??` was
non-`null`. I've renamed variables and added comments to clarify
this. Also, based on the implementation of
`ifNullExpression_rightBegin`, it looked like it might update
`_current`, but in fact it did not (because `tryMarkNonNullable`
always returns an `ExpressionInfo` whose `ifFalse` is unchanged); I've
refactored the code so that this is clearer, and added a comment
explaining why it is the right thing to do.
Bug: https://github.com/dart-lang/language/issues/2020
Change-Id: Ib8a5235f9b9482f4b290ad90686385514660a95d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/251142
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
diff --git a/pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis.dart b/pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis.dart
index 57df318..6a287a8 100644
--- a/pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis.dart
+++ b/pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis.dart
@@ -3734,7 +3734,7 @@
void ifNullExpression_end() {
_IfNullExpressionContext<Type> context =
_stack.removeLast() as _IfNullExpressionContext<Type>;
- _current = _merge(_current, context._previous);
+ _current = _merge(_current, context._shortcutState);
}
@override
@@ -3742,17 +3742,30 @@
Expression leftHandSide, Type leftHandSideType) {
ReferenceWithType<Type>? lhsReference =
_getExpressionReference(leftHandSide);
- FlowModel<Type> promoted;
+ FlowModel<Type> shortcutState;
_current = _current.split();
if (lhsReference != null) {
- ExpressionInfo<Type> promotionInfo = _current.tryMarkNonNullable(
- operations, lhsReference, _promotionKeyStore);
- _current = promotionInfo.ifFalse;
- promoted = promotionInfo.ifTrue;
+ shortcutState = _current
+ .tryMarkNonNullable(operations, lhsReference, _promotionKeyStore)
+ .ifTrue;
} else {
- promoted = _current;
+ shortcutState = _current;
}
- _stack.add(new _IfNullExpressionContext<Type>(promoted));
+ _stack.add(new _IfNullExpressionContext<Type>(shortcutState));
+ // Note: we are now on the RHS of the `??`, and so at this point in the
+ // flow, it is known that the LHS evaluated to `null`. It's tempting to
+ // update `_current` to reflect this (either promoting the type of the LHS,
+ // if it's a variable reference, or marking the flow as unreachable, if the
+ // LHS had a non-nullable static type). However:
+ // - In the case where the LHS was a variable reference, we can't promote
+ // it, because we don't promote to `Null` (see
+ // https://github.com/dart-lang/language/issues/1505#issuecomment-975706918)
+ // - In the case where the LHS had a non-nullable static type, it still
+ // might have been `null` due to mixed-mode unsoundness, so we can't mark
+ // the flow as unreachable without allowing the unsoundness to escalate
+ // (see https://github.com/dart-lang/language/issues/1143)
+ //
+ // So we just leave `_current` as is.
}
@override
@@ -4372,12 +4385,15 @@
}
/// [_FlowContext] representing an "if-null" (`??`) expression.
-class _IfNullExpressionContext<Type extends Object>
- extends _SimpleContext<Type> {
- _IfNullExpressionContext(super.previous);
+class _IfNullExpressionContext<Type extends Object> extends _FlowContext {
+ /// The state if the operation short-cuts (i.e. if the expression before the
+ /// `??` was non-`null`).
+ final FlowModel<Type> _shortcutState;
+
+ _IfNullExpressionContext(this._shortcutState);
@override
- String toString() => '_IfNullExpressionContext(previous: $_previous)';
+ String toString() => '_IfNullExpressionContext($_shortcutState)';
}
/// Contextual information tracked by legacy type promotion about a binary "and"
@@ -5002,9 +5018,7 @@
abstract class _SimpleContext<Type extends Object> extends _FlowContext {
/// The stored state. For a `try` statement, this is the state from the
/// beginning of the `try` block. For a function expression, this is the
- /// state at the point the function expression was created. For an "if-null"
- /// expression, this is the state after execution of the expression before the
- /// `??`.
+ /// state at the point the function expression was created.
final FlowModel<Type> _previous;
_SimpleContext(this._previous);
diff --git a/pkg/front_end/test/spell_checking_list_code.txt b/pkg/front_end/test/spell_checking_list_code.txt
index 48d0dcd..3ce420a 100644
--- a/pkg/front_end/test/spell_checking_list_code.txt
+++ b/pkg/front_end/test/spell_checking_list_code.txt
@@ -290,6 +290,7 @@
customizable
customized
cut
+cuts
cwd
cyclical
cyclically
@@ -437,6 +438,7 @@
erase
erased
es
+escalate
establish
established
estimate
