Skip to content

Commit 2d1094a

Browse files
stereotype441Commit Queue
authored and
Commit Queue
committed
[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: #60438 Change-Id: I217da37847beff5a43bddb5ed734cc3e4c74970a Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/420184 Reviewed-by: Konstantin Shcheglov <[email protected]> Commit-Queue: Paul Berry <[email protected]>
1 parent b194e29 commit 2d1094a

File tree

1 file changed

+88
-86
lines changed

1 file changed

+88
-86
lines changed

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

+88-86
Original file line numberDiff line numberDiff line change
@@ -4290,7 +4290,7 @@ class _EqualityCheckIsNullCheck<Type extends Object>
42904290

42914291
/// Result of performing equality check. This class is used as the return value
42924292
/// for [_FlowAnalysisImpl._equalityCheck].
4293-
abstract class _EqualityCheckResult {
4293+
sealed class _EqualityCheckResult {
42944294
const _EqualityCheckResult._();
42954295
}
42964296

@@ -4686,30 +4686,33 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
46864686
// information about legacy operands. But since we are currently in full
46874687
// (post null safety) flow analysis logic, we can safely assume that they
46884688
// are not null.
4689-
_EqualityCheckResult equalityCheckResult = _equalityCheck(
4690-
leftOperandInfo, leftOperandType, rightOperandInfo, rightOperandType);
4691-
if (equalityCheckResult is _GuaranteedEqual) {
4692-
// Both operands are known by flow analysis to compare equal, so the whole
4693-
// expression behaves equivalently to a boolean (either `true` or `false`
4694-
// depending whether the check uses the `!=` operator).
4695-
booleanLiteral(wholeExpression, !notEqual);
4696-
} else if (equalityCheckResult is _EqualityCheckIsNullCheck<Type>) {
4697-
_Reference<Type>? reference = equalityCheckResult.reference;
4698-
if (reference == null) {
4699-
// One side of the equality check is `null`, but the other side is not a
4700-
// promotable reference. So there's no promotion to do.
4701-
return;
4702-
}
4703-
// The equality check is a null check of something potentially promotable
4704-
// (e.g. a local variable). Record the necessary information so that if
4705-
// this null check winds up being used for a conditional branch, the
4706-
// variable's will be promoted on the appropriate code path.
4707-
ExpressionInfo<Type> equalityInfo =
4708-
_current.tryMarkNonNullable(this, reference);
4709-
_storeExpressionInfo(
4710-
wholeExpression, notEqual ? equalityInfo : equalityInfo._invert());
4711-
} else {
4712-
assert(equalityCheckResult is _NoEqualityInformation);
4689+
switch (_equalityCheck(
4690+
leftOperandInfo, leftOperandType, rightOperandInfo, rightOperandType)) {
4691+
case _GuaranteedEqual():
4692+
// Both operands are known by flow analysis to compare equal, so the
4693+
// whole expression behaves equivalently to a boolean (either `true` or
4694+
// `false` depending whether the check uses the `!=` operator).
4695+
booleanLiteral(wholeExpression, !notEqual);
4696+
4697+
// SAFETY: we can assume `reference` is a `_Reference<Type>` because we
4698+
// require clients not to mix data obtained from different
4699+
// instantiations of `FlowAnalysis`.
4700+
case _EqualityCheckIsNullCheck(:var reference as _Reference<Type>?):
4701+
if (reference == null) {
4702+
// One side of the equality check is `null`, but the other side is not
4703+
// a promotable reference. So there's no promotion to do.
4704+
return;
4705+
}
4706+
// The equality check is a null check of something potentially
4707+
// promotable (e.g. a local variable). Record the necessary information
4708+
// so that if this null check winds up being used for a conditional
4709+
// branch, the variable's will be promoted on the appropriate code path.
4710+
ExpressionInfo<Type> equalityInfo =
4711+
_current.tryMarkNonNullable(this, reference);
4712+
_storeExpressionInfo(
4713+
wholeExpression, notEqual ? equalityInfo : equalityInfo._invert());
4714+
4715+
case _NoEqualityInformation():
47134716
// Since flow analysis can't garner any information from this equality
47144717
// check, nothing needs to be done; by not calling `_storeExpressionInfo`,
47154718
// we ensure that if `_getExpressionInfo` is later called on this
@@ -5958,69 +5961,68 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
59585961
_Reference<Type> newReference = context
59595962
.createReference(matchedValueType, _current)
59605963
.addPreviousInfo(context._matchedValueInfo, this, _current);
5961-
_EqualityCheckResult equalityCheckResult = _equalityCheck(newReference,
5962-
matchedValueType, _getExpressionInfo(operand), operandType);
5963-
if (equalityCheckResult is _NoEqualityInformation) {
5964-
// We have no information so we have to assume the pattern might or
5965-
// might not match.
5966-
_unmatched = _join(_unmatched!, _current);
5967-
} else if (equalityCheckResult is _EqualityCheckIsNullCheck<Type>) {
5968-
FlowModel<Type>? ifNotNull;
5969-
if (!equalityCheckResult.isReferenceOnRight) {
5970-
// The `null` literal is on the right hand side of the implicit
5971-
// equality check, meaning it is the constant value. So the user is
5972-
// doing something like this:
5973-
//
5974-
// if (v case == null) { ... }
5975-
//
5976-
// So we want to promote the type of `v` in the case where the
5977-
// constant pattern *didn't* match.
5978-
ifNotNull = _nullCheckPattern(matchedValueType: matchedValueType);
5979-
if (ifNotNull == null) {
5980-
// `_nullCheckPattern` returns `null` in the case where the matched
5981-
// value type is non-nullable. In fully sound programs, this would
5982-
// mean that the pattern cannot possibly match. However, in mixed
5983-
// mode programs it might match due to unsoundness. Since we don't
5984-
// want type inference results to change when a program becomes
5985-
// fully sound, we have to assume that we're in mixed mode, and thus
5986-
// the pattern might match.
5964+
switch (_equalityCheck(newReference, matchedValueType,
5965+
_getExpressionInfo(operand), operandType)) {
5966+
case _NoEqualityInformation():
5967+
// We have no information so we have to assume the pattern might or
5968+
// might not match.
5969+
_unmatched = _join(_unmatched!, _current);
5970+
case _EqualityCheckIsNullCheck<Object>(:var isReferenceOnRight):
5971+
FlowModel<Type>? ifNotNull;
5972+
if (!isReferenceOnRight) {
5973+
// The `null` literal is on the right hand side of the implicit
5974+
// equality check, meaning it is the constant value. So the user is
5975+
// doing something like this:
5976+
//
5977+
// if (v case == null) { ... }
5978+
//
5979+
// So we want to promote the type of `v` in the case where the
5980+
// constant pattern *didn't* match.
5981+
ifNotNull = _nullCheckPattern(matchedValueType: matchedValueType);
5982+
if (ifNotNull == null) {
5983+
// `_nullCheckPattern` returns `null` in the case where the matched
5984+
// value type is non-nullable. In fully sound programs, this would
5985+
// mean that the pattern cannot possibly match. However, in mixed
5986+
// mode programs it might match due to unsoundness. Since we don't
5987+
// want type inference results to change when a program becomes
5988+
// fully sound, we have to assume that we're in mixed mode, and thus
5989+
// the pattern might match.
5990+
ifNotNull = _current;
5991+
}
5992+
} else {
5993+
// The `null` literal is on the left hand side of the implicit
5994+
// equality check, meaning it is the scrutinee. So the user is doing
5995+
// something silly like this:
5996+
//
5997+
// if (null case == c) { ... }
5998+
//
5999+
// (where `c` is some constant). There's no variable to promote.
6000+
//
6001+
// Since flow analysis can't make use of the results of constant
6002+
// evaluation, we can't really assume anything; as far as we know, the
6003+
// pattern might or might not match.
59876004
ifNotNull = _current;
59886005
}
5989-
} else {
5990-
// The `null` literal is on the left hand side of the implicit
5991-
// equality check, meaning it is the scrutinee. So the user is doing
5992-
// something silly like this:
5993-
//
5994-
// if (null case == c) { ... }
5995-
//
5996-
// (where `c` is some constant). There's no variable to promote.
5997-
//
5998-
// Since flow analysis can't make use of the results of constant
5999-
// evaluation, we can't really assume anything; as far as we know, the
6000-
// pattern might or might not match.
6001-
ifNotNull = _current;
6002-
}
6003-
if (notEqual) {
6004-
_unmatched = _join(_unmatched!, _current);
6005-
_current = ifNotNull;
6006-
} else {
6007-
_unmatched = _join(_unmatched!, ifNotNull);
6008-
}
6009-
} else {
6010-
assert(equalityCheckResult is _GuaranteedEqual);
6011-
if (notEqual) {
6012-
// Both operands are known by flow analysis to compare equal, so the
6013-
// constant pattern is guaranteed *not* to match.
6014-
_unmatched = _join(_unmatched!, _current);
6015-
_current = _current.setUnreachable();
6016-
} else {
6017-
// Both operands are known by flow analysis to compare equal, so the
6018-
// constant pattern is guaranteed to match. Since our approach to
6019-
// handling patterns in flow analysis uses "implicit and" semantics
6020-
// (initially assuming that the pattern always matches, and then
6021-
// updating the `_current` and `_unmatched` states to reflect what
6022-
// values the pattern rejects), we don't have to do any updates.
6023-
}
6006+
if (notEqual) {
6007+
_unmatched = _join(_unmatched!, _current);
6008+
_current = ifNotNull;
6009+
} else {
6010+
_unmatched = _join(_unmatched!, ifNotNull);
6011+
}
6012+
case _GuaranteedEqual():
6013+
if (notEqual) {
6014+
// Both operands are known by flow analysis to compare equal, so the
6015+
// constant pattern is guaranteed *not* to match.
6016+
_unmatched = _join(_unmatched!, _current);
6017+
_current = _current.setUnreachable();
6018+
} else {
6019+
// 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.
6025+
}
60246026
}
60256027
}
60266028

0 commit comments

Comments
 (0)