Fix handling of for-elements with unparenthesized bodies.
The CFE defers the final build of the body of a for-element until
after building the rest of the body. This caused a bug: if the body
was an assignment expression, flow analysis was not picking up on the
fact that the assignment was inside the loop.
Fixed by introducing a mechanism that allows the CFE to temporarily
pop the innermost structure on the AssignedVariables stack and then
push it again.
Change-Id: I11d526302934e9283807124ebd4a23e9d3e5ec66
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/164980
Reviewed-by: Johnni Winther <johnniwinther@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 218b4f6..e404a1a 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
@@ -155,6 +155,22 @@
}());
}
+ /// Call this method between calls to [beginNode] and [endNode]/[deferNode],
+ /// if it is necessary to temporarily process some code outside the current
+ /// node. Returns a data structure that should be passed to [pushNode].
+ ///
+ /// This is used by the front end when building for-elements in lists, maps,
+ /// and sets; their initializers are partially built after building their
+ /// loop conditions but before completely building their bodies.
+ AssignedVariablesNodeInfo<Variable> popNode() {
+ return _stack.removeLast();
+ }
+
+ /// Call this method to un-do the effect of [popNode].
+ void pushNode(AssignedVariablesNodeInfo<Variable> node) {
+ _stack.add(node);
+ }
+
/// Call this method to register that the node [from] for which information
/// has been stored is replaced by the node [to].
// TODO(johnniwinther): Remove this when unified collections are encoded as
diff --git a/pkg/_fe_analyzer_shared/test/flow_analysis/assigned_variables/data/for_element.dart b/pkg/_fe_analyzer_shared/test/flow_analysis/assigned_variables/data/for_element.dart
index 8259c35..cb3d20b 100644
--- a/pkg/_fe_analyzer_shared/test/flow_analysis/assigned_variables/data/for_element.dart
+++ b/pkg/_fe_analyzer_shared/test/flow_analysis/assigned_variables/data/for_element.dart
@@ -7,11 +7,21 @@
[/*assigned={b, c, d}*/ for (a = 0; (b = 0) != 0; c = 0) (d = 0)];
}
+/*member: cStyle_unparenthesized:declared={a, b, c, d, e}, assigned={a, b, d, e}*/
+cStyle_unparenthesized(int a, bool b, bool c, int d, int e) {
+ [/*assigned={b, d, e}*/ for (a = 0; b = c; d = 0) e = 0];
+}
+
/*member: cStyleWithDeclaration:declared={a, b, c, d, e}, assigned={a, b, c, d}*/
cStyleWithDeclaration(int a, int b, int c, int d) {
[/*assigned={b, c, d}*/ for (int e = (a = 0); (b = 0) != 0; c = 0) (d = 0)];
}
+/*member: cStyleWithDeclaration_unparenthesized:declared={a, b, c, d, e, f}, assigned={a, b, d, e}*/
+cStyleWithDeclaration_unparenthesized(int a, bool b, bool c, int d, int e) {
+ [/*assigned={b, d, e}*/ for (int f = a = 0; b = c; d = 0) e = 0];
+}
+
/*member: forEach:declared={a, b, c}, assigned={a, b, c}*/
forEach(int a, int b, int c) {
[
@@ -19,9 +29,23 @@
];
}
+/*member: forEach_unparenthesized:declared={a, b, c}, assigned={a, b, c}*/
+forEach_unparenthesized(int a, int b, int c) {
+ [
+ /*assigned={c}*/ for (a in [b = 0]) c = 0
+ ];
+}
+
/*member: forEachWithDeclaration:declared={a, b, c}, assigned={a, b}*/
forEachWithDeclaration(int a, int b) {
[
/*assigned={b}*/ for (var c in [a = 0]) (b = 0)
];
}
+
+/*member: forEachWithDeclaration_unparenthesized:declared={a, b, c}, assigned={a, b}*/
+forEachWithDeclaration_unparenthesized(int a, int b) {
+ [
+ /*assigned={b}*/ for (var c in [a = 0]) b = 0
+ ];
+}
diff --git a/pkg/front_end/lib/src/fasta/kernel/body_builder.dart b/pkg/front_end/lib/src/fasta/kernel/body_builder.dart
index a07d1a8..de52a7e 100644
--- a/pkg/front_end/lib/src/fasta/kernel/body_builder.dart
+++ b/pkg/front_end/lib/src/fasta/kernel/body_builder.dart
@@ -2804,7 +2804,7 @@
// [handleForInitializerExpressionStatement], and
// [handleForInitializerLocalVariableDeclaration].
AssignedVariablesNodeInfo<VariableDeclaration> assignedVariablesNodeInfo =
- typeInferrer?.assignedVariables?.deferNode();
+ typeInferrer?.assignedVariables?.popNode();
Object variableOrExpression = pop();
exitLocalScope();
@@ -2812,6 +2812,7 @@
transformCollections = true;
List<VariableDeclaration> variables =
_buildForLoopVariableDeclarations(variableOrExpression);
+ typeInferrer?.assignedVariables?.pushNode(assignedVariablesNodeInfo);
Expression condition;
if (conditionStatement is ExpressionStatement) {
condition = conditionStatement.expression;
@@ -2821,14 +2822,12 @@
if (entry is MapEntry) {
ForMapEntry result = forest.createForMapEntry(
offsetForToken(forToken), variables, condition, updates, entry);
- typeInferrer?.assignedVariables
- ?.storeInfo(result, assignedVariablesNodeInfo);
+ typeInferrer?.assignedVariables?.endNode(result);
push(result);
} else {
ForElement result = forest.createForElement(offsetForToken(forToken),
variables, condition, updates, toValue(entry));
- typeInferrer?.assignedVariables
- ?.storeInfo(result, assignedVariablesNodeInfo);
+ typeInferrer?.assignedVariables?.endNode(result);
push(result);
}
}
@@ -5039,7 +5038,7 @@
// This is matched by the call to [beginNode] in [handleForInLoopParts].
AssignedVariablesNodeInfo<VariableDeclaration> assignedVariablesNodeInfo =
- typeInferrer?.assignedVariables?.deferNode();
+ typeInferrer?.assignedVariables?.popNode();
Expression iterable = popForValue();
Object lvalue = pop(); // lvalue
@@ -5048,6 +5047,7 @@
transformCollections = true;
ForInElements elements =
_computeForInElements(forToken, inToken, lvalue, null);
+ typeInferrer?.assignedVariables?.pushNode(assignedVariablesNodeInfo);
VariableDeclaration variable = elements.variable;
Expression problem = elements.expressionProblem;
if (entry is MapEntry) {
@@ -5060,8 +5060,7 @@
entry,
problem,
isAsync: awaitToken != null);
- typeInferrer?.assignedVariables
- ?.storeInfo(result, assignedVariablesNodeInfo);
+ typeInferrer?.assignedVariables?.endNode(result);
push(result);
} else {
ForInElement result = forest.createForInElement(
@@ -5073,8 +5072,7 @@
toValue(entry),
problem,
isAsync: awaitToken != null);
- typeInferrer?.assignedVariables
- ?.storeInfo(result, assignedVariablesNodeInfo);
+ typeInferrer?.assignedVariables?.endNode(result);
push(result);
}
}
