Account for differing flow analysis conventions between CFE and shared code.

With one exception (noted below), the shared analysis logic uses the
convention that the expressions passed to flow analysis are the
original (pre-lowered) expressions, whereas the expressions passed to
flow analysis by the CFE are the lowered expressions. This difference
caused two problems:

- If a boolean expression appeared on the right hand side of a pattern
  assignment or pattern variable declaration, and it was lowered, the
  flow analysis implications of that boolean expression would be lost.

- If a boolean expression appeared in a `when` clause, and it was
  lowered, the flow analysis implications of that boolean expression
  would be lost. Exception: for switch statements, the shared analysis
  logic has been passing lowered expressions to flow analysis, so this
  problem didn't occur for `when` clauses in switch statements.

Notably, when compiling for the VM, the CFE lowers expressions like
`x != null` to something more like `!(x == null)`.

Fortunately, the first of these two situations shouldn't cause
problems very often, since typically if the right hand side of an
assignment or variable declaration is a boolean expression, there is
no need for the left hand side to involve patterns.

As for the second of these two situations, it's also not too likely to
cause problems, since typically null checks occur inside patterns
rather than in `when` clauses.

As a short term fix, we remove the exception noted above, and we
account for the difference in conventions by adding a call to
`FlowAnalysis.forwardExpression` to the CFE's implementation of
`dispatchExpression`, so that when transitioning between CFE logic and
shared logic, flow analysis will be informed how to match up the
lowered expressions to their pre-lowered counterparts.

Longer term, I would like to switch everything to the convention of
using the original (pre-lowered) expressions; this will bring the
analyzer and CFE into better alignment with each other and should
eliminate a class of subtle bugs. This long term goal is tracked in
https://github.com/dart-lang/sdk/issues/52189.

Fixes #52183.
Fixes #52241.

Bug: https://github.com/dart-lang/sdk/issues/52183, https://github.com/dart-lang/sdk/issues/52241.
Change-Id: I2449ce34c54325603bc2730d1660a7cfc7d79aec
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/298840
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This commit is contained in:
Paul Berry 2023-05-10 19:31:19 +00:00 committed by Commit Queue
parent 3ed8736bdd
commit cb0eb28cba
4 changed files with 108 additions and 1 deletions

View file

@ -1856,7 +1856,6 @@ mixin TypeAnalyzer<
}
head = handleCaseHead(node, head,
caseIndex: caseIndex, subIndex: headIndex);
guard = head.guard;
} else {
hasDefault = true;
handleDefault(node, caseIndex: caseIndex, subIndex: headIndex);

View file

@ -9411,6 +9411,19 @@ class InferenceVisitorImpl extends InferenceVisitorBase
}
pushRewrite(expressionResult.expression);
// The shared analysis logic uses the convention that the expressions passed
// to flow analysis are the original (pre-lowered) expressions, whereas the
// expressions passed to flow analysis by the CFE are the lowered
// expressions. Since the caller of `dispatchExpression` is the shared
// analysis logic, we need to use `flow.forwardExpression` let flow analysis
// know that in future, we'll be referring to the expression using `node`
// (its pre-lowered form) rather than `expressionResult.expression` (its
// post-lowered form).
//
// TODO(paulberry): eliminate the need for this--see
// https://github.com/dart-lang/sdk/issues/52189.
flow.forwardExpression(node, expressionResult.expression);
return new SimpleTypeAnalysisResult(type: expressionResult.inferredType);
}

View file

@ -0,0 +1,29 @@
// Copyright (c) 2023, 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.
// Verifies that a boolean condition can be stored in a variable using a pattern
// assignment, or pattern variable declaration, and later used for type
// promotion.
import '../../static_type_helper.dart';
void patternAssignment(int? x) {
bool b;
(b) = x != null;
if (b) {
x.expectStaticType<Exactly<int>>();
}
}
void patternVariableDeclaration(int? x) {
var (b) = x != null;
if (b) {
x.expectStaticType<Exactly<int>>();
}
}
main() {
patternAssignment(0);
patternVariableDeclaration(0);
}

View file

@ -0,0 +1,66 @@
// Copyright (c) 2023, 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.
// Verifies that a null check in a guard expression causes promotion.
import 'package:expect/expect.dart';
import '../../static_type_helper.dart';
bool inSwitchStatement(x) {
switch (x) {
case int? y when y != null:
y.expectStaticType<Exactly<int>>();
return true;
default:
return false;
}
}
bool inSwitchExpression(x) => switch (x) {
int? y when y != null => [y.expectStaticType<Exactly<int>>()].isNotEmpty,
_ => false
};
bool inIfCaseStatement(x) {
if (x case int? y when y != null) {
y.expectStaticType<Exactly<int>>();
return true;
} else {
return false;
}
}
bool inIfCaseElementInList(x) => [
if (x case int? y when y != null) y.expectStaticType<Exactly<int>>()
].isNotEmpty;
bool inIfCaseElementInMap(x) => {
if (x case int? y when y != null) '': y.expectStaticType<Exactly<int>>()
}.isNotEmpty;
bool inIfCaseElementInSet(x) => {
if (x case int? y when y != null) y.expectStaticType<Exactly<int>>()
}.isNotEmpty;
main() {
Expect.equals(true, inSwitchStatement(0));
Expect.equals(false, inSwitchStatement(null));
Expect.equals(false, inSwitchStatement(''));
Expect.equals(true, inSwitchExpression(0));
Expect.equals(false, inSwitchExpression(null));
Expect.equals(false, inSwitchExpression(''));
Expect.equals(true, inIfCaseStatement(0));
Expect.equals(false, inIfCaseStatement(null));
Expect.equals(false, inIfCaseStatement(''));
Expect.equals(true, inIfCaseElementInList(0));
Expect.equals(false, inIfCaseElementInList(null));
Expect.equals(false, inIfCaseElementInList(''));
Expect.equals(true, inIfCaseElementInMap(0));
Expect.equals(false, inIfCaseElementInMap(null));
Expect.equals(false, inIfCaseElementInMap(''));
Expect.equals(true, inIfCaseElementInSet(0));
Expect.equals(false, inIfCaseElementInSet(null));
Expect.equals(false, inIfCaseElementInSet(''));
}