Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(177)

Unified Diff: pkg/kernel/lib/transformations/closure/info.dart

Issue 2939043002: Remove unnecessary contexts in closure conversion (Closed)
Patch Set: Small comment edit Created 3 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: pkg/kernel/lib/transformations/closure/info.dart
diff --git a/pkg/kernel/lib/transformations/closure/info.dart b/pkg/kernel/lib/transformations/closure/info.dart
index 2d9c2ca3c90f6f38ea3b78201741b63a3eaa7c94..b47da636cc3c54acac95d7a22522dd7726b69b29 100644
--- a/pkg/kernel/lib/transformations/closure/info.dart
+++ b/pkg/kernel/lib/transformations/closure/info.dart
@@ -9,13 +9,17 @@ import '../../ast.dart'
Class,
Constructor,
Field,
+ FieldInitializer,
FunctionDeclaration,
FunctionNode,
+ LocalInitializer,
Member,
Name,
Procedure,
ProcedureKind,
PropertyGet,
+ RedirectingInitializer,
+ SuperInitializer,
ThisExpression,
TypeParameter,
TypeParameterType,
@@ -100,8 +104,30 @@ class ClosureInfo extends RecursiveVisitor {
}
visitConstructor(Constructor node) {
+ /// [currentFunction] should be set to [currentMemberFunction] before
+ /// visiting the [FunctionNode] of the constructor, because initializers may
+ /// use constructor parameters and it shouldn't be treated as capturing
+ /// them. Consider the following code:
+ ///
+ /// class A {
+ /// int x;
+ /// A(int x) /* [x] is visible in initializers and body. */
+ /// : this.x = x { /* Initializer. */
+ /// /* Constructor body. */
+ /// }
+ /// }
+ ///
+ /// Here the parameter shouldn't be captured into a context in the
+ /// initializer. However, [currentFunction] is `null` if not set, and
+ /// `function[node.variable]` in this case points to the [FunctionNode] of
+ /// the constructor (which is not `null`). It leads to `x` being treated as
+ /// captured, because it's seen as used outside of the function where it is
+ /// declared. In turn, it leads to unnecessary context creation and usage.
beginMember(node, node.function);
- super.visitConstructor(node);
+ saveCurrentFunction(() {
+ currentFunction = currentMemberFunction;
+ super.visitConstructor(node);
+ });
endMember();
}
@@ -158,10 +184,12 @@ class ClosureInfo extends RecursiveVisitor {
visitFunctionNode(FunctionNode node) {
localNames.putIfAbsent(node, computeUniqueLocalName);
- var saved = currentFunction;
- currentFunction = node;
- node.visitChildren(this);
- currentFunction = saved;
+
+ saveCurrentFunction(() {
+ currentFunction = node;
+ node.visitChildren(this);
+ });
+
Set<TypeParameter> capturedTypeVariables = typeVariables[node];
if (capturedTypeVariables != null && !isOuterMostContext) {
// Propagate captured type variables to enclosing function.
@@ -209,4 +237,13 @@ class ClosureInfo extends RecursiveVisitor {
invokedGetters.add(node.name);
super.visitPropertyGet(node);
}
+
+ saveCurrentFunction(void f()) {
+ var saved = currentFunction;
+ try {
+ f();
+ } finally {
+ currentFunction = saved;
+ }
+ }
}

Powered by Google App Engine
This is Rietveld 408576698