[dart2js] Adding closure dump info tests + fixing sibling logic in kernel dump info

- Reconciles some logic for closure/LocalFunction name resolution between dump info and the element model
- Handles closure disambiguation (j world <-> k world) by assuming fixed visit order

Change-Id: Ic9a61208ed93372633e118541b71f20543e250a3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/245600
Commit-Queue: Mark Zhou <markzipan@google.com>
Reviewed-by: Joshua Litt <joshualitt@google.com>
diff --git a/pkg/compiler/lib/src/dump_info.dart b/pkg/compiler/lib/src/dump_info.dart
index e84fc46..a1b132b 100644
--- a/pkg/compiler/lib/src/dump_info.dart
+++ b/pkg/compiler/lib/src/dump_info.dart
@@ -24,6 +24,7 @@
 import 'deferred_load/output_unit.dart' show OutputUnit, deferredPartFileName;
 import 'dump_info_javascript_monitor.dart';
 import 'elements/entities.dart';
+import 'elements/entity_utils.dart' as entity_utils;
 import 'inferrer/abstract_value_domain.dart';
 import 'inferrer/types.dart'
     show GlobalTypeInferenceMemberResult, GlobalTypeInferenceResults;
@@ -663,14 +664,16 @@
     List<ClosureInfo> nestedClosures = <ClosureInfo>[];
     localFunctionInfoCollector.localFunctions.forEach((key, value) {
       FunctionEntity closureEntity;
+      int closureOrder = value.order;
       environment.forEachNestedClosure(memberEntity, (closure) {
-        if (closure.enclosingClass.name == value.name) {
+        if (closure.enclosingClass.name == value.name &&
+            (closureOrder-- == 0)) {
           closureEntity = closure;
         }
       });
       final closureClassEntity = closureEntity.enclosingClass;
-      final closureInfo =
-          ClosureInfo(name: value.name, outputUnit: null, size: null);
+      final closureInfo = ClosureInfo(
+          name: value.disambiguatedName, outputUnit: null, size: null);
       state.entityToInfo[closureClassEntity] = closureInfo;
 
       FunctionEntity callMethod = closedWorld.elementEnvironment
@@ -691,6 +694,32 @@
   }
 }
 
+/// Maps JWorld Entity objects to disambiguated names in order to map them
+/// to/from Kernel.
+///
+/// This is primarily used for naming closure objects, which rely on Entity
+/// object identity to determine uniqueness.
+///
+/// Note: this relies on the Kernel traversal order to determine order, which
+/// may change in the future.
+class EntityDisambiguator {
+  final nameFrequencies = <String, int>{};
+  final entityNames = <Entity, String>{};
+
+  String name(Entity entity) {
+    final disambiguatedName = entityNames[entity];
+    if (disambiguatedName != null) {
+      return disambiguatedName;
+    }
+    nameFrequencies[entity.name] = (nameFrequencies[entity.name] ?? -1) + 1;
+    final order = nameFrequencies[entity.name];
+    entityNames[entity] =
+        order == 0 ? entity.name : '${entity.name}%${order - 1}';
+
+    return entityNames[entity];
+  }
+}
+
 /// Annotates [KernelInfoCollector] with info extracted from closed-world
 /// analysis.
 class DumpInfoAnnotator {
@@ -699,6 +728,7 @@
   final JClosedWorld closedWorld;
   final GlobalTypeInferenceResults _globalInferenceResults;
   final DumpInfoTask dumpInfoTask;
+  final entityDisambiguator = EntityDisambiguator();
 
   JElementEnvironment get environment => closedWorld.elementEnvironment;
 
@@ -901,8 +931,9 @@
   }
 
   ClosureInfo visitClosureClass(ClassEntity element) {
+    final disambiguatedElementName = entityDisambiguator.name(element);
     final kClosureInfos = kernelInfo.state.info.closures
-        .where((info) => info.name == element.name)
+        .where((info) => info.name == disambiguatedElementName)
         .toList();
     assert(
         kClosureInfos.length == 1,
@@ -916,7 +947,8 @@
     FunctionEntity callMethod = closedWorld.elementEnvironment
         .lookupClassMember(element, Identifiers.call);
 
-    visitFunction(callMethod, element.name);
+    final functionInfo = visitFunction(callMethod, disambiguatedElementName);
+    if (functionInfo == null) return null;
 
     kClosureInfo.treeShakenStatus = TreeShakenStatus.Live;
     return kClosureInfo;
@@ -926,8 +958,8 @@
   // not always be valid. Check and validate later.
   FunctionInfo visitFunction(FunctionEntity function, String parentName) {
     int size = dumpInfoTask.sizeOf(function);
-    // TODO(sigmund): consider adding a small info to represent unreachable
-    // code here.
+    if (size == 0 && !shouldKeep(function)) return null;
+
     var compareName = function.name;
     if (function.isConstructor) {
       compareName = compareName == ""
@@ -1477,63 +1509,36 @@
 
 class LocalFunctionInfo {
   final ir.LocalFunction localFunction;
-  final List<ir.TreeNode> hierarchy;
   final String name;
+  final int order;
   bool isInvoked = false;
 
-  LocalFunctionInfo._(this.localFunction, this.hierarchy, this.name);
+  LocalFunctionInfo(this.localFunction, this.name, this.order);
 
-  factory LocalFunctionInfo(ir.LocalFunction localFunction) {
-    String name = '';
-    ir.TreeNode node = localFunction;
-    final hierarchy = <ir.TreeNode>[];
-    bool inClosure = false;
-    while (node != null) {
-      // Only consider nodes used for resolving a closure's full name.
-      if (node is ir.FunctionDeclaration) {
-        hierarchy.add(node);
-        name = '_${node.variable.name}' + name;
-        inClosure = false;
-      } else if (node is ir.FunctionExpression) {
-        hierarchy.add(node);
-        name = (inClosure ? '_' : '_closure') + name;
-        inClosure = true;
-      } else if (node is ir.Member) {
-        hierarchy.add(node);
-        var cleanName = node.toStringInternal();
-        if (cleanName.endsWith('.'))
-          cleanName = cleanName.substring(0, cleanName.length - 1);
-        final isFactory = node is ir.Procedure && node.isFactory;
-        if (isFactory) {
-          cleanName = cleanName.replaceAll('.', '\$');
-          cleanName = '${node.enclosingClass.toStringInternal()}_' + cleanName;
-        } else {
-          cleanName = cleanName.replaceAll('.', '_');
-        }
-        name = cleanName + name;
-        inClosure = false;
-      }
-      node = node.parent;
-    }
-
-    return LocalFunctionInfo._(localFunction, hierarchy, name);
-  }
+  get disambiguatedName => order == 0 ? name : '$name%${order - 1}';
 }
 
 class LocalFunctionInfoCollector extends ir.RecursiveVisitor<void> {
   final localFunctions = <ir.LocalFunction, LocalFunctionInfo>{};
+  final localFunctionNames = <String, int>{};
+
+  LocalFunctionInfo generateLocalFunctionInfo(ir.LocalFunction localFunction) {
+    final name = _computeClosureName(localFunction);
+    localFunctionNames[name] = (localFunctionNames[name] ?? -1) + 1;
+    return LocalFunctionInfo(localFunction, name, localFunctionNames[name]);
+  }
 
   @override
   void visitFunctionExpression(ir.FunctionExpression node) {
     assert(localFunctions[node] == null);
-    localFunctions[node] = LocalFunctionInfo(node);
+    localFunctions[node] = generateLocalFunctionInfo(node);
     defaultExpression(node);
   }
 
   @override
   void visitFunctionDeclaration(ir.FunctionDeclaration node) {
     assert(localFunctions[node] == null);
-    localFunctions[node] = LocalFunctionInfo(node);
+    localFunctions[node] = generateLocalFunctionInfo(node);
     defaultStatement(node);
   }
 
@@ -1544,3 +1549,57 @@
     localFunctions[node.localFunction].isInvoked = true;
   }
 }
+
+// Returns a non-unique name for the given closure element.
+//
+// Must be kept logically identical to js_model/element_map_impl.dart.
+String _computeClosureName(ir.TreeNode treeNode) {
+  String reconstructConstructorName(ir.Member node) {
+    String className = node.enclosingClass.name;
+    if (node.name.text == '') {
+      return className;
+    } else {
+      return '$className\$${node.name}';
+    }
+  }
+
+  var parts = <String>[];
+  // First anonymous is called 'closure', outer ones called '' to give a
+  // compound name where increasing nesting level corresponds to extra
+  // underscores.
+  var anonymous = 'closure';
+  ir.TreeNode current = treeNode;
+  while (current != null) {
+    var node = current;
+    if (node is ir.FunctionExpression) {
+      parts.add(anonymous);
+      anonymous = '';
+    } else if (node is ir.FunctionDeclaration) {
+      String name = node.variable.name;
+      if (name != null && name != "") {
+        parts.add(entity_utils.operatorNameToIdentifier(name));
+      } else {
+        parts.add(anonymous);
+        anonymous = '';
+      }
+    } else if (node is ir.Class) {
+      parts.add(node.name);
+      break;
+    } else if (node is ir.Procedure) {
+      if (node.kind == ir.ProcedureKind.Factory) {
+        parts.add(reconstructConstructorName(node));
+      } else {
+        parts.add(entity_utils.operatorNameToIdentifier(node.name.text));
+      }
+    } else if (node is ir.Constructor) {
+      parts.add(reconstructConstructorName(node));
+      break;
+    } else if (node is ir.Field) {
+      // Add the field name for closures in field initializers.
+      String name = node.name?.text;
+      if (name != null) parts.add(name);
+    }
+    current = current.parent;
+  }
+  return parts.reversed.join('_');
+}
diff --git a/pkg/compiler/test/dump_info/data/closures.dart b/pkg/compiler/test/dump_info/data/closures.dart
index e8b525a..997b340 100644
--- a/pkg/compiler/test/dump_info/data/closures.dart
+++ b/pkg/compiler/test/dump_info/data/closures.dart
@@ -46,10 +46,13 @@
   "id": "library/memory:sdk/tests/web/native/main.dart::",
   "kind": "library",
   "name": "<unnamed>",
-  "size": 10341,
+  "size": 12431,
   "children": [
     "class/memory:sdk/tests/web/native/main.dart::Class1",
     "function/memory:sdk/tests/web/native/main.dart::main",
+    "function/memory:sdk/tests/web/native/main.dart::nested",
+    "function/memory:sdk/tests/web/native/main.dart::nested2",
+    "function/memory:sdk/tests/web/native/main.dart::siblings",
     "function/memory:sdk/tests/web/native/main.dart::topLevelMethod1",
     "function/memory:sdk/tests/web/native/main.dart::topLevelMethod2",
     "function/memory:sdk/tests/web/native/main.dart::topLevelMethod3",
@@ -66,7 +69,6 @@
   "imports": []
 }]
 */
-
 /*class: Class1:class=[{
   "id": "class/memory:sdk/tests/web/native/main.dart::Class1",
   "kind": "class",
@@ -1080,12 +1082,216 @@
   return local2;
 }
 
+/*member: nested:
+ closure=[
+  {
+  "id": "closure/memory:sdk/tests/web/native/main.dart::nested.nested_nested1",
+  "kind": "closure",
+  "name": "nested_nested1",
+  "size": 225,
+  "outputUnit": "outputUnit/main",
+  "parent": "function/memory:sdk/tests/web/native/main.dart::nested",
+  "function": "function/memory:sdk/tests/web/native/main.dart::nested.nested_nested1.call"
+},
+  {
+  "id": "closure/memory:sdk/tests/web/native/main.dart::nested.nested_nested1_nested2",
+  "kind": "closure",
+  "name": "nested_nested1_nested2",
+  "size": 227,
+  "outputUnit": "outputUnit/main",
+  "parent": "function/memory:sdk/tests/web/native/main.dart::nested",
+  "function": "function/memory:sdk/tests/web/native/main.dart::nested.nested_nested1_nested2.call"
+}],
+ function=[{
+  "id": "function/memory:sdk/tests/web/native/main.dart::nested",
+  "kind": "function",
+  "name": "nested",
+  "size": 568,
+  "outputUnit": "outputUnit/main",
+  "parent": "library/memory:sdk/tests/web/native/main.dart::",
+  "children": [
+    "closure/memory:sdk/tests/web/native/main.dart::nested.nested_nested1",
+    "closure/memory:sdk/tests/web/native/main.dart::nested.nested_nested1_nested2"
+  ],
+  "modifiers": {
+    "static": false,
+    "const": false,
+    "factory": false,
+    "external": false
+  },
+  "returnType": "dynamic",
+  "inferredReturnType": "[null]",
+  "parameters": [],
+  "sideEffects": "SideEffects(reads anything; writes anything)",
+  "inlinedCount": 0,
+  "code": "nested() {\n      var t1 = {};\n      t1.x = null;\n      new A.nested_nested1(t1).call$0();\n      t1.x.call$0();\n    }",
+  "type": "dynamic Function()"
+}],
+ holding=[
+  {"id":"function/dart:_rti::_setArrayType","mask":null},
+  {"id":"function/memory:sdk/tests/web/native/main.dart::nested.nested_nested1.call","mask":null},
+  {"id":"function/memory:sdk/tests/web/native/main.dart::nested.nested_nested1.call","mask":null}]
+*/
+dynamic nested() {
+  dynamic x;
+  nested1() {
+    nested2() => nested1;
+    x = nested2;
+  }
+
+  nested1();
+  x();
+}
+
+/*member: nested2:
+ closure=[
+  {
+  "id": "closure/memory:sdk/tests/web/native/main.dart::nested2.nested2_local1",
+  "kind": "closure",
+  "name": "nested2_local1",
+  "size": 195,
+  "outputUnit": "outputUnit/main",
+  "parent": "function/memory:sdk/tests/web/native/main.dart::nested2",
+  "function": "function/memory:sdk/tests/web/native/main.dart::nested2.nested2_local1.call"
+},
+  {
+  "id": "closure/memory:sdk/tests/web/native/main.dart::nested2.nested2_local1__closure",
+  "kind": "closure",
+  "name": "nested2_local1__closure",
+  "size": 193,
+  "outputUnit": "outputUnit/main",
+  "parent": "function/memory:sdk/tests/web/native/main.dart::nested2",
+  "function": "function/memory:sdk/tests/web/native/main.dart::nested2.nested2_local1__closure.call"
+},
+  {
+  "id": "closure/memory:sdk/tests/web/native/main.dart::nested2.nested2_local1_closure",
+  "kind": "closure",
+  "name": "nested2_local1_closure",
+  "size": 311,
+  "outputUnit": "outputUnit/main",
+  "parent": "function/memory:sdk/tests/web/native/main.dart::nested2",
+  "function": "function/memory:sdk/tests/web/native/main.dart::nested2.nested2_local1_closure.call"
+}],
+ function=[{
+  "id": "function/memory:sdk/tests/web/native/main.dart::nested2",
+  "kind": "function",
+  "name": "nested2",
+  "size": 764,
+  "outputUnit": "outputUnit/main",
+  "parent": "library/memory:sdk/tests/web/native/main.dart::",
+  "children": [
+    "closure/memory:sdk/tests/web/native/main.dart::nested2.nested2_local1",
+    "closure/memory:sdk/tests/web/native/main.dart::nested2.nested2_local1__closure",
+    "closure/memory:sdk/tests/web/native/main.dart::nested2.nested2_local1_closure"
+  ],
+  "modifiers": {
+    "static": false,
+    "const": false,
+    "factory": false,
+    "external": false
+  },
+  "returnType": "dynamic",
+  "inferredReturnType": "[null]",
+  "parameters": [],
+  "sideEffects": "SideEffects(reads anything; writes anything)",
+  "inlinedCount": 0,
+  "code": "nested2() {\n      A.print(new A.nested2_local1().call$0());\n    }",
+  "type": "dynamic Function()"
+}],
+ holding=[
+  {"id":"function/dart:_rti::_setArrayType","mask":null},
+  {"id":"function/dart:core::print","mask":null},
+  {"id":"function/memory:sdk/tests/web/native/main.dart::nested2.nested2_local1.call","mask":null},
+  {"id":"function/memory:sdk/tests/web/native/main.dart::nested2.nested2_local1.call","mask":null}]
+*/
+dynamic nested2() {
+  dynamic y;
+  int local1() {
+    return (() => 1 + (() => 2)())();
+  }
+
+  y = local1();
+  print(y);
+}
+
+/*member: siblings:
+ closure=[
+  {
+  "id": "closure/memory:sdk/tests/web/native/main.dart::siblings.siblings_local1",
+  "kind": "closure",
+  "name": "siblings_local1",
+  "size": 244,
+  "outputUnit": "outputUnit/main",
+  "parent": "function/memory:sdk/tests/web/native/main.dart::siblings",
+  "function": "function/memory:sdk/tests/web/native/main.dart::siblings.siblings_local1.call"
+},
+  {
+  "id": "closure/memory:sdk/tests/web/native/main.dart::siblings.siblings_local1_closure",
+  "kind": "closure",
+  "name": "siblings_local1_closure",
+  "size": 193,
+  "outputUnit": "outputUnit/main",
+  "parent": "function/memory:sdk/tests/web/native/main.dart::siblings",
+  "function": "function/memory:sdk/tests/web/native/main.dart::siblings.siblings_local1_closure.call"
+},
+  {
+  "id": "closure/memory:sdk/tests/web/native/main.dart::siblings.siblings_local1_closure%0",
+  "kind": "closure",
+  "name": "siblings_local1_closure",
+  "size": 197,
+  "outputUnit": "outputUnit/main",
+  "parent": "function/memory:sdk/tests/web/native/main.dart::siblings",
+  "function": "function/memory:sdk/tests/web/native/main.dart::siblings.siblings_local1_closure.call%0"
+}],
+ function=[{
+  "id": "function/memory:sdk/tests/web/native/main.dart::siblings",
+  "kind": "function",
+  "name": "siblings",
+  "size": 701,
+  "outputUnit": "outputUnit/main",
+  "parent": "library/memory:sdk/tests/web/native/main.dart::",
+  "children": [
+    "closure/memory:sdk/tests/web/native/main.dart::siblings.siblings_local1",
+    "closure/memory:sdk/tests/web/native/main.dart::siblings.siblings_local1_closure",
+    "closure/memory:sdk/tests/web/native/main.dart::siblings.siblings_local1_closure%0"
+  ],
+  "modifiers": {
+    "static": false,
+    "const": false,
+    "factory": false,
+    "external": false
+  },
+  "returnType": "dynamic",
+  "inferredReturnType": "[null]",
+  "parameters": [],
+  "sideEffects": "SideEffects(reads anything; writes anything)",
+  "inlinedCount": 0,
+  "code": "siblings() {\n      A.print(new A.siblings_local1().call$0());\n    }",
+  "type": "dynamic Function()"
+}],
+ holding=[
+  {"id":"function/dart:_rti::_setArrayType","mask":null},
+  {"id":"function/dart:core::print","mask":null},
+  {"id":"function/memory:sdk/tests/web/native/main.dart::siblings.siblings_local1.call","mask":null},
+  {"id":"function/memory:sdk/tests/web/native/main.dart::siblings.siblings_local1.call","mask":null}]
+*/
+dynamic siblings() {
+  int local1() {
+    int a = (() => 1)();
+    dynamic b = () => 2;
+    int c = (() => 3)();
+    return a + c;
+  }
+
+  print(local1());
+}
+
 /*member: main:
  function=[{
   "id": "function/memory:sdk/tests/web/native/main.dart::main",
   "kind": "function",
   "name": "main",
-  "size": 649,
+  "size": 706,
   "outputUnit": "outputUnit/main",
   "parent": "library/memory:sdk/tests/web/native/main.dart::",
   "children": [],
@@ -1100,7 +1306,7 @@
   "parameters": [],
   "sideEffects": "SideEffects(reads anything; writes anything)",
   "inlinedCount": 0,
-  "code": "main() {\n      var t2,\n        t1 = type$.int;\n      A.createRuntimeType(A.Class1$(t1).$ti._precomputed1);\n      A.Class1$(t1).method2$0();\n      A.Class1_Class1$fact2(t1).funcField.call$0();\n      A.Class1$(t1);\n      t2 = type$.double;\n      A.createRuntimeType(t2);\n      A.Class1$(t1).method4$1$0(t2);\n      A.Class1$(t1).method5$0();\n      A.Class1$(t1).method6$1$0(t2);\n      A.createRuntimeType(t2);\n      A.Class1_staticMethod2(t2);\n      A.Class1_staticMethod3();\n      A.Class1_staticMethod4(t2);\n      A.createRuntimeType(t2);\n      A.topLevelMethod2(t2);\n      A.topLevelMethod3();\n      A.topLevelMethod4(t2);\n      A.twoLocals();\n    }",
+  "code": "main() {\n      var t2,\n        t1 = type$.int;\n      A.createRuntimeType(A.Class1$(t1).$ti._precomputed1);\n      A.Class1$(t1).method2$0();\n      A.Class1_Class1$fact2(t1).funcField.call$0();\n      A.Class1$(t1);\n      t2 = type$.double;\n      A.createRuntimeType(t2);\n      A.Class1$(t1).method4$1$0(t2);\n      A.Class1$(t1).method5$0();\n      A.Class1$(t1).method6$1$0(t2);\n      A.createRuntimeType(t2);\n      A.Class1_staticMethod2(t2);\n      A.Class1_staticMethod3();\n      A.Class1_staticMethod4(t2);\n      A.createRuntimeType(t2);\n      A.topLevelMethod2(t2);\n      A.topLevelMethod3();\n      A.topLevelMethod4(t2);\n      A.twoLocals();\n      A.nested();\n      A.nested2();\n      A.siblings();\n    }",
   "type": "dynamic Function()"
 }],
  holding=[
@@ -1125,6 +1331,9 @@
   {"id":"function/memory:sdk/tests/web/native/main.dart::Class1.staticMethod2","mask":null},
   {"id":"function/memory:sdk/tests/web/native/main.dart::Class1.staticMethod3","mask":null},
   {"id":"function/memory:sdk/tests/web/native/main.dart::Class1.staticMethod4","mask":null},
+  {"id":"function/memory:sdk/tests/web/native/main.dart::nested","mask":null},
+  {"id":"function/memory:sdk/tests/web/native/main.dart::nested2","mask":null},
+  {"id":"function/memory:sdk/tests/web/native/main.dart::siblings","mask":null},
   {"id":"function/memory:sdk/tests/web/native/main.dart::topLevelMethod1","mask":"inlined"},
   {"id":"function/memory:sdk/tests/web/native/main.dart::topLevelMethod1","mask":null},
   {"id":"function/memory:sdk/tests/web/native/main.dart::topLevelMethod2","mask":null},
@@ -1149,4 +1358,7 @@
   topLevelMethod3();
   topLevelMethod4<double>();
   twoLocals();
+  nested();
+  nested2();
+  siblings();
 }