initializeFromComponent has to include platform

For initializeFromComponent to work correctly the platform has to be
provided (as the other libraries provided are linked to some platform,
and loading another one is thus no good).
This CL changes it so we don't load another one, and checks that the
component we're trying to initialize from actually contains dart:core.

Change-Id: I88d30436c101c589b0555ff70ae21297ed665d7b
Reviewed-on: https://dart-review.googlesource.com/c/93435
Commit-Queue: Jens Johansen <[email protected]>
Reviewed-by: Alexander Aprelev <[email protected]>

Author: jensjoha

pkg/front_end/lib/src/api_prototype/incremental_kernel_generator.dart

@@ -26,6 +26,10 @@ abstract class IncrementalKernelGenerator {
         initializeFromDillUri);
   }
 
+  /// Initialize the incremental compiler from a component.
+  ///
+  /// Notice that the component has to include the platform, and that no other
+  /// platform will be loaded.
   factory IncrementalKernelGenerator.fromComponent(
       CompilerOptions options, Uri entryPoint, Component component) {
     return new IncrementalCompiler.fromComponent(

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

@@ -27,7 +27,6 @@ import 'package:kernel/kernel.dart'
         Procedure,
         ProcedureKind,
         ReturnStatement,
-        Source,
         TreeNode,
         TypeParameter;
 
@@ -126,56 +125,62 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
       ticker.logMs("Read packages file");
 
       if (dillLoadedData == null) {
-        List<int> summaryBytes = await c.options.loadSdkSummaryBytes();
-        int bytesLength = prepareSummary(summaryBytes, uriTranslator, c, data);
-        if (initializeFromDillUri != null) {
-          try {
-            bytesLength += await initializeFromDill(uriTranslator, c, data);
-          } catch (e, st) {
-            // We might have loaded x out of y libraries into the component.
-            // To avoid any unforeseen problems start over.
-            bytesLength = prepareSummary(summaryBytes, uriTranslator, c, data);
-
-            if (e is InvalidKernelVersionError || e is PackageChangedError) {
-              // Don't report any warning.
-            } else {
-              Uri gzInitializedFrom;
-              if (c.options.writeFileOnCrashReport) {
-                gzInitializedFrom = saveAsGzip(
-                    data.initializationBytes, "initialize_from.dill");
-                recordTemporaryFileForTesting(gzInitializedFrom);
-              }
-              if (e is CanonicalNameError) {
-                Message message = gzInitializedFrom != null
-                    ? templateInitializeFromDillNotSelfContained.withArguments(
-                        initializeFromDillUri.toString(), gzInitializedFrom)
-                    : templateInitializeFromDillNotSelfContainedNoDump
-                        .withArguments(initializeFromDillUri.toString());
-                dillLoadedData.loader
-                    .addProblem(message, TreeNode.noOffset, 1, null);
+        int bytesLength = 0;
+        if (componentToInitializeFrom != null) {
+          // If initializing from a component it has to include the sdk,
+          // so we explicitly don't load it here.
+          initializeFromComponent(uriTranslator, c, data);
+        } else {
+          List<int> summaryBytes = await c.options.loadSdkSummaryBytes();
+          bytesLength = prepareSummary(summaryBytes, uriTranslator, c, data);
+          if (initializeFromDillUri != null) {
+            try {
+              bytesLength += await initializeFromDill(uriTranslator, c, data);
+            } catch (e, st) {
+              // We might have loaded x out of y libraries into the component.
+              // To avoid any unforeseen problems start over.
+              bytesLength =
+                  prepareSummary(summaryBytes, uriTranslator, c, data);
+
+              if (e is InvalidKernelVersionError || e is PackageChangedError) {
+                // Don't report any warning.
               } else {
-                // Unknown error: Report problem as such.
-                Message message = gzInitializedFrom != null
-                    ? templateInitializeFromDillUnknownProblem.withArguments(
-                        initializeFromDillUri.toString(),
-                        "$e",
-                        "$st",
-                        gzInitializedFrom)
-                    : templateInitializeFromDillUnknownProblemNoDump
-                        .withArguments(
-                            initializeFromDillUri.toString(), "$e", "$st");
-                dillLoadedData.loader
-                    .addProblem(message, TreeNode.noOffset, 1, null);
+                Uri gzInitializedFrom;
+                if (c.options.writeFileOnCrashReport) {
+                  gzInitializedFrom = saveAsGzip(
+                      data.initializationBytes, "initialize_from.dill");
+                  recordTemporaryFileForTesting(gzInitializedFrom);
+                }
+                if (e is CanonicalNameError) {
+                  Message message = gzInitializedFrom != null
+                      ? templateInitializeFromDillNotSelfContained
+                          .withArguments(initializeFromDillUri.toString(),
+                              gzInitializedFrom)
+                      : templateInitializeFromDillNotSelfContainedNoDump
+                          .withArguments(initializeFromDillUri.toString());
+                  dillLoadedData.loader
+                      .addProblem(message, TreeNode.noOffset, 1, null);
+                } else {
+                  // Unknown error: Report problem as such.
+                  Message message = gzInitializedFrom != null
+                      ? templateInitializeFromDillUnknownProblem.withArguments(
+                          initializeFromDillUri.toString(),
+                          "$e",
+                          "$st",
+                          gzInitializedFrom)
+                      : templateInitializeFromDillUnknownProblemNoDump
+                          .withArguments(
+                              initializeFromDillUri.toString(), "$e", "$st");
+                  dillLoadedData.loader
+                      .addProblem(message, TreeNode.noOffset, 1, null);
+                }
               }
             }
           }
-        } else if (componentToInitializeFrom != null) {
-          initializeFromComponent(uriTranslator, c, data);
         }
         appendLibraries(data, bytesLength);
 
         await dillLoadedData.buildOutlines();
-        summaryBytes = null;
         userBuilders = <Uri, LibraryBuilder>{};
         platformBuilders = <LibraryBuilder>[];
         dillLoadedData.loader.builders.forEach((uri, builder) {
@@ -522,35 +527,32 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
   // This procedure will set up compiler from [componentToInitializeFrom].
   void initializeFromComponent(UriTranslator uriTranslator, CompilerContext c,
       IncrementalCompilerData data) {
-    ticker.logMs("Read initializeFromComponent");
-
-    // [libraries] and [uriToSource] from [componentToInitializeFrom] take
-    // precedence over what was already read into [data.component]. Assumption
-    // is that [data.component] is initialized with standard prebuilt various
-    // platform libraries.
-    List<Library> combinedLibs = <Library>[];
-    Set<Uri> readLibs =
-        componentToInitializeFrom.libraries.map((lib) => lib.fileUri).toSet();
-    combinedLibs.addAll(componentToInitializeFrom.libraries);
-    for (Library lib in data.component.libraries) {
-      if (!readLibs.contains(lib.fileUri)) {
-        combinedLibs.add(lib);
+    ticker.logMs("About to initializeFromComponent");
+
+    dillLoadedData = new DillTarget(ticker, uriTranslator, c.options.target);
+    data.component = new Component(
+        libraries: componentToInitializeFrom.libraries,
+        uriToSource: componentToInitializeFrom.uriToSource)
+      ..mainMethod = componentToInitializeFrom.mainMethod;
+    data.userLoadedUriMain = componentToInitializeFrom.mainMethod;
+    saveComponentProblems(data);
+
+    bool foundDartCore = false;
+    for (int i = 0; i < data.component.libraries.length; i++) {
+      Library library = data.component.libraries[i];
+      if (library.importUri.scheme == "dart" &&
+          library.importUri.path == "core") {
+        foundDartCore = true;
+        break;
       }
     }
-    Map<Uri, Source> combinedMaps = new Map<Uri, Source>();
-    combinedMaps.addAll(componentToInitializeFrom.uriToSource);
-    Set<Uri> uris = combinedMaps.keys.toSet();
-    for (MapEntry<Uri, Source> entry in data.component.uriToSource.entries) {
-      if (!uris.contains(entry.key)) {
-        combinedMaps[entry.key] = entry.value;
-      }
+
+    if (!foundDartCore) {
+      throw const InitializeFromComponentError("Did not find dart:core when "
+          "tried to initialize from component.");
     }
 
-    data.component =
-        new Component(libraries: combinedLibs, uriToSource: combinedMaps)
-          ..mainMethod = componentToInitializeFrom.mainMethod;
-    data.userLoadedUriMain = data.component.mainMethod;
-    saveComponentProblems(data);
+    ticker.logMs("Ran initializeFromComponent");
   }
 
   void appendLibraries(IncrementalCompilerData data, int bytesLength) {
@@ -798,6 +800,14 @@ class PackageChangedError {
   const PackageChangedError();
 }
 
+class InitializeFromComponentError {
+  final String message;
+
+  const InitializeFromComponentError(this.message);
+
+  String toString() => message;
+}
+
 class IncrementalCompilerData {
   Procedure userLoadedUriMain = null;
   Component component = null;

pkg/front_end/testcases/incremental_initialize_from_dill/load_from_component_explicitly_import_dart_core.yaml

@@ -2,14 +2,16 @@
 # 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.
 
-# Initialize from component, where the component is linked to one sdk, and where
-# the incremental compiler loads another sdk. Risk when doing this: Having two
-# definitions of the same thing (e.g. the class 'String'), which could lead to
-# errors such as "The argument type 'dart.core::String' can't be assigned to
+# Initialize from component, where the component is linked to a sdk, and where
+# the incremental compiler (could) load another sdk.
+# Risk when doing this: Having two definitions of the same thing
+# (e.g. the class 'String'), which could lead to errors such as
+# "The argument type 'dart.core::String' can't be assigned to
 # the parameter type 'dart.core::String'".
 
 type: newworld
 strong: true
+omitPlatform: false
 worlds:
   - entry: main.dart
     errors: false
@@ -28,9 +30,11 @@ worlds:
           print("Hello from useString: $s");
         }
     expectedLibraryCount: 2
+    expectsPlatform: true
   - entry: main.dart
     errors: false
     warnings: false
+    expectInitializeFromDill: false
     fromComponent: true
     invalidate:
       - main.dart
@@ -47,4 +51,5 @@ worlds:
         void useString(String s) {
           print("Hello from useString: $s");
         }
-    expectedLibraryCount: 2
+    expectedLibraryCount: 2
+    expectsPlatform: true

pkg/front_end/testcases/incremental_initialize_from_dill/status.status

@@ -3,5 +3,3 @@
 # BSD-style license that can be found in the LICENSE.md file.
 
 # Status file for the test suite ../test/incremental_load_from_dill_yaml_test.dart.
-
-load_from_component_explicitly_import_dart_core: Crash

pkg/vm/lib/frontend_server.dart

@@ -333,7 +333,7 @@ class FrontendCompiler implements CompilerInterface {
     Component component;
     if (options['incremental']) {
       _compilerOptions = compilerOptions;
-      _compilerOptions.omitPlatform = true;
+      _compilerOptions.omitPlatform = false;
       _generator =
           generator ?? _createGenerator(new Uri.file(_initializeFromDill));
       await invalidateIfInitializingFromDill();

pkg/vm/lib/incremental_compiler.dart

@@ -17,10 +17,6 @@ const String kDebugProcedureName = ":Eval";
 class IncrementalCompiler {
   IncrementalKernelGenerator _generator;
 
-  // Component that reflect current state of the compiler, which has not
-  // been yet accepted by the client. Is [null] if no compilation was done
-  // since last accept/reject acknowledgement by the client.
-  Component _candidate;
   // Component that reflect the state that was most recently accepted by the
   // client. Is [null], if no compilation results were accepted by the client.
   Component _lastKnownGood;
@@ -51,62 +47,70 @@ class IncrementalCompiler {
         entryPoint: entryPoint, fullComponent: fullComponent);
     initialized = true;
     fullComponent = false;
-    final bool firstDelta = _pendingDeltas.isEmpty;
     _pendingDeltas.add(component);
-    if (!firstDelta) {
-      // If more than one delta is pending, we need to combine them.
-      Procedure mainMethod;
-      Map<Uri, Library> combined = <Uri, Library>{};
-      for (Component delta in _pendingDeltas) {
-        if (delta.mainMethod != null) {
-          mainMethod = delta.mainMethod;
-        }
-        for (Library library in delta.libraries) {
-          combined[library.importUri] = library;
-        }
+    return _combinePendingDeltas(false);
+  }
+
+  _combinePendingDeltas(bool includePlatform) {
+    Procedure mainMethod;
+    Map<Uri, Library> combined = <Uri, Library>{};
+    Map<Uri, Source> uriToSource = new Map<Uri, Source>();
+    for (Component delta in _pendingDeltas) {
+      if (delta.mainMethod != null) {
+        mainMethod = delta.mainMethod;
+      }
+      uriToSource.addAll(delta.uriToSource);
+      for (Library library in delta.libraries) {
+        bool isPlatform =
+            library.importUri.scheme == "dart" && !library.isSynthetic;
+        if (!includePlatform && isPlatform) continue;
+        combined[library.importUri] = library;
       }
-      // TODO(vegorov) this needs to merge metadata repositories from deltas.
-      component = new Component(libraries: combined.values.toList())
-        ..mainMethod = mainMethod;
     }
-    _candidate = component;
-    return component;
+
+    // TODO(vegorov) this needs to merge metadata repositories from deltas.
+    return new Component(
+        libraries: combined.values.toList(), uriToSource: uriToSource)
+      ..mainMethod = mainMethod;
   }
 
   /// This lets incremental compiler know that results of last [compile] call
   /// were accepted, don't need to be included into subsequent [compile] calls
   /// results.
   accept() {
-    _pendingDeltas.clear();
-
     Map<Uri, Library> combined = <Uri, Library>{};
+    Map<Uri, Source> uriToSource = <Uri, Source>{};
+
     if (_lastKnownGood != null) {
       // TODO(aam): Figure out how to skip no-longer-used libraries from
       // [_lastKnownGood] libraries.
       for (Library library in _lastKnownGood.libraries) {
         combined[library.importUri] = library;
       }
+      uriToSource.addAll(_lastKnownGood.uriToSource);
     }
-    for (Library library in _candidate.libraries) {
+
+    Component candidate = _combinePendingDeltas(true);
+    for (Library library in candidate.libraries) {
       combined[library.importUri] = library;
     }
+    uriToSource.addAll(candidate.uriToSource);
+
     _lastKnownGood = new Component(
       libraries: combined.values.toList(),
-      uriToSource: _candidate.uriToSource,
-    )..mainMethod = _candidate.mainMethod;
-    for (final repo in _candidate.metadata.values) {
+      uriToSource: uriToSource,
+    )..mainMethod = candidate.mainMethod;
+    for (final repo in candidate.metadata.values) {
       _lastKnownGood.addMetadataRepository(repo);
     }
-
-    _candidate = null;
+    _pendingDeltas.clear();
   }
 
   /// This lets incremental compiler know that results of last [compile] call
   /// were rejected. Subsequent [compile] or [compileExpression] calls need to
   /// be processed without changes picked up by rejected [compile] call.
   reject() async {
     _pendingDeltas.clear();
-    _candidate = null;
     // Need to reset and warm up compiler so that expression evaluation requests
     // are processed in that known good state.
     _generator = new IncrementalKernelGenerator.fromComponent(