[CFE] Replace 'first' with 'roots'

Before we could get into a situation where we didn't produce the entire
class hierarchy when we loaded from dill even though we set up the
entry points correctly.
This might not be perfect, but seems better.

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

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

@@ -369,7 +369,7 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
       while (true) {
         _benchmarker?.enterPhase(BenchmarkPhases.incremental_setupInLoop);
         currentKernelTarget = _setupNewKernelTarget(c, uriTranslator, hierarchy,
-            reusedLibraries, experimentalInvalidation, entryPoints.first);
+            reusedLibraries, experimentalInvalidation, entryPoints);
         Map<LibraryBuilder, List<LibraryBuilder>>? rebuildBodiesMap =
             _experimentalInvalidationCreateRebuildBodiesBuilders(
                 currentKernelTarget, experimentalInvalidation, uriTranslator);
@@ -989,7 +989,7 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
       ClassHierarchy? hierarchy,
       List<LibraryBuilder> reusedLibraries,
       ExperimentalInvalidation? experimentalInvalidation,
-      Uri firstEntryPoint) {
+      List<Uri> entryPoints) {
     IncrementalKernelTarget kernelTarget = createIncrementalKernelTarget(
         new HybridFileSystem(
             new MemoryFileSystem(
@@ -1044,8 +1044,10 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
 
     // The entry point(s) has to be set first for loader.firstUri to be setup
     // correctly.
-    kernelTarget.loader.firstUri =
-        kernelTarget.getEntryPointUri(firstEntryPoint, issueProblem: false);
+    kernelTarget.loader.roots.clear();
+    for (Uri entryPoint in entryPoints) {
+      kernelTarget.loader.roots.add(kernelTarget.getEntryPointUri(entryPoint));
+    }
     return kernelTarget;
   }
 

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

@@ -285,8 +285,7 @@ class KernelTarget extends TargetImplementation {
   List<Uri> setEntryPoints(List<Uri> entryPoints) {
     List<Uri> result = <Uri>[];
     for (Uri entryPoint in entryPoints) {
-      Uri translatedEntryPoint =
-          getEntryPointUri(entryPoint, issueProblem: true);
+      Uri translatedEntryPoint = getEntryPointUri(entryPoint);
       result.add(translatedEntryPoint);
       loader.readAsEntryPoint(translatedEntryPoint,
           fileUri: translatedEntryPoint != entryPoint ? entryPoint : null);
@@ -295,7 +294,7 @@ class KernelTarget extends TargetImplementation {
   }
 
   /// Return list of same size as input with possibly translated uris.
-  Uri getEntryPointUri(Uri entryPoint, {bool issueProblem = false}) {
+  Uri getEntryPointUri(Uri entryPoint) {
     String scheme = entryPoint.scheme;
     switch (scheme) {
       case "package":
@@ -347,7 +346,7 @@ class KernelTarget extends TargetImplementation {
     assert(!_hasComputedNeededPrecompilations,
         "Needed precompilations have already been computed.");
     _hasComputedNeededPrecompilations = true;
-    if (loader.first == null) return null;
+    if (loader.roots.isEmpty) return null;
     return withCrashReporting<NeededPrecompilations?>(() async {
       benchmarker?.enterPhase(BenchmarkPhases.outline_kernelBuildOutlines);
       await loader.buildOutlines();
@@ -402,7 +401,7 @@ class KernelTarget extends TargetImplementation {
   }
 
   Future<BuildResult> buildOutlines({CanonicalName? nameRoot}) async {
-    if (loader.first == null) return new BuildResult();
+    if (loader.roots.isEmpty) return new BuildResult();
     return withCrashReporting<BuildResult>(() async {
       if (!_hasComputedNeededPrecompilations) {
         NeededPrecompilations? neededPrecompilations =
@@ -587,7 +586,7 @@ class KernelTarget extends TargetImplementation {
   Future<BuildResult> buildComponent(
       {required MacroApplications? macroApplications,
       bool verify = false}) async {
-    if (loader.first == null) {
+    if (loader.roots.isEmpty) {
       return new BuildResult(macroApplications: macroApplications);
     }
     return withCrashReporting<BuildResult>(() async {
@@ -710,10 +709,11 @@ class KernelTarget extends TargetImplementation {
 
     Reference? mainReference;
 
-    if (loader.first != null) {
+    LibraryBuilder? firstRoot = loader.firstRoot;
+    if (firstRoot != null) {
       // TODO(sigmund): do only for full program
       Builder? declaration =
-          loader.first!.exportScope.lookup("main", -1, loader.first!.fileUri);
+          firstRoot.exportScope.lookup("main", -1, firstRoot.fileUri);
       if (declaration is AmbiguousBuilder) {
         AmbiguousBuilder problem = declaration;
         declaration = problem.getFirstDeclaration();

pkg/front_end/lib/src/fasta/source/source_library_builder.dart

@@ -1616,7 +1616,7 @@ class SourceLibraryBuilder extends LibraryBuilderImpl {
   /// Reports [message] on all libraries that access this library.
   void addProblemAtAccessors(Message message) {
     if (accessProblem == null) {
-      if (accessors.isEmpty && this == loader.first) {
+      if (accessors.isEmpty && loader.roots.contains(this.importUri)) {
         // This is the entry point library, and nobody access it directly. So
         // we need to report a problem.
         loader.addProblem(message, -1, 1, null);

pkg/front_end/lib/src/fasta/source/source_loader.dart

@@ -60,7 +60,6 @@ import '../builder/type_alias_builder.dart';
 import '../builder/type_builder.dart';
 import '../builder/type_declaration_builder.dart';
 import '../builder_graph.dart';
-import '../crash.dart' show firstSourceUri;
 import '../denylisted_classes.dart'
     show denylistedCoreClasses, denylistedTypedDataClasses;
 import '../dill/dill_library_builder.dart';
@@ -212,16 +211,16 @@ class SourceLoader extends Loader {
   LibraryBuilder? _coreLibrary;
   LibraryBuilder? typedDataLibrary;
 
-  /// The first library that we've been asked to compile. When compiling a
-  /// program (aka script), this is the library that should have a main method.
-  Uri? _firstUri;
+  final Set<Uri> roots = {};
 
-  LibraryBuilder? get first => _builders[_firstUri];
-
-  Uri? get firstUri => _firstUri;
-
-  void set firstUri(Uri? value) {
-    _firstUri = value;
+  // TODO(johnniwinther): Replace with a `singleRoot`.
+  // See also https://dart-review.googlesource.com/c/sdk/+/273381.
+  LibraryBuilder? get firstRoot {
+    for (Uri uri in roots) {
+      LibraryBuilder? builder = _builders[uri];
+      if (builder != null) return builder;
+    }
+    return null;
   }
 
   int byteCount = 0;
@@ -375,7 +374,8 @@ class SourceLoader extends Loader {
       SourceLibraryBuilder? origin,
       Library? referencesFrom,
       bool? referenceIsPartOwner,
-      bool isAugmentation) {
+      bool isAugmentation,
+      bool addAsRoot) {
     if (fileUri != null &&
         (fileUri.isScheme("dart") ||
             fileUri.isScheme("package") ||
@@ -462,11 +462,9 @@ class SourceLoader extends Loader {
           packageLanguageVersionProblem, 0, noLength, libraryBuilder.fileUri);
     }
 
-    // Add any additional logic after this block. Setting the
-    // firstSourceUri and first library should be done as early as
-    // possible.
-    firstSourceUri ??= uri;
-    firstUri ??= libraryBuilder.importUri;
+    if (addAsRoot) {
+      roots.add(uri);
+    }
 
     _checkForDartCore(uri, libraryBuilder);
 
@@ -575,7 +573,8 @@ class SourceLoader extends Loader {
         origin: origin,
         referencesFrom: referencesFrom,
         referenceIsPartOwner: referenceIsPartOwner,
-        isAugmentation: isAugmentation);
+        isAugmentation: isAugmentation,
+        addAsRoot: false);
     libraryBuilder.recordAccess(
         accessor, charOffset, noLength, accessor.fileUri);
     if (!_hasLibraryAccess(imported: uri, importer: accessor.importUri) &&
@@ -594,22 +593,22 @@ class SourceLoader extends Loader {
   /// that access to platform private libraries cannot be granted.
   LibraryBuilder readAsEntryPoint(Uri uri,
       {Uri? fileUri, Library? referencesFrom}) {
-    LibraryBuilder libraryBuilder =
-        _read(uri, fileUri: fileUri, referencesFrom: referencesFrom);
+    LibraryBuilder libraryBuilder = _read(uri,
+        fileUri: fileUri, referencesFrom: referencesFrom, addAsRoot: true);
     // TODO(johnniwinther): Avoid using the first library, if present, as the
     // accessor of [libraryBuilder]. Currently the incremental compiler doesn't
     // handle errors reported without an accessor, since the messages are not
     // associated with a library. This currently has the side effect that
     // the first library is the accessor of itself.
-    LibraryBuilder? firstLibrary = first;
+    LibraryBuilder? firstLibrary = firstRoot;
     if (firstLibrary != null) {
       libraryBuilder.recordAccess(
           firstLibrary, -1, noLength, firstLibrary.fileUri);
     }
     if (!_hasLibraryAccess(imported: uri, importer: firstLibrary?.importUri)) {
       if (firstLibrary != null) {
-        firstLibrary.addProblem(
-            messagePlatformPrivateLibraryAccess, -1, noLength, firstUri);
+        firstLibrary.addProblem(messagePlatformPrivateLibraryAccess, -1,
+            noLength, firstLibrary.importUri);
       } else {
         addProblem(messagePlatformPrivateLibraryAccess, -1, noLength, null);
       }
@@ -634,7 +633,8 @@ class SourceLoader extends Loader {
       LibraryBuilder? origin,
       Library? referencesFrom,
       bool? referenceIsPartOwner,
-      bool isAugmentation = false}) {
+      bool isAugmentation = false,
+      required bool addAsRoot}) {
     LibraryBuilder? libraryBuilder = _builders[uri];
     if (libraryBuilder == null) {
       if (target.dillTarget.isLoaded) {
@@ -647,7 +647,8 @@ class SourceLoader extends Loader {
             origin as SourceLibraryBuilder?,
             referencesFrom,
             referenceIsPartOwner,
-            isAugmentation);
+            isAugmentation,
+            addAsRoot);
       }
       _builders[uri] = libraryBuilder;
     }
@@ -1064,7 +1065,7 @@ severity: $severity
       _nnbdMismatchLibraries = null;
     }
     if (_unavailableDartLibraries.isNotEmpty) {
-      LibraryBuilder? rootLibrary = first;
+      LibraryBuilder? rootLibrary = firstRoot;
       LoadedLibraries? loadedLibraries;
       for (SourceLibraryBuilder libraryBuilder in _unavailableDartLibraries) {
         List<LocatedMessage>? context;
@@ -1373,8 +1374,9 @@ severity: $severity
     for (Uri uri in parts) {
       if (usedParts.contains(uri)) {
         LibraryBuilder? part = _builders.remove(uri);
-        if (_firstUri == uri) {
-          firstUri = part!.partOfLibrary!.importUri;
+        if (roots.contains(uri)) {
+          roots.remove(uri);
+          roots.add(part!.partOfLibrary!.importUri);
         }
       } else {
         SourceLibraryBuilder part =
@@ -2194,7 +2196,7 @@ severity: $severity
       if (!libraryBuilder.isPatch &&
           (libraryBuilder.loader == this ||
               libraryBuilder.importUri.isScheme("dart") ||
-              libraryBuilder == this.first)) {
+              roots.contains(libraryBuilder.importUri))) {
         if (libraries.add(libraryBuilder.library)) {
           workList.add(libraryBuilder.library);
         }
@@ -2703,7 +2705,6 @@ severity: $severity
     _typeInferenceEngine = null;
     _builders.clear();
     libraries.clear();
-    firstUri = null;
     sourceBytes.clear();
     target.releaseAncillaryResources();
     _coreTypes = null;

pkg/front_end/test/crashing_test_case_minimizer_impl.dart

@@ -814,7 +814,7 @@ worlds:
 
   Uri _getImportUri(Uri uri) {
     return _latestCrashingIncrementalCompiler!.kernelTargetForTesting!
-        .getEntryPointUri(uri, issueProblem: false);
+        .getEntryPointUri(uri);
   }
 
   Uint8List _sublist(Uint8List data, int start, int end) {

pkg/front_end/test/incremental_suite.dart

@@ -344,6 +344,9 @@ class ExpressionCompilationProperties {
 
   static const Property<String> expression =
       Property.required('expression', StringValue());
+
+  static const Property<String?> className =
+      Property.optional('className', StringValue());
 }
 
 final ExpectationSet staticExpectationSet =
@@ -781,12 +784,14 @@ class ExpressionCompilation {
   final bool warnings;
   final String uri;
   final String expression;
+  final String? className;
 
   ExpressionCompilation(
       {required this.errors,
       required this.warnings,
       required this.uri,
-      required this.expression});
+      required this.expression,
+      required this.className});
 
   static ExpressionCompilation create(Map yaml) {
     Set<String> keys = new Set<String>.from(yaml.keys);
@@ -796,12 +801,18 @@ class ExpressionCompilation {
     String uri = ExpressionCompilationProperties.uri.read(yaml, keys);
     String expression =
         ExpressionCompilationProperties.expression.read(yaml, keys);
+    String? className =
+        ExpressionCompilationProperties.className.read(yaml, keys);
 
     if (keys.isNotEmpty) {
       throw "Unknown key(s) for ExpressionCompilation: $keys";
     }
     return new ExpressionCompilation(
-        errors: errors, warnings: warnings, uri: uri, expression: expression);
+        errors: errors,
+        warnings: warnings,
+        uri: uri,
+        expression: expression,
+        className: className);
   }
 }
 
@@ -1668,7 +1679,13 @@ class NewWorldTest {
           clearPrevErrorsEtc();
           Uri uri = base.resolve(compilation.uri);
           Procedure procedure = (await compiler.compileExpression(
-              compilation.expression, {}, [], "debugExpr", uri))!;
+            compilation.expression,
+            {},
+            [],
+            "debugExpr",
+            uri,
+            className: compilation.className,
+          ))!;
           if (gotError && !compilation.errors) {
             return new Result<TestData>(data, UnexpectedErrors,
                 "Got error(s) on expression compilation: ${formattedErrors}.");

pkg/front_end/test/macros/application/macro_application_test.dart

@@ -140,8 +140,8 @@ class MacroDataComputer extends DataComputer<String> {
         .dataForTesting!
         .macroApplicationData;
     StringBuffer sb = new StringBuffer();
-    if (library.importUri ==
-        testResultData.compilerResult.kernelTargetForTesting!.loader.firstUri) {
+    if (testResultData.compilerResult.kernelTargetForTesting!.loader.roots
+        .contains(library.importUri)) {
       if (macroApplicationData.typesApplicationOrder.isNotEmpty) {
         sb.write('\nTypes Order:');
         for (ApplicationDataForTesting application

pkg/front_end/testcases/incremental/multiple_entry_points.yaml

@@ -0,0 +1,65 @@
+# Copyright (c) 2022, 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.
+
+# Use multiple entry points, try for force the class hierarchy not to contain
+# all the entry point libraries, then try to force using it via expression
+# compilation. This used to crash.
+
+type: newworld
+worlds:
+  - entry:
+    - a.dart
+    - b.dart
+    sources:
+      a.dart: |
+        class A {
+          void foo() {}
+        }
+        class B extends A {
+          void foo() {
+            super.foo();
+          }
+        }
+      b.dart: |
+        class C {
+          void foo() {}
+        }
+        class D extends C {
+          void foo() {
+            super.foo();
+          }
+        }
+    expectedLibraryCount: 2
+
+  - entry:
+    - a.dart
+    - b.dart
+    expectInitializeFromDill: true
+    invalidate:
+      - a.dart
+    sources:
+      a.dart: |
+        class A {
+          void bar() {}
+        }
+        class B extends A {
+          void bar() {
+            super.bar();
+          }
+        }
+      b.dart: |
+        class C {
+          void foo() {}
+        }
+        class D extends C {
+          void foo() {
+            super.foo();
+          }
+        }
+    advancedInvalidation: outlineChange
+    expectedLibraryCount: 2
+    expressionCompilation:
+      uri: b.dart
+      expression: super.foo()
+      className: D

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

@@ -0,0 +1,35 @@
+main = <No Member>;
+library from "org-dartlang-test:///a.dart" as a {
+
+  class A extends dart.core::Object {
+    synthetic constructor •() → a::A
+      : super dart.core::Object::•()
+      ;
+    method foo() → void {}
+  }
+  class B extends a::A {
+    synthetic constructor •() → a::B
+      : super a::A::•()
+      ;
+    method foo() → void {
+      super.{a::A::foo}();
+    }
+  }
+}
+library from "org-dartlang-test:///b.dart" as b {
+
+  class C extends dart.core::Object {
+    synthetic constructor •() → b::C
+      : super dart.core::Object::•()
+      ;
+    method foo() → void {}
+  }
+  class D extends b::C {
+    synthetic constructor •() → b::D
+      : super b::C::•()
+      ;
+    method foo() → void {
+      super.{b::C::foo}();
+    }
+  }
+}

pkg/front_end/testcases/incremental/multiple_entry_points.yaml.world.2.expect

@@ -0,0 +1,35 @@
+main = <No Member>;
+library from "org-dartlang-test:///a.dart" as a {
+
+  class A extends dart.core::Object {
+    synthetic constructor •() → a::A
+      : super dart.core::Object::•()
+      ;
+    method bar() → void {}
+  }
+  class B extends a::A {
+    synthetic constructor •() → a::B
+      : super a::A::•()
+      ;
+    method bar() → void {
+      super.{a::A::bar}();
+    }
+  }
+}
+library from "org-dartlang-test:///b.dart" as b {
+
+  class C extends dart.core::Object {
+    synthetic constructor •() → b::C
+      : super dart.core::Object::•()
+      ;
+    method foo() → void {}
+  }
+  class D extends b::C {
+    synthetic constructor •() → b::D
+      : super b::C::•()
+      ;
+    method foo() → void {
+      super.{b::C::foo}();
+    }
+  }
+}

pkg/front_end/testcases/incremental/multiple_entry_points.yaml.world.2.expression.1.expect

@@ -0,0 +1,2 @@
+method /* from org-dartlang-debug:synthetic_debug_expression */ debugExpr() → dynamic
+  return super.{#lib1::C::foo}();