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

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

Issue 2991853002: Fix duplicate context creation when closures appear in initializers. (Closed)
Patch Set: Review comments: don't use the redirecting technique unless necessary. Created 3 years, 4 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 FunctionDeclaration, 12 FunctionDeclaration,
13 FunctionNode, 13 FunctionNode,
14 Member, 14 Member,
15 Procedure, 15 Procedure,
16 ThisExpression, 16 ThisExpression,
17 TypeParameter, 17 TypeParameter,
18 TypeParameterType, 18 TypeParameterType,
19 VariableDeclaration, 19 VariableDeclaration,
20 VariableGet, 20 VariableGet,
21 VariableSet, 21 VariableSet,
22 visitList; 22 visitList;
23 23
24 import '../../visitor.dart' show RecursiveVisitor; 24 import '../../visitor.dart' show RecursiveVisitor;
25 25
26 class ClosureInfo extends RecursiveVisitor { 26 class ClosureInfo extends RecursiveVisitor {
27 FunctionNode currentFunction; 27 FunctionNode currentFunction;
28
29 static const int OUTSIDE_INITIALIZER = 1;
30 static const int INSIDE_INITIALIZER = 2;
31
32 int captureFlags = OUTSIDE_INITIALIZER;
33
34 final Map<VariableDeclaration, int /*captured flags*/ > variables =
Dmitry Stefantsov 2017/07/31 15:05:35 I think a more elaborate comment about the flags,
sjindel 2017/07/31 15:32:16 Done.
35 <VariableDeclaration, int>{};
36
28 final Map<VariableDeclaration, FunctionNode> function = 37 final Map<VariableDeclaration, FunctionNode> function =
29 <VariableDeclaration, FunctionNode>{}; 38 <VariableDeclaration, FunctionNode>{};
30 39
31 final Set<VariableDeclaration> variables = new Set<VariableDeclaration>();
32
33 /// Map from functions to set of type variables captured within them. 40 /// Map from functions to set of type variables captured within them.
34 final Map<FunctionNode, Set<TypeParameter>> typeVariables = 41 final Map<FunctionNode, Set<TypeParameter>> typeVariables =
35 <FunctionNode, Set<TypeParameter>>{}; 42 <FunctionNode, Set<TypeParameter>>{};
36 43
37 /// Map from members to synthetic variables for accessing `this` in a local 44 /// Map from members to synthetic variables for accessing `this` in a local
38 /// function. 45 /// function.
39 final Map<FunctionNode, VariableDeclaration> thisAccess = 46 final Map<FunctionNode, VariableDeclaration> thisAccess =
40 <FunctionNode, VariableDeclaration>{}; 47 <FunctionNode, VariableDeclaration>{};
41 48
42 final Set<String> currentMemberLocalNames = new Set<String>(); 49 final Set<String> currentMemberLocalNames = new Set<String>();
(...skipping 57 matching lines...) Expand 10 before | Expand all | Expand 10 after
100 /// the parameters that may be used in the initializers. If the nodes are 107 /// the parameters that may be used in the initializers. If the nodes are
101 /// visited in another order, the encountered parameters in initializers 108 /// visited in another order, the encountered parameters in initializers
102 /// are treated as captured, because they are not yet associated with the 109 /// are treated as captured, because they are not yet associated with the
103 /// function. 110 /// function.
104 beginMember(node, node.function); 111 beginMember(node, node.function);
105 saveCurrentFunction(() { 112 saveCurrentFunction(() {
106 currentFunction = currentMemberFunction; 113 currentFunction = currentMemberFunction;
107 114
108 visitList(node.annotations, this); 115 visitList(node.annotations, this);
109 node.name?.accept(this); 116 node.name?.accept(this);
110 node.function?.accept(this); 117
118 visitList(node.function.typeParameters, this);
119 visitList(node.function.positionalParameters, this);
120 visitList(node.function.namedParameters, this);
121
122 assert(captureFlags == OUTSIDE_INITIALIZER);
123 captureFlags = INSIDE_INITIALIZER;
111 visitList(node.initializers, this); 124 visitList(node.initializers, this);
125 captureFlags = OUTSIDE_INITIALIZER;
126
127 node.function.accept(this);
Dmitry Stefantsov 2017/07/31 15:05:35 To this point, we've already visited [typeParamete
sjindel 2017/07/31 15:32:16 Yes, but I wrote it this way so if a new field is
Dmitry Stefantsov 2017/08/01 09:36:13 I guess it's ok to leave it like this, but conside
112 }); 128 });
113 endMember(); 129 endMember();
114 } 130 }
115 131
116 visitProcedure(Procedure node) { 132 visitProcedure(Procedure node) {
117 beginMember(node, node.function); 133 beginMember(node, node.function);
118 super.visitProcedure(node); 134 super.visitProcedure(node);
119 endMember(); 135 endMember();
120 } 136 }
121 137
(...skipping 52 matching lines...) Expand 10 before | Expand all | Expand 10 after
174 } 190 }
175 } 191 }
176 192
177 visitVariableDeclaration(VariableDeclaration node) { 193 visitVariableDeclaration(VariableDeclaration node) {
178 function[node] = currentFunction; 194 function[node] = currentFunction;
179 node.visitChildren(this); 195 node.visitChildren(this);
180 } 196 }
181 197
182 visitVariableGet(VariableGet node) { 198 visitVariableGet(VariableGet node) {
183 if (function[node.variable] != currentFunction) { 199 if (function[node.variable] != currentFunction) {
184 variables.add(node.variable); 200 variables.putIfAbsent(node.variable, () => 0);
201 }
202 if (variables.containsKey(node.variable)) {
203 variables[node.variable] = variables[node.variable] | captureFlags;
Dmitry Stefantsov 2017/07/31 15:05:35 How about using `|=` operation here and below?
sjindel 2017/07/31 15:32:16 Done.
185 } 204 }
186 node.visitChildren(this); 205 node.visitChildren(this);
187 } 206 }
188 207
189 visitVariableSet(VariableSet node) { 208 visitVariableSet(VariableSet node) {
190 if (function[node.variable] != currentFunction) { 209 if (function[node.variable] != currentFunction) {
191 variables.add(node.variable); 210 variables.putIfAbsent(node.variable, () => 0);
211 }
212 if (variables.containsKey(node.variable)) {
213 variables[node.variable] = variables[node.variable] | captureFlags;
192 } 214 }
193 node.visitChildren(this); 215 node.visitChildren(this);
194 } 216 }
195 217
196 visitTypeParameterType(TypeParameterType node) { 218 visitTypeParameterType(TypeParameterType node) {
197 if (!isOuterMostContext && node.parameter.parent != currentFunction) { 219 if (!isOuterMostContext && node.parameter.parent != currentFunction) {
198 typeVariables 220 typeVariables
199 .putIfAbsent(currentFunction, () => new Set<TypeParameter>()) 221 .putIfAbsent(currentFunction, () => new Set<TypeParameter>())
200 .add(node.parameter); 222 .add(node.parameter);
201 } 223 }
202 } 224 }
203 225
204 visitThisExpression(ThisExpression node) { 226 visitThisExpression(ThisExpression node) {
205 if (!isOuterMostContext) { 227 if (!isOuterMostContext) {
206 thisAccess.putIfAbsent( 228 thisAccess.putIfAbsent(
207 currentMemberFunction, () => new VariableDeclaration("#self")); 229 currentMemberFunction, () => new VariableDeclaration("#self"));
208 } 230 }
209 } 231 }
210 232
211 saveCurrentFunction(void f()) { 233 saveCurrentFunction(void f()) {
212 var saved = currentFunction; 234 var saved = currentFunction;
213 try { 235 try {
214 f(); 236 f();
215 } finally { 237 } finally {
216 currentFunction = saved; 238 currentFunction = saved;
217 } 239 }
218 } 240 }
219 } 241 }
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698