[CFE] Fix dependency pruning in regards to constant evaluation

Change-Id: I82226c433e28ada640b4fa102f9def506a2e8758
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/212466
Commit-Queue: Jens Johansen <jensj@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>

pkg/front_end/lib/src/fasta/incremental_compiler.dart

@@ -618,9 +618,15 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
     if (trackNeededDillLibraries) {
       // Which dill builders were built?
       neededDillLibraries = new Set<Library>();
+
+      // Propagate data from constant evaluator: Libraries used in the constant
+      // evaluator - that comes from dill - are marked.
+      Set<Library> librariesUsedByConstantEvaluator = userCode!.librariesUsed;
+
       for (LibraryBuilder builder in dillLoadedData!.loader.builders.values) {
         if (builder is DillLibraryBuilder) {
-          if (builder.isBuiltAndMarked) {
+          if (builder.isBuiltAndMarked ||
+              librariesUsedByConstantEvaluator.contains(builder.library)) {
             neededDillLibraries!.add(builder.library);
           }
         }
@@ -2396,6 +2402,7 @@ class IncrementalKernelTarget extends KernelTarget
     implements ChangedStructureNotifier {
   Set<Class>? classHierarchyChanges;
   Set<Class>? classMemberChanges;
+  Set<Library> librariesUsed = {};
 
   IncrementalKernelTarget(FileSystem fileSystem, bool includeComments,
       DillTarget dillTarget, UriTranslator uriTranslator)
@@ -2415,4 +2422,9 @@ class IncrementalKernelTarget extends KernelTarget
     classHierarchyChanges ??= <Class>{};
     classHierarchyChanges!.add(cls);
   }
+
+  @override
+  void markLibrariesUsed(Set<Library> visitedLibraries) {
+    librariesUsed.addAll(visitedLibraries);
+  }
 }

pkg/front_end/lib/src/fasta/kernel/constant_evaluator.dart

@@ -82,7 +82,7 @@ Component transformComponent(
   return component;
 }
 
-ConstantCoverage transformLibraries(
+ConstantEvaluationData transformLibraries(
     List<Library> libraries,
     ConstantsBackend backend,
     Map<String, String>? environmentDefines,
@@ -118,7 +118,10 @@ ConstantCoverage transformLibraries(
   for (final Library library in libraries) {
     constantsTransformer.convertLibrary(library);
   }
-  return constantsTransformer.constantEvaluator.getConstantCoverage();
+
+  return new ConstantEvaluationData(
+      constantsTransformer.constantEvaluator.getConstantCoverage(),
+      constantsTransformer.constantEvaluator.visitedLibraries);
 }
 
 void transformProcedure(
@@ -911,6 +914,8 @@ class ConstantEvaluator implements ExpressionVisitor<Constant> {
   final BoolConstant trueConstant = new BoolConstant(true);
   final BoolConstant falseConstant = new BoolConstant(false);
 
+  final Set<Library> visitedLibraries = {};
+
   InstanceBuilder? instanceBuilder;
   EvaluationEnvironment env;
   Set<Expression> replacementNodes = new Set<Expression>.identity();
@@ -2835,6 +2840,7 @@ class ConstantEvaluator implements ExpressionVisitor<Constant> {
   Constant visitStaticGet(StaticGet node) {
     return withNewEnvironment(() {
       final Member target = node.target;
+      visitedLibraries.add(target.enclosingLibrary);
       if (target is Field) {
         if (target.isConst) {
           return _evaluateExpressionInContext(target, target.initializer!);
@@ -4028,6 +4034,13 @@ class ConstantCoverage {
   ConstantCoverage(this.constructorCoverage);
 }
 
+class ConstantEvaluationData {
+  final ConstantCoverage coverage;
+  final Set<Library> visitedLibraries;
+
+  ConstantEvaluationData(this.coverage, this.visitedLibraries);
+}
+
 /// Holds the necessary information for a constant object, namely
 ///   * the [klass] being instantiated
 ///   * the [typeArguments] used for the instantiation

pkg/front_end/lib/src/fasta/kernel/kernel_target.dart

@@ -80,7 +80,8 @@ import 'constant_evaluator.dart' as constants
         EvaluationMode,
         transformLibraries,
         transformProcedure,
-        ConstantCoverage;
+        ConstantCoverage,
+        ConstantEvaluationData;
 import 'kernel_constants.dart' show KernelConstantErrorReporter;
 import 'kernel_helper.dart';
 import 'verifier.dart' show verifyComponent, verifyGetStaticType;
@@ -1230,23 +1231,27 @@ class KernelTarget extends TargetImplementation {
         new TypeEnvironment(loader.coreTypes, loader.hierarchy);
     constants.EvaluationMode evaluationMode = _getConstantEvaluationMode();
 
-    constants.ConstantCoverage coverage = constants.transformLibraries(
-        loader.libraries,
-        backendTarget.constantsBackend(loader.coreTypes),
-        environmentDefines,
-        environment,
-        new KernelConstantErrorReporter(loader),
-        evaluationMode,
-        evaluateAnnotations: true,
-        enableTripleShift:
-            isExperimentEnabledGlobally(ExperimentalFlag.tripleShift),
-        enableConstFunctions:
-            isExperimentEnabledGlobally(ExperimentalFlag.constFunctions),
-        enableConstructorTearOff:
-            isExperimentEnabledGlobally(ExperimentalFlag.constructorTearoffs),
-        errorOnUnevaluatedConstant: errorOnUnevaluatedConstant);
+    constants.ConstantEvaluationData constantEvaluationData =
+        constants.transformLibraries(
+            loader.libraries,
+            backendTarget.constantsBackend(loader.coreTypes),
+            environmentDefines,
+            environment,
+            new KernelConstantErrorReporter(loader),
+            evaluationMode,
+            evaluateAnnotations: true,
+            enableTripleShift:
+                isExperimentEnabledGlobally(ExperimentalFlag.tripleShift),
+            enableConstFunctions:
+                isExperimentEnabledGlobally(ExperimentalFlag.constFunctions),
+            enableConstructorTearOff: isExperimentEnabledGlobally(
+                ExperimentalFlag.constructorTearoffs),
+            errorOnUnevaluatedConstant: errorOnUnevaluatedConstant);
     ticker.logMs("Evaluated constants");
 
+    markLibrariesUsed(constantEvaluationData.visitedLibraries);
+
+    constants.ConstantCoverage coverage = constantEvaluationData.coverage;
     coverage.constructorCoverage.forEach((Uri fileUri, Set<Reference> value) {
       Source? source = uriToSource[fileUri];
       // ignore: unnecessary_null_comparison
@@ -1387,6 +1392,10 @@ class KernelTarget extends TargetImplementation {
   void releaseAncillaryResources() {
     component = null;
   }
+
+  void markLibrariesUsed(Set<Library> visitedLibraries) {
+    // Default implementation does nothing.
+  }
 }
 
 /// Looks for a constructor call that matches `super()` from a constructor in

pkg/front_end/test/incremental_suite.dart

@@ -299,8 +299,12 @@ Future<Null> basicTest(YamlMap sourceFiles, String entryPoint,
   checkIsEqual(normalDillData, initializedDillData);
 }
 
-Future<Map<String, List<int>>> createModules(Map module,
-    final List<int> sdkSummaryData, Target target, String sdkSummary) async {
+Future<Map<String, List<int>>> createModules(
+    Map module,
+    final List<int> sdkSummaryData,
+    Target target,
+    Target originalTarget,
+    String sdkSummary) async {
   final Uri base = Uri.parse("org-dartlang-test:///");
   final Uri sdkSummaryUri = base.resolve(sdkSummary);
 
@@ -332,6 +336,10 @@ Future<Map<String, List<int>>> createModules(Map module,
         moduleSources.add(uri);
       }
     }
+    bool outlineOnly = false;
+    if (originalTarget is DevCompilerTarget) {
+      outlineOnly = true;
+    }
     CompilerOptions options =
         getOptions(target: target, sdkSummary: sdkSummary);
     options.fileSystem = fs;
@@ -345,8 +353,8 @@ Future<Map<String, List<int>>> createModules(Map module,
     if (packagesUri != null) {
       options.packagesFileUri = packagesUri;
     }
-    TestIncrementalCompiler compiler =
-        new TestIncrementalCompiler(options, moduleSources.first, null);
+    TestIncrementalCompiler compiler = new TestIncrementalCompiler(
+        options, moduleSources.first, /* initializeFrom = */ null, outlineOnly);
     Component c = await compiler.computeDelta(entryPoints: moduleSources);
     c.computeCanonicalNames();
     List<Library> wantedLibs = <Library>[];
@@ -416,6 +424,7 @@ class NewWorldTest {
         throw "Unknown target name '$targetName'";
       }
     }
+    Target originalTarget = target;
     target = new TestTargetWrapper(target, targetFlags);
 
     String sdkSummary = computePlatformDillName(
@@ -442,8 +451,8 @@ class NewWorldTest {
     Map<String, Component>? moduleComponents;
 
     if (modules != null) {
-      moduleData =
-          await createModules(modules, sdkSummaryData, target, sdkSummary);
+      moduleData = await createModules(
+          modules, sdkSummaryData, target, originalTarget, sdkSummary);
       sdk = newestWholeComponent = new Component();
       new BinaryBuilder(sdkSummaryData,
               filename: null, disableLazyReading: false)

pkg/front_end/testcases/incremental/changing_modules_14.yaml

@@ -0,0 +1,59 @@
+# Copyright (c) 2021, 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.md file.
+
+# Compile an application with modules compiled as outlines with constants.
+# When compiling the "real" library (as non-outline) constant evaluation
+# goes into more modules than in just the outline version.
+
+type: newworld
+# Set to DDC for compiling modules as outline.
+target: DDC
+modules:
+  moduleC:
+    moduleC/lib.dart: |
+      const constC = ['value_c'];
+    moduleC/.packages: |
+      moduleC:.
+  moduleB:
+    moduleB/lib.dart: |
+      import "package:moduleC/lib.dart";
+      const constB = ['value_b', constC];
+    moduleB/.packages: |
+      moduleB:.
+      moduleC:../moduleC
+worlds:
+  - entry: main.dart
+    fromComponent: true
+    sources:
+      main.dart: |
+        import 'package:moduleB/lib.dart';
+        import 'lib.dart';
+        const constA = ['value_a', constB];
+        const constLib = constFromLib;
+      lib.dart: |
+        const constFromLib = 42;
+      .packages: |
+        moduleB:moduleB
+        moduleC:moduleC
+    modules:
+      - moduleB
+      - moduleC
+    expectedLibraryCount: 4
+    neededDillLibraries:
+      # Because the modules are complied as outlines they haven't had constants
+      # evaluated. As this is fully compiled it does evaluate constants and thus
+      # goes into both B and C.
+      # We do not mark "lib.dart" though as it is not loaded from dill.
+      - package:moduleB/lib.dart
+      - package:moduleC/lib.dart
+    expectedContent:
+      org-dartlang-test:///main.dart:
+        - Field constA
+        - Field constLib
+      org-dartlang-test:///lib.dart:
+        - Field constFromLib
+      package:moduleB/lib.dart:
+        - Field constB
+      package:moduleC/lib.dart:
+        - Field constC

pkg/front_end/testcases/incremental/changing_modules_14.yaml.world.1.expect

@@ -0,0 +1,32 @@
+main = <No Member>;
+library from "org-dartlang-test:///lib.dart" as lib {
+
+  static const field dart.core::int constFromLib = #C1;
+}
+library from "org-dartlang-test:///main.dart" as main {
+
+  import "package:moduleB/lib.dart";
+  import "org-dartlang-test:///lib.dart";
+
+  static const field dart.core::List<dart.core::Object> constA = #C7;
+  static const field dart.core::int constLib = #C1;
+}
+library from "package:moduleB/lib.dart" as lib2 {
+
+  import "package:moduleC/lib.dart";
+
+  static const field dart.core::List<dart.core::Object*>* constB = const <dart.core::Object*>["value_b", lib3::constC];
+}
+library from "package:moduleC/lib.dart" as lib3 {
+
+  static const field dart.core::List<dart.core::String*>* constC = const <dart.core::String*>["value_c"];
+}
+constants  {
+  #C1 = 42.0
+  #C2 = "value_a"
+  #C3 = "value_b"
+  #C4 = "value_c"
+  #C5 = <dart.core::String*>[#C4]
+  #C6 = <dart.core::Object*>[#C3, #C5]
+  #C7 = <dart.core::Object*>[#C2, #C6]
+}

pkg/front_end/testcases/incremental/changing_modules_15.yaml

@@ -0,0 +1,51 @@
+# Copyright (c) 2021, 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.md file.
+
+# Compile an application with modules compiled as outlines with constants.
+# When compiling the "real" library (as non-outline) constant evaluation
+# goes into more modules than in just the outline version.
+
+type: newworld
+# Set to DDC for compiling modules as outline.
+target: DDC
+modules:
+  moduleC:
+    moduleC/lib.dart: |
+      const constC = true;
+    moduleC/.packages: |
+      moduleC:.
+  moduleB:
+    moduleB/lib.dart: |
+      import "package:moduleC/lib.dart";
+      const constB = false || constC;
+    moduleB/.packages: |
+      moduleB:.
+      moduleC:../moduleC
+worlds:
+  - entry: main.dart
+    fromComponent: true
+    sources:
+      main.dart: |
+        import 'package:moduleB/lib.dart';
+        const constA = true && constB;
+      .packages: |
+        moduleB:moduleB
+        moduleC:moduleC
+    modules:
+      - moduleB
+      - moduleC
+    expectedLibraryCount: 3
+    neededDillLibraries:
+      # Because the modules are complied as outlines they haven't had constants
+      # evaluated. As this is fully compiled it does evaluate constants and thus
+      # goes into both B and C.
+      - package:moduleB/lib.dart
+      - package:moduleC/lib.dart
+    expectedContent:
+      org-dartlang-test:///main.dart:
+        - Field constA
+      package:moduleB/lib.dart:
+        - Field constB
+      package:moduleC/lib.dart:
+        - Field constC

pkg/front_end/testcases/incremental/changing_modules_15.yaml.world.1.expect

@@ -0,0 +1,20 @@
+main = <No Member>;
+library from "org-dartlang-test:///main.dart" as main {
+
+  import "package:moduleB/lib.dart";
+
+  static const field dart.core::bool constA = #C1;
+}
+library from "package:moduleB/lib.dart" as lib {
+
+  import "package:moduleC/lib.dart";
+
+  static const field dart.core::bool* constB = false || lib2::constC;
+}
+library from "package:moduleC/lib.dart" as lib2 {
+
+  static const field dart.core::bool* constC = true;
+}
+constants  {
+  #C1 = true
+}