Use sdk summaries in front_end/kernel_generator.

pkg/front_end/lib/kernel_generator.dart

 // Copyright (c) 2016, 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.
 
 /// Defines the front-end API for converting source code to Dart Kernel objects.
 library front_end.kernel_generator;
 
 import 'compilation_error.dart';
 import 'compiler_options.dart';
 import 'dart:async';
 
+import 'package:analyzer/src/generated/source.dart' show SourceKind;
+import 'package:analyzer/src/summary/package_bundle_reader.dart'
+    show InSummarySource;
 // TODO(sigmund): move loader logic under front_end/lib/src/kernel/
 import 'package:kernel/analyzer/loader.dart';
 import 'package:kernel/kernel.dart';
 import 'package:source_span/source_span.dart' show SourceSpan;
 
 /// Generates a kernel representation of the program whose main library is in
 /// the given [source].
 ///
 /// Intended for whole program (non-modular) compilation.
 ///
 /// Given the Uri of a file containing a program's `main` method, this function
 /// follows `import`, `export`, and `part` declarations to discover the whole
 /// program, and converts the result to Dart Kernel format.
 ///
 /// If summaries are provided in [options], they may be used to speed up
 /// analysis, but they will not take the place of Dart source code (since the
 /// Dart source code is still needed to access the contents of method bodies).
 ///
 /// TODO(paulberry): will the VM have a pickled version of the SDK inside it? If
 /// so, then maybe this method should not convert SDK libraries to kernel.
 Future<Program> kernelForProgram(Uri source, CompilerOptions options) async {
-  var loader = await _createLoader(options);
+  var loader = await _createLoader(options, entry: source);
+  // TODO(sigmund): consider adding support for sdk summaries here as well.
   Program program = loader.loadProgram(source);
   _reportErrors(loader.errors, options.onError);
   return program;
 }
 
 /// Generates a kernel representation of the build unit whose source files are
 /// in [sources].
 ///
 /// Intended for modular compilation.
 ///
