[flow analysis] Rework _EqualityCheckResult using patterns.
The class `_EqualityCheckResult`, and its subtypes, were introduced
prior to support for patterns. This change makes
`_EqualityCheckResult` into a sealed class and changes the logic that
consumes it to use `switch` statements rather than `is` tests. The
resulting logic is equivalent, but I believe it's easier to read.
This paves the way for adding a new kind of `_EqualityCheckResult`,
which I'll need to do to support the new `sound-flow-analysis`
feature. (The new kind of `_EqualityCheckResult` will be used in the
circumstance where the two operands of an equality comparison are
guaranteed to be unequal because of their types).
Bug: https://github.com/dart-lang/sdk/issues/60438
Change-Id: I217da37847beff5a43bddb5ed734cc3e4c74970a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/420184
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@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 83b2ce8..d6d5f44 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
@@ -4290,7 +4290,7 @@
/// Result of performing equality check. This class is used as the return value
/// for [_FlowAnalysisImpl._equalityCheck].
-abstract class _EqualityCheckResult {
+sealed class _EqualityCheckResult {
const _EqualityCheckResult._();
}
@@ -4686,30 +4686,33 @@
// information about legacy operands. But since we are currently in full
// (post null safety) flow analysis logic, we can safely assume that they
// are not null.
- _EqualityCheckResult equalityCheckResult = _equalityCheck(
- leftOperandInfo, leftOperandType, rightOperandInfo, rightOperandType);
- if (equalityCheckResult is _GuaranteedEqual) {
- // Both operands are known by flow analysis to compare equal, so the whole
- // expression behaves equivalently to a boolean (either `true` or `false`
- // depending whether the check uses the `!=` operator).
- booleanLiteral(wholeExpression, !notEqual);
- } else if (equalityCheckResult is _EqualityCheckIsNullCheck<Type>) {
- _Reference<Type>? reference = equalityCheckResult.reference;
- if (reference == null) {
- // One side of the equality check is `null`, but the other side is not a
- // promotable reference. So there's no promotion to do.
- return;
- }
- // The equality check is a null check of something potentially promotable
- // (e.g. a local variable). Record the necessary information so that if
- // this null check winds up being used for a conditional branch, the
- // variable's will be promoted on the appropriate code path.
- ExpressionInfo<Type> equalityInfo =
- _current.tryMarkNonNullable(this, reference);
- _storeExpressionInfo(
- wholeExpression, notEqual ? equalityInfo : equalityInfo._invert());
- } else {
- assert(equalityCheckResult is _NoEqualityInformation);
+ switch (_equalityCheck(
+ leftOperandInfo, leftOperandType, rightOperandInfo, rightOperandType)) {
+ case _GuaranteedEqual():
+ // Both operands are known by flow analysis to compare equal, so the
+ // whole expression behaves equivalently to a boolean (either `true` or
+ // `false` depending whether the check uses the `!=` operator).
+ booleanLiteral(wholeExpression, !notEqual);
+
+ // SAFETY: we can assume `reference` is a `_Reference<Type>` because we
+ // require clients not to mix data obtained from different
+ // instantiations of `FlowAnalysis`.
+ case _EqualityCheckIsNullCheck(:var reference as _Reference<Type>?):
+ if (reference == null) {
+ // One side of the equality check is `null`, but the other side is not
+ // a promotable reference. So there's no promotion to do.
+ return;
+ }
+ // The equality check is a null check of something potentially
+ // promotable (e.g. a local variable). Record the necessary information
+ // so that if this null check winds up being used for a conditional
+ // branch, the variable's will be promoted on the appropriate code path.
+ ExpressionInfo<Type> equalityInfo =
+ _current.tryMarkNonNullable(this, reference);
+ _storeExpressionInfo(
+ wholeExpression, notEqual ? equalityInfo : equalityInfo._invert());
+
+ case _NoEqualityInformation():
// Since flow analysis can't garner any information from this equality
// check, nothing needs to be done; by not calling `_storeExpressionInfo`,
// we ensure that if `_getExpressionInfo` is later called on this
@@ -5958,69 +5961,68 @@
_Reference<Type> newReference = context
.createReference(matchedValueType, _current)
.addPreviousInfo(context._matchedValueInfo, this, _current);
- _EqualityCheckResult equalityCheckResult = _equalityCheck(newReference,
- matchedValueType, _getExpressionInfo(operand), operandType);
- if (equalityCheckResult is _NoEqualityInformation) {
- // We have no information so we have to assume the pattern might or
- // might not match.
- _unmatched = _join(_unmatched!, _current);
- } else if (equalityCheckResult is _EqualityCheckIsNullCheck<Type>) {
- FlowModel<Type>? ifNotNull;
- if (!equalityCheckResult.isReferenceOnRight) {
- // The `null` literal is on the right hand side of the implicit
- // equality check, meaning it is the constant value. So the user is
- // doing something like this:
- //
- // if (v case == null) { ... }
- //
- // So we want to promote the type of `v` in the case where the
- // constant pattern *didn't* match.
- ifNotNull = _nullCheckPattern(matchedValueType: matchedValueType);
- if (ifNotNull == null) {
- // `_nullCheckPattern` returns `null` in the case where the matched
- // value type is non-nullable. In fully sound programs, this would
- // mean that the pattern cannot possibly match. However, in mixed
- // mode programs it might match due to unsoundness. Since we don't
- // want type inference results to change when a program becomes
- // fully sound, we have to assume that we're in mixed mode, and thus
- // the pattern might match.
+ switch (_equalityCheck(newReference, matchedValueType,
+ _getExpressionInfo(operand), operandType)) {
+ case _NoEqualityInformation():
+ // We have no information so we have to assume the pattern might or
+ // might not match.
+ _unmatched = _join(_unmatched!, _current);
+ case _EqualityCheckIsNullCheck<Object>(:var isReferenceOnRight):
+ FlowModel<Type>? ifNotNull;
+ if (!isReferenceOnRight) {
+ // The `null` literal is on the right hand side of the implicit
+ // equality check, meaning it is the constant value. So the user is
+ // doing something like this:
+ //
+ // if (v case == null) { ... }
+ //
+ // So we want to promote the type of `v` in the case where the
+ // constant pattern *didn't* match.
+ ifNotNull = _nullCheckPattern(matchedValueType: matchedValueType);
+ if (ifNotNull == null) {
+ // `_nullCheckPattern` returns `null` in the case where the matched
+ // value type is non-nullable. In fully sound programs, this would
+ // mean that the pattern cannot possibly match. However, in mixed
+ // mode programs it might match due to unsoundness. Since we don't
+ // want type inference results to change when a program becomes
+ // fully sound, we have to assume that we're in mixed mode, and thus
+ // the pattern might match.
+ ifNotNull = _current;
+ }
+ } else {
+ // The `null` literal is on the left hand side of the implicit
+ // equality check, meaning it is the scrutinee. So the user is doing
+ // something silly like this:
+ //
+ // if (null case == c) { ... }
+ //
+ // (where `c` is some constant). There's no variable to promote.
+ //
+ // Since flow analysis can't make use of the results of constant
+ // evaluation, we can't really assume anything; as far as we know, the
+ // pattern might or might not match.
ifNotNull = _current;
}
- } else {
- // The `null` literal is on the left hand side of the implicit
- // equality check, meaning it is the scrutinee. So the user is doing
- // something silly like this:
- //
- // if (null case == c) { ... }
- //
- // (where `c` is some constant). There's no variable to promote.
- //
- // Since flow analysis can't make use of the results of constant
- // evaluation, we can't really assume anything; as far as we know, the
- // pattern might or might not match.
- ifNotNull = _current;
- }
- if (notEqual) {
- _unmatched = _join(_unmatched!, _current);
- _current = ifNotNull;
- } else {
- _unmatched = _join(_unmatched!, ifNotNull);
- }
- } else {
- assert(equalityCheckResult is _GuaranteedEqual);
- if (notEqual) {
- // Both operands are known by flow analysis to compare equal, so the
- // constant pattern is guaranteed *not* to match.
- _unmatched = _join(_unmatched!, _current);
- _current = _current.setUnreachable();
- } else {
- // Both operands are known by flow analysis to compare equal, so the
- // constant pattern is guaranteed to match. Since our approach to
- // handling patterns in flow analysis uses "implicit and" semantics
- // (initially assuming that the pattern always matches, and then
- // updating the `_current` and `_unmatched` states to reflect what
- // values the pattern rejects), we don't have to do any updates.
- }
+ if (notEqual) {
+ _unmatched = _join(_unmatched!, _current);
+ _current = ifNotNull;
+ } else {
+ _unmatched = _join(_unmatched!, ifNotNull);
+ }
+ case _GuaranteedEqual():
+ if (notEqual) {
+ // Both operands are known by flow analysis to compare equal, so the
+ // constant pattern is guaranteed *not* to match.
+ _unmatched = _join(_unmatched!, _current);
+ _current = _current.setUnreachable();
+ } else {
+ // Both operands are known by flow analysis to compare equal, so the
+ // constant pattern is guaranteed to match. Since our approach to
+ // handling patterns in flow analysis uses "implicit and" semantics
+ // (initially assuming that the pattern always matches, and then
+ // updating the `_current` and `_unmatched` states to reflect what
+ // values the pattern rejects), we don't have to do any updates.
+ }
}
}
