Reimplement member usage tracking

We now track member usage in terms of static, dynamic and super access
for reads, writes and invocations. The information collected during
closed world computation is now the basis for the potential member usage
in codegen, thus ensuring that we cannot conclude in codegen that for
instance a field is read dynamically when the closed world knows that
it is never read dynamically.

Closes #36516

Change-Id: I3a1cb87c71268c34bcd67e14a035d9d1be324ab0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/100840
Commit-Queue: Johnni Winther <[email protected]>
Reviewed-by: Sigmund Cherem <[email protected]>

Author: johnniwinther

pkg/compiler/lib/src/deferred_load.dart

@@ -296,7 +296,7 @@ abstract class DeferredLoadTask extends CompilerTask {
               _collectTypeArgumentDependencies(
                   staticUse.type.typeArguments, dependencies);
               break;
-            case StaticUseKind.INVOKE:
+            case StaticUseKind.STATIC_INVOKE:
             case StaticUseKind.CLOSURE_CALL:
             case StaticUseKind.DIRECT_INVOKE:
               // TODO(johnniwinther): Use rti need data to skip unneeded type

pkg/compiler/lib/src/enqueue.dart

@@ -207,7 +207,7 @@ abstract class EnqueuerImpl extends Enqueuer {
 
   /// Check enqueuer consistency after the queue has been closed.
   bool checkEnqueuerConsistency(ElementEnvironment elementEnvironment) {
-    task.measure(() {
+    task.measureSubtask('resolution.check', () {
       // Run through the classes and see if we need to enqueue more methods.
       for (ClassEntity classElement
           in worldBuilder.directlyInstantiatedClasses) {
@@ -284,7 +284,7 @@ class ResolutionEnqueuer extends EnqueuerImpl {
       {ConstructorEntity constructor,
       bool nativeUsage: false,
       bool globalDependency: false}) {
-    task.measure(() {
+    task.measureSubtask('resolution.typeUse', () {
       _worldBuilder.registerTypeInstantiation(type, _applyClassUse,
           constructor: constructor);
       listener.registerInstantiatedType(type,
@@ -307,7 +307,7 @@ class ResolutionEnqueuer extends EnqueuerImpl {
         _reporter.internalError(member,
             'Unenqueued use of $member: ${useSet.iterable(MemberUse.values)}');
       }
-    }, dryRun: true);
+    }, checkEnqueuerConsistency: true);
   }
 
   /// Callback for applying the use of a [member].
@@ -343,14 +343,14 @@ class ResolutionEnqueuer extends EnqueuerImpl {
 
   @override
   void processDynamicUse(DynamicUse dynamicUse) {
-    task.measure(() {
+    task.measureSubtask('resolution.dynamicUse', () {
       _worldBuilder.registerDynamicUse(dynamicUse, _applyMemberUse);
     });
   }
 
   @override
   void processConstantUse(ConstantUse constantUse) {
-    task.measure(() {
+    task.measureSubtask('resolution.constantUse', () {
       if (_worldBuilder.registerConstantUse(constantUse)) {
         applyImpact(listener.registerUsedConstant(constantUse.value),
             impactSource: 'constant use');
@@ -361,18 +361,20 @@ class ResolutionEnqueuer extends EnqueuerImpl {
 
   @override
   void processStaticUse(StaticUse staticUse) {
-    _worldBuilder.registerStaticUse(staticUse, _applyMemberUse);
-    // TODO(johnniwinther): Add `ResolutionWorldBuilder.registerConstructorUse`
-    // for these:
-    switch (staticUse.kind) {
-      case StaticUseKind.CONSTRUCTOR_INVOKE:
-      case StaticUseKind.CONST_CONSTRUCTOR_INVOKE:
-        _registerInstantiatedType(staticUse.type,
-            constructor: staticUse.element, globalDependency: false);
-        break;
-      default:
-        break;
-    }
+    task.measureSubtask('resolution.staticUse', () {
+      _worldBuilder.registerStaticUse(staticUse, _applyMemberUse);
+      // TODO(johnniwinther): Add `ResolutionWorldBuilder.registerConstructorUse`
+      // for these:
+      switch (staticUse.kind) {
+        case StaticUseKind.CONSTRUCTOR_INVOKE:
+        case StaticUseKind.CONST_CONSTRUCTOR_INVOKE:
+          _registerInstantiatedType(staticUse.type,
+              constructor: staticUse.element, globalDependency: false);
+          break;
+        default:
+          break;
+      }
+    });
   }
 
   @override

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

@@ -107,7 +107,7 @@ class CodegenEnqueuer extends EnqueuerImpl {
 
   void _registerInstantiatedType(InterfaceType type,
       {bool nativeUsage: false}) {
-    task.measure(() {
+    task.measureSubtask('codegen.typeUse', () {
       _worldBuilder.registerTypeInstantiation(type, _applyClassUse);
       listener.registerInstantiatedType(type, nativeUsage: nativeUsage);
     });
@@ -127,7 +127,7 @@ class CodegenEnqueuer extends EnqueuerImpl {
         failedAt(member,
             'Unenqueued use of $member: ${useSet.iterable(MemberUse.values)}');
       }
-    }, dryRun: true);
+    }, checkEnqueuerConsistency: true);
   }
 
   /// Callback for applying the use of a [cls].
@@ -160,26 +160,28 @@ class CodegenEnqueuer extends EnqueuerImpl {
 
   @override
   void processDynamicUse(DynamicUse dynamicUse) {
-    task.measure(() {
+    task.measureSubtask('codegen.dynamicUse', () {
       _worldBuilder.registerDynamicUse(dynamicUse, _applyMemberUse);
     });
   }
 
   @override
   void processStaticUse(StaticUse staticUse) {
-    _worldBuilder.registerStaticUse(staticUse, _applyMemberUse);
-    switch (staticUse.kind) {
-      case StaticUseKind.CONSTRUCTOR_INVOKE:
-      case StaticUseKind.CONST_CONSTRUCTOR_INVOKE:
-        processTypeUse(new TypeUse.instantiation(staticUse.type));
-        break;
-      case StaticUseKind.INLINING:
-        // TODO(johnniwinther): Should this be tracked with _MemberUsage ?
-        listener.registerUsedElement(staticUse.element);
-        break;
-      default:
-        break;
-    }
+    task.measureSubtask('codegen.staticUse', () {
+      _worldBuilder.registerStaticUse(staticUse, _applyMemberUse);
+      switch (staticUse.kind) {
+        case StaticUseKind.CONSTRUCTOR_INVOKE:
+        case StaticUseKind.CONST_CONSTRUCTOR_INVOKE:
+          processTypeUse(new TypeUse.instantiation(staticUse.type));
+          break;
+        case StaticUseKind.INLINING:
+          // TODO(johnniwinther): Should this be tracked with _MemberUsage ?
+          listener.registerUsedElement(staticUse.element);
+          break;
+        default:
+          break;
+      }
+    });
   }
 
   @override
@@ -229,7 +231,7 @@ class CodegenEnqueuer extends EnqueuerImpl {
 
   @override
   void processConstantUse(ConstantUse constantUse) {
-    task.measure(() {
+    task.measureSubtask('codegen.constantUse', () {
       if (_worldBuilder.registerConstantUse(constantUse)) {
         applyImpact(listener.registerUsedConstant(constantUse.value));
         _recentConstants = true;

pkg/compiler/lib/src/js_emitter/parameter_stub_generator.dart

@@ -53,7 +53,7 @@ class ParameterStubGenerator {
       _closedWorld.elementEnvironment;
 
   bool needsSuperGetter(FunctionEntity element) =>
-      _codegenWorld.methodsNeedingSuperGetter.contains(element);
+      _codegenWorld.methodsNeedsSuperGetter(element);
 
   /// Generates stubs to fill in missing optional named or positional arguments
   /// and missing type arguments.  Returns `null` if no stub is needed.

pkg/compiler/lib/src/js_emitter/program_builder/program_builder.dart

@@ -477,7 +477,7 @@ class ProgramBuilder {
 
   void _addJsInteropStubs(LibrariesMap librariesMap) {
     if (_classes.containsKey(_commonElements.objectClass)) {
-      var toStringInvocation = _namer.invocationName(Selectors.toString_);
+      js.Name toStringInvocation = _namer.invocationName(Selectors.toString_);
       // TODO(jacobr): register toString as used so that it is always accessible
       // from JavaScript.
       _classes[_commonElements.objectClass].callStubs.add(_buildStubMethod(
@@ -492,22 +492,23 @@ class ProgramBuilder {
     // a regular getter that returns a JavaScript function and tearing off
     // a method in the case where there exist multiple JavaScript classes
     // that conflict on whether the member is a getter or a method.
-    var interceptorClass = _classes[_commonElements.jsJavaScriptObjectClass];
-    var stubNames = new Set<String>();
+    Class interceptorClass = _classes[_commonElements.jsJavaScriptObjectClass];
+    Set<String> stubNames = {};
     librariesMap
         .forEach((LibraryEntity library, List<ClassEntity> classElements, _) {
       for (ClassEntity cls in classElements) {
         if (_nativeData.isJsInteropClass(cls)) {
           _elementEnvironment.forEachLocalClassMember(cls,
               (MemberEntity member) {
-            var jsName = _nativeData.computeUnescapedJSInteropName(member.name);
+            String jsName =
+                _nativeData.computeUnescapedJSInteropName(member.name);
             if (!member.isInstanceMember) return;
             if (member.isGetter || member.isField || member.isFunction) {
-              var selectors =
+              Iterable<Selector> selectors =
                   _codegenWorld.getterInvocationsByName(member.name);
               if (selectors != null && !selectors.isEmpty) {
-                for (var selector in selectors.keys) {
-                  var stubName = _namer.invocationName(selector);
+                for (Selector selector in selectors) {
+                  js.Name stubName = _namer.invocationName(selector);
                   if (stubNames.add(stubName.key)) {
                     interceptorClass.callStubs.add(_buildStubMethod(stubName,
                         js.js('function(obj) { return obj.# }', [jsName]),
@@ -518,7 +519,7 @@ class ProgramBuilder {
             }
 
             if (member.isSetter || (member.isField && !member.isConst)) {
-              var selectors =
+              Iterable<Selector> selectors =
                   _codegenWorld.setterInvocationsByName(member.name);
               if (selectors != null && !selectors.isEmpty) {
                 var stubName = _namer.setterForMember(member);
@@ -765,11 +766,11 @@ class ProgramBuilder {
           assert(!field.needsUncheckedSetter);
           FieldEntity element = field.element;
           js.Expression code = _generatedCode[element];
+          assert(code != null, "No setter code for field: $field");
           if (code == null) {
-            // TODO(johnniwinther): Static types are not honoured in the dynamic
-            // uses created in codegen, leading to dead code, as known by the
-            // closed world computation, being triggered by the codegen
-            // enqueuer. We cautiously generate an empty function for this case.
+            // This should never occur because codegen member usage is now
+            // limited by closed world member usage. In the case we've missed a
+            // spot we cautiously generate an empty function.
             code = js.js("function() {}");
           }
           js.Name name = _namer.deriveSetterName(field.accessorName);
@@ -901,9 +902,8 @@ class ProgramBuilder {
         isClosureCallMethod = true;
       } else {
         // Careful with operators.
-        canTearOff = _codegenWorld.hasInvokedGetter(element);
-        assert(canTearOff ||
-            !_codegenWorld.methodsNeedingSuperGetter.contains(element));
+        canTearOff = _codegenWorld.hasInvokedGetter(element) ||
+            _codegenWorld.methodsNeedsSuperGetter(element);
         tearOffName = _namer.getterForElement(element);
       }
     }

pkg/compiler/lib/src/js_emitter/startup_emitter/fragment_emitter.dart

@@ -1084,11 +1084,12 @@ class FragmentEmitter {
     js.Expression code;
     if (field.isElided) {
       ConstantValue constantValue = field.constantValue;
+      assert(
+          constantValue != null, "No constant value for elided field: $field");
       if (constantValue == null) {
-        // TODO(johnniwinther): Static types are not honoured in the dynamic
-        // uses created in codegen, leading to dead code, as known by the closed
-        // world computation, being triggered by the codegen enqueuer. We
-        // cautiously generate a null constant for this case.
+        // This should never occur because codegen member usage is now limited
+        // by closed world member usage. In the case we've missed a spot we
+        // cautiously generate a null constant.
         constantValue = new NullConstantValue();
       }
       code = js.js(

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

@@ -32,6 +32,7 @@ import '../serialization/serialization.dart';
 import '../universe/class_hierarchy.dart';
 import '../universe/class_set.dart';
 import '../universe/function_set.dart' show FunctionSet;
+import '../universe/member_usage.dart';
 import '../universe/selector.dart';
 import '../world.dart';
 import 'element_map.dart';
@@ -93,6 +94,8 @@ class JsClosedWorld implements JClosedWorld {
   final OutputUnitData outputUnitData;
   Sorter _sorter;
 
+  final Map<MemberEntity, MemberAccess> memberAccess;
+
   JsClosedWorld(
       this.elementMap,
       this.nativeData,
@@ -113,7 +116,8 @@ class JsClosedWorld implements JClosedWorld {
       this.annotationsData,
       this.globalLocalsMap,
       this.closureDataLookup,
-      this.outputUnitData) {
+      this.outputUnitData,
+      this.memberAccess) {
     _abstractValueDomain = abstractValueStrategy.createDomain(this);
   }
 
@@ -167,6 +171,9 @@ class JsClosedWorld implements JClosedWorld {
     OutputUnitData outputUnitData =
         new OutputUnitData.readFromDataSource(source);
 
+    Map<MemberEntity, MemberAccess> memberAccess =
+        source.readMemberMap(() => new MemberAccess.readFromDataSource(source));
+
     source.end(tag);
 
     return new JsClosedWorld(
@@ -189,7 +196,8 @@ class JsClosedWorld implements JClosedWorld {
         annotationsData,
         globalLocalsMap,
         closureData,
-        outputUnitData);
+        outputUnitData,
+        memberAccess);
   }
 
   /// Serializes this [JsClosedWorld] to [sink].
@@ -217,6 +225,8 @@ class JsClosedWorld implements JClosedWorld {
     annotationsData.writeToDataSink(sink);
     closureDataLookup.writeToDataSink(sink);
     outputUnitData.writeToDataSink(sink);
+    sink.writeMemberMap(
+        memberAccess, (MemberAccess access) => access.writeToDataSink(sink));
     sink.end(tag);
   }
 
@@ -539,6 +549,11 @@ class JsClosedWorld implements JClosedWorld {
     return elementEnvironment.isMixinApplication(cls) &&
         !elementEnvironment.isUnnamedMixinApplication(cls);
   }
+
+  @override
+  MemberAccess getMemberAccess(MemberEntity member) {
+    return memberAccess[member];
+  }
 }
 
 class KernelSorter implements Sorter {

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

@@ -29,6 +29,7 @@ import '../options.dart';
 import '../universe/class_hierarchy.dart';
 import '../universe/class_set.dart';
 import '../universe/feature.dart';
+import '../universe/member_usage.dart';
 import '../universe/selector.dart';
 import '../world.dart';
 import 'closure.dart';
@@ -202,6 +203,11 @@ class JsClosedWorldBuilder {
     OutputUnitData outputUnitData =
         _convertOutputUnitData(map, kOutputUnitData, closureData);
 
+    Map<MemberEntity, MemberAccess> memberAccess = map.toBackendMemberMap(
+        closedWorld.liveMemberUsage,
+        (MemberUsage usage) =>
+            new MemberAccess(usage.reads, usage.writes, usage.invokes));
+
     return new JsClosedWorld(
         _elementMap,
         nativeData,
@@ -226,7 +232,8 @@ class JsClosedWorldBuilder {
         annotationsData,
         _globalLocalsMap,
         closureData,
-        outputUnitData);
+        outputUnitData,
+        memberAccess);
   }
 
   BackendUsage _convertBackendUsage(
@@ -487,7 +494,7 @@ class JsClosedWorldBuilder {
         map.toBackendLibrary,
         convertClassMap,
         convertMemberMap,
-        (m) => convertMap<ConstantValue, OutputUnit>(
+        (m) => convertMap<ConstantValue, OutputUnit, OutputUnit>(
             m, map.toBackendConstant, (v) => v));
   }
 }
@@ -633,20 +640,20 @@ abstract class JsToFrontendMap {
     return convertMap(map, toBackendClass, convert);
   }
 
-  Map<MemberEntity, V> toBackendMemberMap<V>(
-      Map<MemberEntity, V> map, V convert(V value)) {
+  Map<MemberEntity, V2> toBackendMemberMap<V1, V2>(
+      Map<MemberEntity, V1> map, V2 convert(V1 value)) {
     return convertMap(map, toBackendMember, convert);
   }
 }
 
 E identity<E>(E element) => element;
 
-Map<K, V> convertMap<K, V>(
-    Map<K, V> map, K convertKey(K key), V convertValue(V value)) {
-  Map<K, V> newMap = <K, V>{};
-  map.forEach((K key, V value) {
+Map<K, V2> convertMap<K, V1, V2>(
+    Map<K, V1> map, K convertKey(K key), V2 convertValue(V1 value)) {
+  Map<K, V2> newMap = <K, V2>{};
+  map.forEach((K key, V1 value) {
     K newKey = convertKey(key);
-    V newValue = convertValue(value);
+    V2 newValue = convertValue(value);
     if (newKey != null && newValue != null) {
       // Entities that are not used don't have a corresponding backend entity.
       newMap[newKey] = newValue;