@@ skipping 12 matching lines @@
 /// permitted to refer to a part file declared in another build unit).
 ///
 /// The return value is a [Program] object with no main method set.
 /// TODO(paulberry): would it be better to define a data type in kernel to
 /// represent a bundle of all the libraries in a given build unit?
 ///
 /// TODO(paulberry): does additional information need to be output to allow the
 /// caller to match up referenced elements to the summary files they were
 /// obtained from?
 Future<Program> kernelForBuildUnit(
-        List<Uri> sources, CompilerOptions options) async {
+    List<Uri> sources, CompilerOptions options) async {
   var repository = new Repository();
-  var loader = await _createLoader(options, repository: repository);
-  // TODO(sigmund): add special handling for part files.
-  sources.forEach(loader.loadLibrary);
+  var loader = await _createLoader(options,
+      repository: repository, entry: sources.first);

[Paul Berry, 2016/12/09 13:41:40]

It seems weird to treat the first source any differently than the others.

Would it be better if we just said that the caller is required to set
CompilerOptions.packagesFilePath when calling `kernelForBuildUnit`?  Then we
wouldn't need to make an arbitrary decision about which file to use for package
discovery.

[Siggi Cherem (dart-lang), 2016/12/09 16:58:21]

Good point, will do once we decide on the other aspects below

[Siggi Cherem (dart-lang), 2016/12/09 20:46:40]

Done.

+  var context = loader.context;
+
+  // Process every library in the build unit.
+  for (var uri in sources) {
+    var source = context.sourceFactory.forUri2(uri);
+    //  We ignore part files, those are handled by their enclosing library.
+    if (context.computeKindOf(source) == SourceKind.PART) continue;
+    loader.loadLibrary(uri);
+  }
+
+  // Process any library that has been referenced and that was not loaded from a
+  // summary file.

[Paul Berry, 2016/12/09 13:41:40]

This is actually undesirable behavior IMHO.  It contradicts the comment above
("The compilation process is hermetic, meaning that the only files which will be
read are those listed in [sources], [CompilerOptions.inputSummaries], and
[CompilerOptions.sdkSummary].")  Keeping the build process hermetic makes the
code simpler, has desirable properties for Bazel integration, and is consistent
with what we're doing for DDC.

I think we should replace this loop with some error checking that verifies that
all referenced libraries either come from summaries or are included in
`sources`.

On a related note, we should also have some error checking to verify that for
each "part" declaration in a library in `sources`,   the corresponding part file
is also in `sources`.

[Siggi Cherem (dart-lang), 2016/12/09 16:58:21]

Let's chat more about this.

I initially was kin to keeping it hermetic too, but then realized this makes it
much easier to use outside of the context of blaze, e.g. from the command line.
I believe DDC also supports this mode of operation on the command line after
some user feedback. IIUC, there is a preference to list just a few entry-points
and not all the reachable files.

For this specific CL, I'm happy to change it back, but I'd like to figure out
how to support the following use cases:
(a) generate kernel for whole program (including sdk)
(b) generate kernel for whole program user code (no sdk is compiled)
(c) generate kernel for whole program user code + attach kernel for the sdk (sdk
is not compiled, but it is attached)
(d) generate kernel for a build unit.
(e) generate kernel for the sdk

Currently we do (a) via generateForProgram, and (d) via generateForBuildUnit.

This CL is sort of doing (b) by changing generateForBuildUnit to make it
non-hermetic. An alternative is to do it like (a) with an option to exclude the
SDK. I'd be happy to do that for now. Let's chat offline about what we do with
(c) and (e) as well. (c) is needed short term to test how to run a program until
the VM supports loading the sdk from a snapshot.

[Siggi Cherem (dart-lang), 2016/12/09 20:46:40]

Updated the code based on an offline discussion. Here is the summary:
- I made it hermetic by default.
- I added CompilerOptions.chaseDependencies to let you override the behavior and
include extra files. 
- kernelForProgram chases dependencies by default, it handles use-case #a and #b
right now.
- use case #c is expected to be temporary and we might not submit that to the
code (it wont be necessary as soon as the VM can load kernel with external
references to the sdk)

I agree about checking that [part] files are covered, but made it a TODO for
now.

+  for (int i = 0; i < repository.libraries.length; ++i) {

[Paul Berry, 2016/12/09 13:41:40]

Nit: since `i` is not used, this would be clearer as:

for (var lib in repository.libraries)

But I guess it's moot if we get rid of this loop :)

[asgerf, 2016/12/09 13:56:53]

I think the number of libraries will grow during iteration as more libraries are
loaded, so maybe that should be clarified in a comment.

[Siggi Cherem (dart-lang), 2016/12/09 16:58:21]

Asger is correct - the list is growing as more libraries are discovered, so the
list of libraries behaves like a work-queue.

[Siggi Cherem (dart-lang), 2016/12/09 20:46:40]

Done, added some documentation.

+    var lib = repository.libraries[i];
+    var source = context.sourceFactory.forUri2(lib.importUri);
+    if (source is InSummarySource) continue;
+    // Depending on the context we can allow or report an error when a library
+    // is loaded here. For command-line tools we want to allow it to
+    // make it easy to define build units in terms of a few entry point files.
+    // For a closed build system like bazel, we can report an error.
+    // TODO(sigmund): expose an option to indicate that this should be an error.
+    loader.ensureLibraryIsLoaded(lib);
+  }
+
   Program program = new Program(repository.libraries);
   _reportErrors(loader.errors, options.onError);
   return program;
 }
 
+/// Create a [DartLoader] using the provided [options].
+///
+/// If [options] contain no configuration to resolve `.packages`, the [entry]
+/// file will be used to search for a `.packages` file.
 Future<DartLoader> _createLoader(CompilerOptions options,
-    {Repository repository}) async {
+    {Repository repository, Uri entry}) async {
   var kernelOptions = _convertOptions(options);
-  var packages = await createPackages(options.packagesFilePath);
+  var packages = await createPackages(options.packagesFilePath,
+      discoveryPath: entry?.path);
   return new DartLoader(
       repository ?? new Repository(), kernelOptions, packages);
 }
 
 DartOptions _convertOptions(CompilerOptions options) {
   return new DartOptions(
       sdk: options.sdkPath,
+      sdkSummary: options.sdkSummary,
       packagePath: options.packagesFilePath,
       declaredVariables: options.declaredVariables);
 }
 
 void _reportErrors(List errors, ErrorHandler onError) {
   if (onError == null) return;
   for (var error in errors) {
     onError(new _DartkError(error));
   }
 }
 
 // TODO(sigmund): delete this class. Dartk should not format errors itself, we
 // should just pass them along.
 class _DartkError implements CompilationError {
   String get correction => null;
   SourceSpan get span => null;
   final String message;
   _DartkError(this.message);
 }