Skip to content

Commit 569c901

Browse files
stereotype441Commit Queue
authored andcommitted
[sound flow analysis] Implement behaviors for equality comparisons.
This change updates the flow analysis logic for `==` and `!=` expressions, `==` and `!=` patterns, and constant patterns, so that when the language feature `sound-flow-analysis` is enabled, the static types of the two expressions being compared are checked for nullability. If one of the types is non-nullable and the other type is `Null`, then it is known that the values will be unequal. Note that these new behaviors break assumptions made by several pre-existing flow analysis tests. I was able to adjust some of the tests to preserve their old behavior, either by adjusting expectations or running the test with `sound-flow-analysis` disabled. Some other tests became redundant, so I removed them. There is no behavioral change if the feature `sound-flow-analysis` is disabled. Bug: #60438 Change-Id: Ib65477e064bb8dcd761542ebe187843fe265a24b Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/420463 Commit-Queue: Paul Berry <[email protected]> Reviewed-by: Chloe Stefantsova <[email protected]>
1 parent a9313bf commit 569c901

File tree

2 files changed

+271
-94
lines changed

2 files changed

+271
-94
lines changed

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

Lines changed: 44 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4693,6 +4693,11 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
46934693
// whole expression behaves equivalently to a boolean (either `true` or
46944694
// `false` depending whether the check uses the `!=` operator).
46954695
booleanLiteral(wholeExpression, !notEqual);
4696+
case _GuaranteedNotEqual():
4697+
// Both operands are known by flow analysis to compare unequal, so the
4698+
// whole expression behaves equivalently to a boolean (either `true` or
4699+
// `false` depending whether the check uses the `!=` operator).
4700+
booleanLiteral(wholeExpression, notEqual);
46964701

46974702
// SAFETY: we can assume `reference` is a `_Reference<Type>` because we
46984703
// require clients not to mix data obtained from different
@@ -5770,10 +5775,15 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
57705775
TypeClassification.nullOrEquivalent &&
57715776
leftOperandTypeClassification == TypeClassification.nonNullable)) {
57725777
// In strong mode the test is guaranteed to produce a "not equal" result,
5773-
// but weak mode it might produce an "equal" result. We don't want flow
5774-
// analysis behavior to depend on mode, so we conservatively assume that
5775-
// either result is possible.
5776-
return const _NoEqualityInformation();
5778+
// but weak mode it might produce an "equal" result. If sound flow
5779+
// analysis is enabled, we assume that the user isn't running in weak mode
5780+
// and so we propagate the known "not equal" result. Otherwise, we
5781+
// conservatively assume that either result is possible.
5782+
if (typeAnalyzerOptions.soundFlowAnalysisEnabled) {
5783+
return const _GuaranteedNotEqual();
5784+
} else {
5785+
return const _NoEqualityInformation();
5786+
}
57775787
} else if (lhsInfo != null && lhsInfo.isNull) {
57785788
return new _EqualityCheckIsNullCheck(
57795789
rhsInfo is _Reference<Type> ? rhsInfo : null,
@@ -6012,16 +6022,30 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
60126022
case _GuaranteedEqual():
60136023
if (notEqual) {
60146024
// Both operands are known by flow analysis to compare equal, so the
6015-
// constant pattern is guaranteed *not* to match.
6025+
// pattern is guaranteed *not* to match.
60166026
_unmatched = _join(_unmatched!, _current);
60176027
_current = _current.setUnreachable();
60186028
} else {
60196029
// Both operands are known by flow analysis to compare equal, so the
6020-
// constant pattern is guaranteed to match. Since our approach to
6021-
// handling patterns in flow analysis uses "implicit and" semantics
6022-
// (initially assuming that the pattern always matches, and then
6023-
// updating the `_current` and `_unmatched` states to reflect what
6024-
// values the pattern rejects), we don't have to do any updates.
6030+
// pattern is guaranteed to match. Since our approach to handling
6031+
// patterns in flow analysis uses "implicit and" semantics (initially
6032+
// assuming that the pattern always matches, and then updating the
6033+
// `_current` and `_unmatched` states to reflect what values the
6034+
// pattern rejects), we don't have to do any updates.
6035+
}
6036+
case _GuaranteedNotEqual():
6037+
if (notEqual) {
6038+
// Both operands are known by flow analysis to compare unequal, so the
6039+
// pattern is guaranteed to match. Since our approach to handling
6040+
// patterns in flow analysis uses "implicit and" semantics (initially
6041+
// assuming that the pattern always matches, and then updating the
6042+
// `_current` and `_unmatched` states to reflect what values the
6043+
// pattern rejects), we don't have to do any updates.
6044+
} else {
6045+
// Both operands are known by flow analysis to compare unequal, so the
6046+
// pattern is guaranteed *not* to match.
6047+
_unmatched = _join(_unmatched!, _current);
6048+
_current = _current.setUnreachable();
60256049
}
60266050
}
60276051
}
@@ -6382,6 +6406,16 @@ class _GuaranteedEqual extends _EqualityCheckResult {
63826406
const _GuaranteedEqual() : super._();
63836407
}
63846408

6409+
/// Specialization of [_EqualityCheckResult] used as the return value for
6410+
/// [_FlowAnalysisImpl._equalityCheck] when it is determined that the two
6411+
/// operands are guaranteed to be not equal to one another, so the code path
6412+
/// that results from an equal result should be marked as unreachable. (This
6413+
/// happens if one operands has type `Null` and the other has a non-nullable
6414+
/// type, and [TypeAnalyzerOptions.soundFlowAnalysisEnabled] is `true`).
6415+
class _GuaranteedNotEqual extends _EqualityCheckResult {
6416+
const _GuaranteedNotEqual() : super._();
6417+
}
6418+
63856419
/// [_FlowContext] representing an `if` statement.
63866420
class _IfContext<Type extends Object> extends _BranchContext<Type> {
63876421
/// Flow model associated with the state of program execution after the `if`

0 commit comments

Comments
 (0)