Skip to content

Commit d40f84f

Browse files
committed
Added for-loop variable tracking and regular closures/initializers captured variable tracking.
This whole thing is hanging together by a thread. A BUNCH of tests and more implementation coming next once the KernelClosureClass is truly functional. BUG= R=sigmund@google.com Review-Url: https://codereview.chromium.org/2961253005 .
1 parent fb36c9d commit d40f84f

File tree

9 files changed

+536
-137
lines changed

9 files changed

+536
-137
lines changed

pkg/compiler/lib/src/closure.dart

Lines changed: 32 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -84,13 +84,13 @@ class ScopeInfo {
8484
/// Also parameters to a `sync*` generator must be boxed, because of the way
8585
/// we rewrite sync* functions. See also comments in
8686
/// [ClosureClassMap.useLocal].
87-
bool variableIsUsedInTryOrSync(Local variable) => false;
87+
bool localIsUsedInTryOrSync(Local variable) => false;
8888

8989
/// Loop through each variable that has been defined in this scope, modified
9090
/// anywhere (this scope or another scope) and used in another scope. Because
9191
/// it is used in another scope, these variables need to be "boxed", creating
9292
/// a thin wrapper around accesses to these variables so that accesses get
93-
/// the correct updated value. The variables in variablesUsedInTryOrSync may
93+
/// the correct updated value. The variables in localsUsedInTryOrSync may
9494
/// be included in this set.
9595
///
9696
/// In the case of loops, this is the set of iteration variables (or any
@@ -100,9 +100,6 @@ class ScopeInfo {
100100

101101
/// True if [variable] has been mutated and is also used in another scope.
102102
bool isBoxed(Local variable) => false;
103-
104-
/// True if this scope declares any variables that need to be boxed.
105-
bool get hasBoxedVariables => false;
106103
}
107104

108105
/// Class representing the usage of a scope that has been captured in the
@@ -141,6 +138,27 @@ class ClosureScope extends ScopeInfo {
141138
/// each iteration, by boxing the iteration variable[s].
142139
class LoopClosureScope extends ClosureScope {
143140
const LoopClosureScope();
141+
142+
/// True if this loop scope declares in the first part of the loop
143+
/// `for (<here>;...;...)` any variables that need to be boxed.
144+
bool get hasBoxedLoopVariables => false;
145+
146+
/// The set of iteration variables (or variables declared in the for loop
147+
/// expression (`for (<here>; ... ; ...)`) that need to be boxed to snapshot
148+
/// their value. These variables are also included in the set of
149+
/// `forEachBoxedVariable` method. The distinction between these two sets is
150+
/// in this example:
151+
///
152+
/// run(f) => f();
153+
/// var a;
154+
/// for (int i = 0; i < 3; i++) {
155+
/// var b = 3;
156+
/// a = () => b = i;
157+
/// }
158+
///
159+
/// `i` would be a part of the boxedLoopVariables AND boxedVariables, but b
160+
/// would only be a part of boxedVariables.
161+
List<Local> get boxedLoopVariables => const <Local>[];
144162
}
145163

146164
/// Class that describes the actual mechanics of how the converted, rewritten
@@ -647,18 +665,10 @@ class ClosureScopeImpl implements ClosureScope, LoopClosureScope {
647665
bool get requiresContextBox => capturedVariables.keys.isNotEmpty;
648666

649667
void forEachBoxedVariable(f(Local local, FieldEntity field)) {
650-
if (capturedVariables.isNotEmpty) {
651-
capturedVariables.forEach(f);
652-
} else {
653-
for (Local l in boxedLoopVariables) {
654-
// The boxes for loop variables are constructed on-demand per-iteration
655-
// in the locals handler.
656-
f(l, null);
657-
}
658-
}
668+
capturedVariables.forEach(f);
659669
}
660670

661-
bool get hasBoxedVariables => !capturedVariables.isEmpty;
671+
bool get hasBoxedLoopVariables => boxedLoopVariables.isNotEmpty;
662672

663673
bool isBoxed(Local variable) {
664674
return capturedVariables.containsKey(variable);
@@ -670,8 +680,8 @@ class ClosureScopeImpl implements ClosureScope, LoopClosureScope {
670680
}
671681

672682
// Should not be called. Added to make the new interface happy.
673-
bool variableIsUsedInTryOrSync(Local variable) =>
674-
throw new UnsupportedError("ClosureScopeImpl.variableIsUsedInTryOrSync");
683+
bool localIsUsedInTryOrSync(Local variable) =>
684+
throw new UnsupportedError("ClosureScopeImpl.localIsUsedInTryOrSync");
675685

676686
// Should not be called. Added to make the new interface happy.
677687
Local get thisLocal =>
@@ -741,14 +751,11 @@ class ClosureClassMap implements ClosureRepresentationInfo {
741751
/// Also parameters to a `sync*` generator must be boxed, because of the way
742752
/// we rewrite sync* functions. See also comments in [useLocal].
743753
// TODO(johnniwinther): Add variables to this only if the variable is mutated.
744-
final Set<Local> variablesUsedInTryOrSync = new Set<Local>();
754+
final Set<Local> localsUsedInTryOrSync = new Set<Local>();
745755

746756
ClosureClassMap(this.closureEntity, this.closureClassEntity, this.callMethod,
747757
this.thisLocal);
748758

749-
bool get hasBoxedVariables =>
750-
throw new UnsupportedError("ClosureClassMap.hasBoxedVariables");
751-
752759
List<Local> get createdFieldEntities {
753760
List<Local> fields = <Local>[];
754761
if (closureClassEntity == null) return const <Local>[];
@@ -775,8 +782,8 @@ class ClosureClassMap implements ClosureRepresentationInfo {
775782

776783
FieldEntity get thisFieldEntity => freeVariableMap[thisLocal];
777784

778-
bool variableIsUsedInTryOrSync(Local variable) =>
779-
variablesUsedInTryOrSync.contains(variable);
785+
bool localIsUsedInTryOrSync(Local variable) =>
786+
localsUsedInTryOrSync.contains(variable);
780787

781788
Local getLocalVariableForClosureField(ClosureFieldElement field) {
782789
return field.local;
@@ -1025,13 +1032,13 @@ class ClosureTranslator extends Visitor {
10251032
if (variable != closureData.thisLocal &&
10261033
variable != closureData.closureEntity &&
10271034
variable is! TypeVariableLocal) {
1028-
closureData.variablesUsedInTryOrSync.add(variable);
1035+
closureData.localsUsedInTryOrSync.add(variable);
10291036
}
10301037
} else if (variable is LocalParameterElement &&
10311038
variable.functionDeclaration.asyncMarker == AsyncMarker.SYNC_STAR) {
10321039
// Parameters in a sync* function are shared between each Iterator created
10331040
// by the Iterable returned by the function, therefore they must be boxed.
1034-
closureData.variablesUsedInTryOrSync.add(variable);
1041+
closureData.localsUsedInTryOrSync.add(variable);
10351042
}
10361043
}
10371044

0 commit comments

Comments
 (0)