Collect types deeply for deferred loading

Change-Id: I7bbd9499975c78fcdcfa88ceba31cb2e8e5a7cc6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/97639
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>

pkg/compiler/lib/src/deferred_load.dart

@@ -8,7 +8,8 @@ import 'dart:collection' show Queue;
 
 import 'common/tasks.dart' show CompilerTask;
 import 'common.dart';
-import 'common_elements.dart' show ElementEnvironment, KElementEnvironment;
+import 'common_elements.dart'
+    show CommonElements, ElementEnvironment, KElementEnvironment;
 import 'compiler.dart' show Compiler;
 import 'constants/values.dart'
     show
@@ -144,6 +145,9 @@ abstract class DeferredLoadTask extends CompilerTask {
 
   KElementEnvironment get elementEnvironment =>
       compiler.frontendStrategy.elementEnvironment;
+
+  CommonElements get commonElements => compiler.frontendStrategy.commonElements;
+
   DiagnosticReporter get reporter => compiler.reporter;
 
   /// Given [imports] that refer to an element from a library, determine whether
@@ -247,42 +251,17 @@ abstract class DeferredLoadTask extends CompilerTask {
   void collectConstantsInBody(MemberEntity element, Dependencies dependencies);
 
   /// Recursively collects all the dependencies of [type].
-  void _collectTypeDependencies(DartType type, Dependencies dependencies) {
-    if (type is FunctionType) {
-      _collectFunctionTypeDependencies(type, dependencies);
-    } else if (type is TypedefType) {
-      _collectTypeArgumentDependencies(type.typeArguments, dependencies);
-      _collectTypeDependencies(type.unaliased, dependencies);
-    } else if (type is InterfaceType) {
-      _collectTypeArgumentDependencies(type.typeArguments, dependencies);
-      // TODO(sigmund): when we are able to split classes from types in our
-      // runtime-type representation, this should track type.element as a type
-      // dependency instead.
-      dependencies.addClass(type.element);
-    }
+  void _collectTypeDependencies(DartType type, Dependencies dependencies,
+      [ImportEntity import]) {
+    new TypeDependencyVisitor(dependencies, import, commonElements).visit(type);
   }
 
   void _collectTypeArgumentDependencies(
-      Iterable<DartType> typeArguments, Dependencies dependencies) {
+      Iterable<DartType> typeArguments, Dependencies dependencies,
+      [ImportEntity import]) {
     if (typeArguments == null) return;
-    typeArguments.forEach((t) => _collectTypeDependencies(t, dependencies));
-  }
-
-  void _collectFunctionTypeDependencies(
-      FunctionType type, Dependencies dependencies) {
-    for (FunctionTypeVariable typeVariable in type.typeVariables) {
-      _collectTypeDependencies(typeVariable.bound, dependencies);
-    }
-    for (DartType argumentType in type.parameterTypes) {
-      _collectTypeDependencies(argumentType, dependencies);
-    }
-    for (DartType argumentType in type.optionalParameterTypes) {
-      _collectTypeDependencies(argumentType, dependencies);
-    }
-    for (DartType argumentType in type.namedParameterTypes) {
-      _collectTypeDependencies(argumentType, dependencies);
-    }
-    _collectTypeDependencies(type.returnType, dependencies);
+    new TypeDependencyVisitor(dependencies, import, commonElements)
+        .visitList(typeArguments);
   }
 
   /// Extract any dependencies that are known from the impact of [element].
@@ -301,8 +280,7 @@ abstract class DeferredLoadTask extends CompilerTask {
                 failedAt(usedEntity, "Unexpected static use $staticUse."));
             KLocalFunction localFunction = usedEntity;
             // TODO(sra): Consult KClosedWorld to see if signature is needed.
-            _collectFunctionTypeDependencies(
-                localFunction.functionType, dependencies);
+            _collectTypeDependencies(localFunction.functionType, dependencies);
             dependencies.localFunctions.add(localFunction);
           }
           switch (staticUse.kind) {
@@ -339,11 +317,8 @@ abstract class DeferredLoadTask extends CompilerTask {
               }
               break;
             case TypeUseKind.CONST_INSTANTIATION:
-              if (type.isInterfaceType) {
-                InterfaceType interface = type;
-                dependencies.addClass(
-                    interface.element, typeUse.deferredImport);
-              }
+              _collectTypeDependencies(
+                  type, dependencies, typeUse.deferredImport);
               break;
             case TypeUseKind.INSTANTIATION:
             case TypeUseKind.NATIVE_INSTANTIATION:
@@ -1588,3 +1563,72 @@ class DependencyInfo {
     }
   }
 }
+
+class TypeDependencyVisitor implements DartTypeVisitor<void, Null> {
+  final Dependencies _dependencies;
+  final ImportEntity _import;
+  final CommonElements _commonElements;
+
+  TypeDependencyVisitor(this._dependencies, this._import, this._commonElements);
+
+  @override
+  void visit(DartType type, [_]) {
+    type.accept(this, null);
+  }
+
+  void visitList(List<DartType> types) {
+    types.forEach(visit);
+  }
+
+  @override
+  void visitFutureOrType(FutureOrType type, Null argument) {
+    _dependencies.addClass(_commonElements.futureClass);
+    visit(type.typeArgument);
+  }
+
+  @override
+  void visitDynamicType(DynamicType type, Null argument) {
+    // Nothing to add.
+  }
+
+  @override
+  void visitTypedefType(TypedefType type, Null argument) {
+    visitList(type.typeArguments);
+    visit(type.unaliased);
+  }
+
+  @override
+  void visitInterfaceType(InterfaceType type, Null argument) {
+    visitList(type.typeArguments);
+    // TODO(sigmund): when we are able to split classes from types in our
+    // runtime-type representation, this should track type.element as a type
+    // dependency instead.
+    _dependencies.addClass(type.element, _import);
+  }
+
+  @override
+  void visitFunctionType(FunctionType type, Null argument) {
+    for (FunctionTypeVariable typeVariable in type.typeVariables) {
+      visit(typeVariable.bound);
+    }
+    visitList(type.parameterTypes);
+    visitList(type.optionalParameterTypes);
+    visitList(type.namedParameterTypes);
+    visit(type.returnType);
+  }
+
+  @override
+  void visitFunctionTypeVariable(FunctionTypeVariable type, Null argument) {
+    // Nothing to add. Handled in [visitFunctionType].
+  }
+
+  @override
+  void visitTypeVariableType(TypeVariableType type, Null argument) {
+    // TODO(johnniwinther): Do we need to collect the bound?
+  }
+
+  @override
+  void visitVoidType(VoidType type, Null argument) {
+    // Nothing to add.
+  }
+}

pkg/compiler/lib/src/kernel/deferred_load.dart

@@ -211,6 +211,7 @@ class ConstantCollector extends ir.RecursiveVisitor {
     // TODO(johnniwinther): The CFE should mark constant instantiations as
     // constant.
     add(node, required: false);
+    super.visitInstantiation(node);
   }
 
   @override

tests/compiler/dart2js/deferred_loading/data/basic_deferred.dart

@@ -1,9 +1,14 @@
 // 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 '../libs/basic_deferred_lib.dart' deferred as lib;
 
-/*element: main:OutputUnit(main, {})*/
+/*strong.element: main:OutputUnit(main, {})*/
+/*strongConst.element: main:
+ OutputUnit(main, {}),
+ constants=[FunctionConstant(funky)=OutputUnit(1, {lib})]
+*/
 main() => lib.loadLibrary().then(/*OutputUnit(main, {})*/ (_) {
       (lib.funky)();
     });

tests/compiler/dart2js/deferred_loading/data/deferred_constant1.dart

@@ -5,16 +5,25 @@
 import '../libs/deferred_constant1_lib1.dart';
 import '../libs/deferred_constant1_lib2.dart' deferred as lib2;
 
-/*element: main:OutputUnit(main, {})*/
+/*strong.element: main:OutputUnit(main, {})*/
+/*strongConst.element: main:
+ OutputUnit(main, {}),
+ constants=[
+  ConstructedConstant(C(value=ConstructedConstant(C(value=IntConstant(7)))))=OutputUnit(1, {lib2}),
+  ConstructedConstant(C(value=IntConstant(1)))=OutputUnit(main, {}),
+  ConstructedConstant(C(value=IntConstant(2)))=OutputUnit(1, {lib2}),
+  ConstructedConstant(C(value=IntConstant(4)))=OutputUnit(main, {}),
+  ConstructedConstant(C(value=IntConstant(5)))=OutputUnit(main, {})]
+*/
 main() async {
   C1.value;
   print(const C(4));
   /*OutputUnit(main, {})*/ () => print(const C(5));
   await lib2.loadLibrary();
-  lib2.C2.value;
-  lib2.C3.value;
-  lib2.C4.value;
-  lib2.C5.value;
-  lib2.C6;
-  lib2.C7.value;
+  print(lib2.C2.value);
+  print(lib2.C3.value);
+  print(lib2.C4.value);
+  print(lib2.C5.value);
+  print(lib2.C6);
+  print(lib2.C7.value);
 }

tests/compiler/dart2js/deferred_loading/data/deferred_constant2.dart

@@ -6,7 +6,12 @@ import 'package:expect/expect.dart';
 
 import '../libs/deferred_constant2_lib.dart' deferred as lib;
 
-/*element: main:OutputUnit(main, {})*/
+/*strong.element: main:OutputUnit(main, {})*/
+/*strongConst.element: main:
+ OutputUnit(main, {}),
+ constants=[
+  ConstructedConstant(Constant(value=IntConstant(499)))=OutputUnit(1, {lib})]
+*/
 main() {
   lib.loadLibrary().then(/*OutputUnit(main, {})*/ (_) {
     Expect.equals(499, lib.C1.value);

tests/compiler/dart2js/deferred_loading/data/deferred_function.dart

@@ -7,7 +7,11 @@
 
 import '../libs/deferred_function_lib.dart' deferred as lib;
 
-/*element: readFoo:OutputUnit(main, {})*/
+/*strong.element: readFoo:OutputUnit(main, {})*/
+/*strongConst.element: readFoo:
+ OutputUnit(main, {}),
+ constants=[FunctionConstant(foo)=OutputUnit(1, {lib})]
+*/
 readFoo() {
   return lib.foo;
 }

tests/compiler/dart2js/deferred_loading/data/deferred_typed_map.dart

@@ -4,7 +4,12 @@
 
 import '../libs/deferred_typed_map_lib1.dart' deferred as lib;
 
-/*element: main:OutputUnit(main, {})*/
+/*strong.element: main:OutputUnit(main, {})*/
+/*strongConst.element: main:
+ OutputUnit(main, {}),
+ constants=[
+  MapConstant(<int, dynamic Function({M b})>{IntConstant(1): FunctionConstant(f1), IntConstant(2): FunctionConstant(f2)})=OutputUnit(1, {lib})]
+*/
 main() async {
   await lib.loadLibrary();
   print(lib.table[1]);

tests/compiler/dart2js/deferred_loading/data/deferred_typedef.dart

@@ -4,7 +4,13 @@
 
 import '../libs/deferred_typedef_lib1.dart' deferred as lib1;
 
-/*element: main:OutputUnit(main, {})*/
+/*strong.element: main:OutputUnit(main, {})*/
+/*strongConst.element: main:
+ OutputUnit(main, {}),
+ constants=[
+  ConstructedConstant(C(a=TypeConstant(void Function()),b=FunctionConstant(topLevelMethod)))=OutputUnit(1, {lib1}),
+  TypeConstant(void Function())=OutputUnit(1, {lib1})]
+*/
 main() async {
   await lib1.loadLibrary();
   print(lib1.cA);

tests/compiler/dart2js/deferred_loading/data/future_or.dart

@@ -0,0 +1,18 @@
+// Copyright (c) 2019, 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 'dart:async';
+import '../libs/future_or_lib1.dart' deferred as lib1;
+import '../libs/future_or_lib2.dart' as lib2;
+
+/*strong.element: main:OutputUnit(main, {})*/
+/*strongConst.element: main:
+ OutputUnit(main, {}),
+ constants=[ConstructedConstant(A())=OutputUnit(1, {lib1})]
+*/
+main() async {
+  await lib1.loadLibrary();
+  lib1.field is FutureOr<lib2.A>;
+  lib1.field.method();
+}

tests/compiler/dart2js/deferred_loading/data/type_arguments.dart

@@ -0,0 +1,24 @@
+// Copyright (c) 2019, 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 '../libs/type_arguments_lib1.dart' deferred as lib1;
+import '../libs/type_arguments_lib2.dart' as lib2;
+import '../libs/type_arguments_lib3.dart' deferred as lib3;
+
+/*strong.element: main:OutputUnit(main, {})*/
+/*strongConst.element: main:
+ OutputUnit(main, {}),
+ constants=[
+  ConstructedConstant(A<B>())=OutputUnit(1, {lib1}),
+  ConstructedConstant(A<F>())=OutputUnit(1, {lib1}),
+  ConstructedConstant(C<D>())=OutputUnit(main, {}),
+  ConstructedConstant(E<F>())=OutputUnit(3, {lib3})]
+*/
+main() async {
+  await lib1.loadLibrary();
+  lib1.field1;
+  lib1.field2;
+  lib2.field;
+  lib3.field;
+}

tests/compiler/dart2js/deferred_loading/deferred_loading_test.dart

@@ -36,7 +36,7 @@ main(List<String> args) {
         options: compilerOptions,
         args: args, setUpFunction: () {
       importPrefixes.clear();
-    });
+    }, testOmit: false, testCFEConstants: true);
   });
 }
 
@@ -128,6 +128,8 @@ class OutputUnitIrComputer extends IrDataExtractor<String> {
   final OutputUnitData _data;
   final ClosureData _closureDataLookup;
 
+  Set<String> _constants = {};
+
   OutputUnitIrComputer(
       DiagnosticReporter reporter,
       Map<Id, ActualData<String>> actualMap,
@@ -137,8 +139,14 @@ class OutputUnitIrComputer extends IrDataExtractor<String> {
       this._closureDataLookup)
       : super(reporter, actualMap);
 
-  String getMemberValue(MemberEntity member) {
-    return outputUnitString(_data.outputUnitForMember(member));
+  String getMemberValue(MemberEntity member, Set<String> constants) {
+    StringBuffer sb = new StringBuffer();
+    sb.write(outputUnitString(_data.outputUnitForMember(member)));
+    if (constants.isNotEmpty) {
+      List<String> text = constants.toList()..sort();
+      sb.write(',constants=[${text.join(',')}]');
+    }
+    return sb.toString();
   }
 
   @override
@@ -166,14 +174,26 @@ class OutputUnitIrComputer extends IrDataExtractor<String> {
       }
     }
 
-    return getMemberValue(_elementMap.getMember(node));
+    String value = getMemberValue(_elementMap.getMember(node), _constants);
+    _constants = {};
+    return value;
+  }
+
+  @override
+  visitConstantExpression(ir.ConstantExpression node) {
+    ConstantValue constant = _elementMap.getConstantValue(node);
+    if (!constant.isPrimitive) {
+      _constants.add('${constant.toStructuredText()}='
+          '${outputUnitString(_data.outputUnitForConstant(constant))}');
+    }
+    return super.visitConstantExpression(node);
   }
 
   @override
   String computeNodeValue(Id id, ir.TreeNode node) {
     if (node is ir.FunctionExpression || node is ir.FunctionDeclaration) {
       ClosureRepresentationInfo info = _closureDataLookup.getClosureInfo(node);
-      return getMemberValue(info.callMethod);
+      return getMemberValue(info.callMethod, const {});
     }
     return null;
   }