[dart2js] Use existing J-model entities where possible when converting from kernel to JS entity maps.

Change-Id: I1bcfecb313fab1e7dc03bf1d8c14b8410226f431
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/309140
Commit-Queue: Nate Biggs <[email protected]>
Reviewed-by: Sigmund Cherem <[email protected]>

Author: natebiggs

pkg/compiler/lib/src/elements/indexed.dart

@@ -132,14 +132,20 @@ class EntityDataMap<E extends _Indexed, D> extends EntityDataMapBase<E, D> {
   ///
   /// The index of [entity] is set to match its index in the entity and data
   /// lists in this map.
-  E0 register<E0 extends E, D0 extends D>(E0 entity, D0 data) {
+  E0 register<E0 extends E, D0 extends D>(E0 entity, D0 data,
+      {bool setEntityIndex = true}) {
     assert(
         !_closed, "Trying to register $entity @ ${_list.length} when closed.");
     assert(
         _list.length == _data.length,
         'Data list length ${_data.length} inconsistent '
         'with entity list length ${_list.length}.');
-    entity._index = _list.length;
+    if (setEntityIndex) {
+      entity._index = _list.length;
+    } else {
+      assert(entity._index == _list.length,
+          'Trying to register entity with in incorrect order. ');
+    }
     _list.add(entity);
     _size++;
     assert(data != null);
@@ -197,7 +203,8 @@ class EntityDataEnvMap<E extends _Indexed, D, V>
   /// The index of [entity] is set to match its index in the entity, data and
   /// environment lists in this map.
   E0 register<E0 extends E, D0 extends D, V0 extends V>(
-      E0 entity, D0 data, V0 env) {
+      E0 entity, D0 data, V0 env,
+      {bool setEntityIndex = true}) {
     assert(
         !_closed, "Trying to register $entity @ ${_list.length} when closed.");
     assert(
@@ -208,7 +215,12 @@ class EntityDataEnvMap<E extends _Indexed, D, V>
         _list.length == _env.length,
         'Env list length ${_env.length} inconsistent '
         'with entity list length ${_list.length}.');
-    entity._index = _list.length;
+    if (setEntityIndex) {
+      entity._index = _list.length;
+    } else {
+      assert(entity._index == _list.length,
+          'Trying to register entity with in incorrect order. ');
+    }
     _list.add(entity);
     _size++;
     assert(data != null);

pkg/compiler/lib/src/js_backend/native_data.dart

@@ -362,10 +362,8 @@ class NativeBasicData {
     Map<ClassEntity, NativeClassTag> nativeClassTagInfo =
         <ClassEntity, NativeClassTag>{};
     _nativeClassTagInfo.forEach((ClassEntity cls, NativeClassTag tag) {
-      ClassEntity? backendClass = map.toBackendClass(cls);
-      if (backendClass != null) {
-        nativeClassTagInfo[backendClass] = tag;
-      }
+      final backendClass = map.toBackendClass(cls);
+      nativeClassTagInfo[backendClass] = tag;
     });
     Map<LibraryEntity, String> jsInteropLibraries =
         map.toBackendLibraryMap(_jsInteropLibraries, identity);

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

@@ -142,35 +142,29 @@ class JsKernelToElementMap implements JsToElementMap, IrToElementMap {
     for (int libraryIndex = 0;
         libraryIndex < _elementMap.libraries.length;
         libraryIndex++) {
-      IndexedLibrary oldLibrary =
-          _elementMap.libraries.getEntity(libraryIndex)!;
-      KLibraryEnv oldEnv = _elementMap.libraries.getEnv(oldLibrary);
-      KLibraryData data = _elementMap.libraries.getData(oldLibrary);
-      IndexedLibrary newLibrary = JLibrary(oldLibrary.name!,
-          oldLibrary.canonicalUri, oldLibrary.isNonNullableByDefault);
+      IndexedLibrary library = _elementMap.libraries.getEntity(libraryIndex)!;
+      KLibraryEnv oldEnv = _elementMap.libraries.getEnv(library);
+      KLibraryData data = _elementMap.libraries.getData(library);
       JLibraryEnv newEnv = oldEnv.convert(_elementMap, liveMemberUsage);
       libraryMap[oldEnv.library] =
           libraries.register<IndexedLibrary, JLibraryData, JLibraryEnv>(
-              newLibrary, data.convert(), newEnv);
-      assert(newLibrary.libraryIndex == oldLibrary.libraryIndex);
+              library, data.convert(), newEnv,
+              setEntityIndex: false);
       programEnv.registerLibrary(newEnv);
     }
     // TODO(johnniwinther): Filter unused classes.
     for (int classIndex = 0;
         classIndex < _elementMap.classes.length;
         classIndex++) {
-      IndexedClass oldClass = _elementMap.classes.getEntity(classIndex)!;
-      KClassEnv env = _elementMap.classes.getEnv(oldClass);
-      KClassData data = _elementMap.classes.getData(oldClass);
-      final oldLibrary = oldClass.library as IndexedLibrary;
-      LibraryEntity newLibrary = libraries.getEntity(oldLibrary.libraryIndex)!;
-      IndexedClass newClass = JClass(newLibrary as JLibrary, oldClass.name,
-          isAbstract: oldClass.isAbstract);
+      IndexedClass cls = _elementMap.classes.getEntity(classIndex)!;
+      KClassEnv env = _elementMap.classes.getEnv(cls);
+      KClassData data = _elementMap.classes.getData(cls);
+      final library = cls.library as JLibrary;
       JClassEnv newEnv = env.convert(_elementMap, liveMemberUsage,
           liveAbstractMembers, (ir.Library library) => libraryMap[library]!);
-      classMap[env.cls] = classes.register(newClass, data.convert(), newEnv);
-      assert(newClass.classIndex == oldClass.classIndex);
-      libraries.getEnv(newLibrary).registerClass(newClass.name, newEnv);
+      classMap[env.cls] =
+          classes.register(cls, data.convert(), newEnv, setEntityIndex: false);
+      libraries.getEnv(library).registerClass(cls.name, newEnv);
     }
 
     for (int memberIndex = 0;
@@ -183,68 +177,45 @@ class JsKernelToElementMap implements JsToElementMap, IrToElementMap {
         continue;
       }
       KMemberData data = _elementMap.members.getData(oldMember);
-      final oldLibrary = oldMember.library as IndexedLibrary;
-      final oldClass = oldMember.enclosingClass as IndexedClass?;
-      JLibrary newLibrary =
-          libraries.getEntity(oldLibrary.libraryIndex) as JLibrary;
-      JClass? newClass = oldClass != null
-          ? classes.getEntity(oldClass.classIndex) as JClass?
-          : null;
-      IndexedMember newMember;
+      final library = oldMember.library as JLibrary;
+      final cls = oldMember.enclosingClass as JClass?;
+      IndexedMember? newMember;
       Name memberName = oldMember.memberName;
-      if (oldMember is IndexedField) {
-        final field = oldMember;
-        newMember = JField(newLibrary, newClass, memberName,
-            isStatic: field.isStatic,
-            isAssignable: field.isAssignable,
-            isConst: field.isConst);
-      } else if (oldMember is ConstructorEntity) {
-        final constructor = oldMember as IndexedConstructor;
-        ParameterStructure parameterStructure =
-            annotations.hasNoElision(constructor) || memberUsage == null
-                ? constructor.parameterStructure
-                : memberUsage.invokedParameters!;
-        if (constructor.isFactoryConstructor) {
-          // TODO(redemption): This should be a JFunction.
-          newMember = JFactoryConstructor(
-              newClass!, memberName, parameterStructure,
-              isExternal: constructor.isExternal,
-              isConst: constructor.isConst,
-              isFromEnvironmentConstructor:
-                  constructor.isFromEnvironmentConstructor);
-        } else {
-          newMember = JGenerativeConstructor(
-              newClass!, memberName, parameterStructure,
-              isExternal: constructor.isExternal, isConst: constructor.isConst);
+
+      // Only create a new entity if some parameters are unused and can be
+      // elided.
+      if (!annotations.hasNoElision(oldMember) &&
+          memberUsage != null &&
+          oldMember is IndexedFunction &&
+          !identical(
+              oldMember.parameterStructure, memberUsage.invokedParameters)) {
+        if (oldMember is ConstructorEntity) {
+          final newParameters = memberUsage.invokedParameters!;
+          final constructor = oldMember as ConstructorEntity;
+          if (constructor.isFactoryConstructor) {
+            // TODO(redemption): This should be a JFunction.
+            newMember = JFactoryConstructor(cls!, memberName, newParameters,
+                isExternal: constructor.isExternal,
+                isConst: constructor.isConst,
+                isFromEnvironmentConstructor:
+                    constructor.isFromEnvironmentConstructor);
+          } else {
+            newMember = JGenerativeConstructor(cls!, memberName, newParameters,
+                isExternal: constructor.isExternal,
+                isConst: constructor.isConst);
+          }
+        } else if (oldMember.isFunction && !oldMember.isAbstract) {
+          final newParameters = memberUsage.invokedParameters!;
+          newMember = JMethod(
+              library, cls, memberName, newParameters, oldMember.asyncMarker,
+              isStatic: oldMember.isStatic,
+              isExternal: oldMember.isExternal,
+              isAbstract: oldMember.isAbstract);
         }
-      } else if (oldMember.isGetter) {
-        final getter = oldMember as IndexedFunction;
-        newMember = JGetter(
-            newLibrary, newClass, memberName, getter.asyncMarker,
-            isStatic: getter.isStatic,
-            isExternal: getter.isExternal,
-            isAbstract: getter.isAbstract);
-      } else if (oldMember.isSetter) {
-        final setter = oldMember as IndexedFunction;
-        newMember = JSetter(newLibrary, newClass, memberName,
-            isStatic: setter.isStatic,
-            isExternal: setter.isExternal,
-            isAbstract: setter.isAbstract);
-      } else {
-        final function = oldMember as IndexedFunction;
-        ParameterStructure parameterStructure =
-            annotations.hasNoElision(function) ||
-                    memberUsage == null ||
-                    function.isAbstract
-                ? function.parameterStructure
-                : memberUsage.invokedParameters!;
-        newMember = JMethod(newLibrary, newClass, memberName,
-            parameterStructure, function.asyncMarker,
-            isStatic: function.isStatic,
-            isExternal: function.isExternal,
-            isAbstract: function.isAbstract);
       }
-      members.register(newMember, data.convert());
+      members.register(newMember ?? oldMember, data.convert(),
+          setEntityIndex: newMember != null);
+      newMember ??= oldMember;
       assert(
           newMember.memberIndex == oldMember.memberIndex,
           "Member index mismatch: "
@@ -265,28 +236,33 @@ class JsKernelToElementMap implements JsToElementMap, IrToElementMap {
           _elementMap.typeVariables.getEntity(typeVariableIndex)!;
       KTypeVariableData oldTypeVariableData =
           _elementMap.typeVariables.getData(oldTypeVariable);
-      Entity? newTypeDeclaration;
-      if (oldTypeVariable.typeDeclaration is ClassEntity) {
-        final cls = oldTypeVariable.typeDeclaration as IndexedClass;
-        newTypeDeclaration = classes.getEntity(cls.classIndex);
-        // TODO(johnniwinther): Skip type variables of unused classes.
-      } else {
-        assert(oldTypeVariable.typeDeclaration is MemberEntity);
+
+      // [JLocalTypeVariable] can have [Local] as a `typeDeclaration` but those
+      // should never be inserted into the TypeVariable entity map.
+      assert(oldTypeVariable.typeDeclaration is ClassEntity ||
+          oldTypeVariable.typeDeclaration is MemberEntity);
+
+      MemberEntity? newTypeDeclaration;
+      // TODO(johnniwinther): Skip type variables of unused classes.
+      if (oldTypeVariable.typeDeclaration is MemberEntity) {
         final member = oldTypeVariable.typeDeclaration as IndexedMember;
         newTypeDeclaration = members.getEntity(member.memberIndex);
         if (newTypeDeclaration == null) {
           typeVariables.skipIndex(typeVariableIndex);
           continue;
         }
       }
-      IndexedTypeVariable newTypeVariable = createTypeVariable(
-              newTypeDeclaration!, oldTypeVariable.name!, oldTypeVariable.index)
-          as IndexedTypeVariable;
+      IndexedTypeVariable? newTypeVariable;
+      if (newTypeDeclaration != null) {
+        newTypeVariable = createTypeVariable(
+            newTypeDeclaration,
+            oldTypeVariable.name!,
+            oldTypeVariable.index) as IndexedTypeVariable;
+      }
       typeVariableMap[oldTypeVariableData.node] =
           typeVariables.register<IndexedTypeVariable, JTypeVariableData>(
-              newTypeVariable, oldTypeVariableData.copy());
-      assert(newTypeVariable.typeVariableIndex ==
-          oldTypeVariable.typeVariableIndex);
+              newTypeVariable ?? oldTypeVariable, oldTypeVariableData.copy(),
+              setEntityIndex: newTypeVariable != null);
     }
     // TODO(johnniwinther): We should close the environment in the beginning of
     // this constructor but currently we need the [MemberEntity] to query if the

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

@@ -779,7 +779,7 @@ class JTypeVariable extends IndexedTypeVariable {
   factory JTypeVariable.readFromDataSource(DataSourceReader source) {
     source.begin(tag);
     JTypeVariableKind kind = source.readEnum(JTypeVariableKind.values);
-    Entity? typeDeclaration;
+    Entity typeDeclaration;
     switch (kind) {
       case JTypeVariableKind.cls:
         typeDeclaration = source.readClass();