Skip to content

Commit 017e0f7

Browse files
sigmundchCommit Bot
authored and
Commit Bot
committed
[dart2js] Fix nested conditions in global inference
A bug in global inference made the analysis accidentally carry over the state of a nested condition in a subexpression and conclude that it was part of the state of the outer condition. This was due to the fact that we didn't clear the state when popping out of the context and reentering the conition state. Fixes #48571 Change-Id: I4e8294bb1a05bb3b910b6aec374357d75acb67cb Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/240321 Reviewed-by: Stephen Adams <[email protected]> Commit-Queue: Sigmund Cherem <[email protected]>
1 parent bbf5dc1 commit 017e0f7

File tree

2 files changed

+19
-16
lines changed

2 files changed

+19
-16
lines changed

pkg/compiler/lib/src/inferrer/builder_kernel.dart

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,12 @@ class KernelTypeGraphBuilder extends ir.Visitor<TypeInformation>
9999
_stateAfterWhenFalseInternal = whenFalse;
100100
}
101101

102+
/// Removes from the current [_state] any data from the boolean value of the
103+
/// most recently visited node.
104+
void _clearConditionalStateAfter() {
105+
_stateAfterWhenTrueInternal = _stateAfterWhenFalseInternal = null;
106+
}
107+
102108
final SideEffectsBuilder _sideEffectsBuilder;
103109
final Map<JumpTarget, List<LocalState>> _breaksFor =
104110
<JumpTarget, List<LocalState>>{};
@@ -257,15 +263,16 @@ class KernelTypeGraphBuilder extends ir.Visitor<TypeInformation>
257263
var oldAccumulateIsChecks = _accumulateIsChecks;
258264
_accumulateIsChecks = conditionContext;
259265
var result = node?.accept(this);
266+
267+
// Clear the conditional state to ensure we don't accidentally carry over
268+
// conclusions from a nested condition into an outer condition. For example:
269+
//
270+
// if (methodCall(x is T && true)) { /* don't assume x is T here. */ }
271+
if (!conditionContext) _clearConditionalStateAfter();
260272
_accumulateIsChecks = oldAccumulateIsChecks;
261273
return result;
262274
}
263275

264-
void visitList(List<ir.Node> nodes) {
265-
if (nodes == null) return;
266-
nodes.forEach(visit);
267-
}
268-
269276
void handleParameter(ir.VariableDeclaration node, {bool isOptional}) {
270277
Local local = _localsMap.getLocalVariable(node);
271278
DartType type = _localsMap.getLocalType(_elementMap, local);
@@ -1696,11 +1703,7 @@ class KernelTypeGraphBuilder extends ir.Visitor<TypeInformation>
16961703
}
16971704

16981705
TypeInformation handleCondition(ir.Node node) {
1699-
bool oldAccumulateIsChecks = _accumulateIsChecks;
1700-
_accumulateIsChecks = true;
1701-
TypeInformation result = visit(node, conditionContext: true);
1702-
_accumulateIsChecks = oldAccumulateIsChecks;
1703-
return result;
1706+
return visit(node, conditionContext: true);
17041707
}
17051708

17061709
void _potentiallyAddIsCheck(ir.IsExpression node) {

pkg/compiler/test/inference/data/issue_48571.dart

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,35 +21,35 @@ Base either = DateTime.now()
2121
? Child2()
2222
: Child1();
2323

24-
/*member: test1:[null|exact=Child1]*/
24+
/*member: test1:Union(null, [exact=Child1], [exact=Child2])*/
2525
test1() {
2626
Base child = either;
2727
if (trivial(child is Child1 && true)) return child;
2828
return null;
2929
}
3030

31-
/*member: test2:[null|exact=Child1]*/
31+
/*member: test2:Union(null, [exact=Child1], [exact=Child2])*/
3232
test2() {
3333
Base child = either;
3434
if (child is Child1 || trivial(child is Child1 && true)) return child;
3535
return null;
3636
}
3737

38-
/*member: test3:[null]*/
38+
/*member: test3:[null|exact=Child2]*/
3939
test3() {
4040
Base child = either;
4141
if (trivial(child is Child1 && true) && child is Child2) return child;
4242
return null;
4343
}
4444

45-
/*member: test4:[null]*/
45+
/*member: test4:[null|exact=Child2]*/
4646
test4() {
4747
Base child = either;
4848
if (child is Child2 && trivial(child is Child1 && true)) return child;
4949
return null;
5050
}
5151

52-
/*member: test5:[null|exact=Child1]*/
52+
/*member: test5:Union(null, [exact=Child1], [exact=Child2])*/
5353
test5() {
5454
Base child = either;
5555
if ((child is Child1 && true) /*invoke: [exact=JSBool]*/ == false)
@@ -64,7 +64,7 @@ test6() {
6464
return null;
6565
}
6666

67-
/*member: test7:[null|exact=Child1]*/
67+
/*member: test7:Union(null, [exact=Child1], [exact=Child2])*/
6868
test7() {
6969
Base child = either;
7070
if (trivial(trivial(child is Child1 && true))) return child;

0 commit comments

Comments
 (0)