Share locals between members

pkg/compiler/lib/src/js_model/closure.dart

 // Copyright (c) 2017, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 import 'package:kernel/ast.dart' as ir;
 
 import '../closure.dart';
 import '../common.dart';
 import '../common/tasks.dart';
 import '../constants/expressions.dart';
@@ skipping 121 matching lines @@
       ir.FunctionNode node,
       KernelScopeInfo info,
       JsClosedWorld closedWorldRefiner) {
     KernelToLocalsMap localsMap = _globalLocalsMap.getLocalsMap(member);
     KernelClosureClass closureClass = closedWorldRefiner.buildClosureClass(
         member, node, member.library, info, node.location, localsMap);
 
     // We want the original declaration where that function is used to point
     // to the correct closure class.
     _closureRepresentationMap[closureClass.callMethod] = closureClass;
+    _globalLocalsMap.setLocalsMap(closureClass.callMethod, localsMap);

[Emily Fortuna, 2017/08/17 19:01:59]

what about grouping the locals map for all the created closures in ScopeModel
(like the capturedScopes)?

[Johnni Winther, 2017/08/18 14:59:39]

This line means we are sharing the map for all those closures. (Not sure what
you mean by 'grouping')

     Entity entity;
     if (node.parent is ir.Member) {
       entity = _elementMap.getMember(node.parent);
     } else {
       entity = localsMap.getLocalFunction(node.parent);
     }
     assert(entity != null);
     _closureRepresentationMap[entity] = closureClass;
     return closureClass;
   }
@@ skipping 239 matching lines @@
 /// A local variable to disambiguate between a variable that has been captured
 /// from one scope to another. This is the ir.Node version that corresponds to
 /// [BoxLocal].
 class NodeBox {
   final String name;
   final ir.TreeNode executableContext;
   NodeBox(this.name, this.executableContext);
 }
 
 class JClosureClass extends JClass {
-  // TODO(efortuna): Storing this map here is so horrible. Instead store this on
-  // the ScopeModel (because all of the closures share that localsMap) and then
-  // set populate the getLocalVariable lookup with this localsMap for all the
-  // closures.
-  final KernelToLocalsMap localsMap;

[Emily Fortuna, 2017/08/17 19:01:59]

if we get rid of this field, all JClosureClasses can just become JClasses,
because there's no need to have a separate type of object. 

[Johnni Winther, 2017/08/18 14:59:39]

It does have a better to `toString` and the right [isClosure] value!

-
-  JClosureClass(this.localsMap, JLibrary library, int classIndex, String name)
+  JClosureClass(JLibrary library, int classIndex, String name)
       : super(library, classIndex, name, isAbstract: false);
 
   @override
   bool get isClosure => true;
 
   String toString() => '${jsElementPrefix}closure_class($name)';
 }
 
 class JClosureField extends JField {
   JClosureField(String name, int memberIndex,
@@ skipping 119 matching lines @@
   KernelScopeInfo scopeInfo;
 
   /// Collected [CapturedScope] data for nodes.
   Map<ir.Node, KernelCapturedScope> capturedScopesMap =
       <ir.Node, KernelCapturedScope>{};
 
   /// Collected [ScopeInfo] data for nodes.
   Map<ir.FunctionNode, KernelScopeInfo> closuresToGenerate =
       <ir.FunctionNode, KernelScopeInfo>{};
 }