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

Side by Side Diff: pkg/kernel/lib/transformations/closure/info.dart

Issue 2939043002: Remove unnecessary contexts in closure conversion (Closed)
Patch Set: Add helper function, add example to the comment 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 unified diff | Download patch
OLDNEW
1 // Copyright (c) 2016, the Dart project authors. Please see the AUTHORS file 1 // Copyright (c) 2016, the Dart project authors. Please see the AUTHORS file
2 // for details. All rights reserved. Use of this source code is governed by a 2 // for details. All rights reserved. Use of this source code is governed by a
3 // BSD-style license that can be found in the LICENSE file. 3 // BSD-style license that can be found in the LICENSE file.
4 4
5 library kernel.transformations.closure.info; 5 library kernel.transformations.closure.info;
6 6
7 import '../../ast.dart' 7 import '../../ast.dart'
8 show 8 show
9 Class, 9 Class,
10 Constructor, 10 Constructor,
11 Field, 11 Field,
12 FieldInitializer,
12 FunctionDeclaration, 13 FunctionDeclaration,
13 FunctionNode, 14 FunctionNode,
15 LocalInitializer,
14 Member, 16 Member,
15 Name, 17 Name,
16 Procedure, 18 Procedure,
17 ProcedureKind, 19 ProcedureKind,
18 PropertyGet, 20 PropertyGet,
21 RedirectingInitializer,
22 SuperInitializer,
19 ThisExpression, 23 ThisExpression,
20 TypeParameter, 24 TypeParameter,
21 TypeParameterType, 25 TypeParameterType,
22 VariableDeclaration, 26 VariableDeclaration,
23 VariableGet, 27 VariableGet,
24 VariableSet; 28 VariableSet;
25 29
26 import '../../visitor.dart' show RecursiveVisitor; 30 import '../../visitor.dart' show RecursiveVisitor;
27 31
28 class ClosureInfo extends RecursiveVisitor { 32 class ClosureInfo extends RecursiveVisitor {
(...skipping 64 matching lines...) Expand 10 before | Expand all | Expand 10 after
93 currentMemberFunction = null; 97 currentMemberFunction = null;
94 } 98 }
95 99
96 visitClass(Class node) { 100 visitClass(Class node) {
97 currentClass = node; 101 currentClass = node;
98 super.visitClass(node); 102 super.visitClass(node);
99 currentClass = null; 103 currentClass = null;
100 } 104 }
101 105
102 visitConstructor(Constructor node) { 106 visitConstructor(Constructor node) {
107 // For constructors we need to set [currentFunction] before visiting the
108 // [FunctionNode] of the constructor, because initializers may use
109 // constructor parameters and it shouldn't be treated as capturing them.
110 // Consider the following code:
111 //
112 // class A {
113 // int x;
114 // A(int x) /* "x" is visible in initializers and body. */
Dmitry Stefantsov 2017/06/16 08:36:27 I think I missed a spot here. "x" should have bee
karlklose 2017/06/16 08:43:40 Consider using '///' and '[x]' here and below (alt
Dmitry Stefantsov 2017/06/16 08:55:28 Done.
115 // : this.x = x { /* Initializer. */
116 // /* Constructor body. */
117 // }
118 // }
119 //
120 // Here _x_ parameter shouldn't be captured into a context in the
karlklose 2017/06/16 08:43:41 "_x_ parameter" -> "the parameter"?
Dmitry Stefantsov 2017/06/16 08:55:27 Done.
121 // initializer. However, if [currentFunction] is not set, it has _null_
karlklose 2017/06/16 08:43:41 How about 'However, [currentFunction] is `null` if
Dmitry Stefantsov 2017/06/16 08:55:28 Agree, that reads much better.
122 // value, and _function[node.variable]_ in this case points to the
123 // [FunctionNode] of the constructor (which is not _null_). It leads to "x"
karlklose 2017/06/16 08:43:41 `null`
Dmitry Stefantsov 2017/06/16 08:55:28 Done.
124 // being treated as captured, because it's seen as used outside of the
125 // function where it is declared. In turn, it leads to unnecessary context
126 // creation and usage.
103 beginMember(node, node.function); 127 beginMember(node, node.function);
104 super.visitConstructor(node); 128 saveCurrentFunction(() {
129 currentFunction = currentMemberFunction;
130 super.visitConstructor(node);
131 });
105 endMember(); 132 endMember();
106 } 133 }
107 134
108 visitProcedure(Procedure node) { 135 visitProcedure(Procedure node) {
109 beginMember(node, node.function); 136 beginMember(node, node.function);
110 if (node.isInstanceMember && node.kind == ProcedureKind.Method) { 137 if (node.isInstanceMember && node.kind == ProcedureKind.Method) {
111 // Ignore the `length` method of [File] subclasses for now, as they 138 // Ignore the `length` method of [File] subclasses for now, as they
112 // will force us to rename the `length` getter (kernel issue #43). 139 // will force us to rename the `length` getter (kernel issue #43).
113 // TODO(ahe): remove this condition. 140 // TODO(ahe): remove this condition.
114 Class parent = node.parent; 141 Class parent = node.parent;
(...skipping 36 matching lines...) Expand 10 before | Expand all | Expand 10 after
151 } 178 }
152 179
153 visitFunctionDeclaration(FunctionDeclaration node) { 180 visitFunctionDeclaration(FunctionDeclaration node) {
154 assert(!localNames.containsKey(node)); 181 assert(!localNames.containsKey(node));
155 localNames[node.function] = computeUniqueLocalName(node.variable.name); 182 localNames[node.function] = computeUniqueLocalName(node.variable.name);
156 return super.visitFunctionDeclaration(node); 183 return super.visitFunctionDeclaration(node);
157 } 184 }
158 185
159 visitFunctionNode(FunctionNode node) { 186 visitFunctionNode(FunctionNode node) {
160 localNames.putIfAbsent(node, computeUniqueLocalName); 187 localNames.putIfAbsent(node, computeUniqueLocalName);
161 var saved = currentFunction; 188
162 currentFunction = node; 189 saveCurrentFunction(() {
163 node.visitChildren(this); 190 currentFunction = node;
164 currentFunction = saved; 191 node.visitChildren(this);
192 });
193
165 Set<TypeParameter> capturedTypeVariables = typeVariables[node]; 194 Set<TypeParameter> capturedTypeVariables = typeVariables[node];
166 if (capturedTypeVariables != null && !isOuterMostContext) { 195 if (capturedTypeVariables != null && !isOuterMostContext) {
167 // Propagate captured type variables to enclosing function. 196 // Propagate captured type variables to enclosing function.
168 typeVariables 197 typeVariables
169 .putIfAbsent(currentFunction, () => new Set<TypeParameter>()) 198 .putIfAbsent(currentFunction, () => new Set<TypeParameter>())
170 .addAll(capturedTypeVariables); 199 .addAll(capturedTypeVariables);
171 } 200 }
172 } 201 }
173 202
174 visitVariableDeclaration(VariableDeclaration node) { 203 visitVariableDeclaration(VariableDeclaration node) {
(...skipping 27 matching lines...) Expand all
202 if (!isOuterMostContext) { 231 if (!isOuterMostContext) {
203 thisAccess.putIfAbsent( 232 thisAccess.putIfAbsent(
204 currentMemberFunction, () => new VariableDeclaration("#self")); 233 currentMemberFunction, () => new VariableDeclaration("#self"));
205 } 234 }
206 } 235 }
207 236
208 visitPropertyGet(PropertyGet node) { 237 visitPropertyGet(PropertyGet node) {
209 invokedGetters.add(node.name); 238 invokedGetters.add(node.name);
210 super.visitPropertyGet(node); 239 super.visitPropertyGet(node);
211 } 240 }
241
242 saveCurrentFunction(void f()) {
243 var saved = currentFunction;
244 try {
245 f();
246 } finally {
247 currentFunction = saved;
248 }
249 }
212 } 250 }
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698