Tweak public APIs and use them in patch_sdk, dart2js, and kernel-service

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 'compiler_options.dart';
 import 'dart:async' show Future;
 import 'dart:async';
-import 'package:front_end/src/base/processed_options.dart';
-import 'src/fasta/dill/dill_target.dart' show DillTarget;
-import 'src/fasta/errors.dart' show InputError;
-import 'src/fasta/kernel/kernel_target.dart' show KernelTarget;
 import 'package:kernel/kernel.dart' show Program;
-import 'package:kernel/target/targets.dart' show TargetFlags;
-import 'package:kernel/target/vm_fasta.dart' show VmFastaTarget;
-import 'src/fasta/ticker.dart' show Ticker;
-import 'src/fasta/translate_uri.dart' show TranslateUri;
 import 'src/simple_error.dart';
+import 'src/kernel_generator_impl.dart';
 
 /// 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 `compileSdk` in [options] is true, the generated program will include
 /// code for the SDK.
 ///
-/// If summaries are provided in [options], they will be used to speed up
-/// the process. If in addition `compileSdk` is false, then the resulting
-/// program will not contain the sdk contents. This is useful when building apps
-/// for platforms that already embed the sdk (e.g. the VM), so there is no need
-/// to spend time and space rebuilding it.
+/// If summaries are provided in [options], the compiler will use them instead
+/// of compiling the libraries contained in those summaries. This is useful, for
+/// example, when compiling for platforms that already embed those sources (like
+/// the sdk in the standalone VM).
+///
+/// The input [source] is expected to be a script with a main method, otherwise
+/// an error is reported.
+// TODO(sigmund): rename to kernelForScript?
 Future<Program> kernelForProgram(Uri source, CompilerOptions options) async {
-  var fs = options.fileSystem;
-  report(String msg) {
-    options.onError(new SimpleError(msg));
+  options = new CompilerOptions.from(options)..chaseDependencies = true;

[Paul Berry, 2017/06/29 18:51:07]

This concerns me a lot.  If someone calls this API and deliberately sets
`chaseDependencies` to false, we should respect that option.

Maybe it would be better to change the default value of `chaseDependencies` to
`null`, so that `true` means "yes, chase dependencies", `false` means "no, be
hermetic", and `null` means "do whatever is the default behavior for the API
you're using.  Then we could move the behavior of providing the default into
ProcessedOptions.

[Siggi Cherem (dart-lang), 2017/06/30 04:12:02]

Done. I like your suggestion, it required that I added more parameters to
ProcessedOptions though, this relates also to the discussion about the
hermetic-file-system.


While we are here - wow do you feel about renaming the option to "hermetic"?

In a way the compiler is chasing dependencies regardless, the distinction in my
mind is whether it has access to them.

[Paul Berry, 2017/07/04 15:12:14]

I don't have strong feelings about the name--if "hermetic" makes more sense to
you, feel free to change it.  (But maybe consider changing it in a separate CL
just so that we keep issues separate; there's a lot going on in this CL
already!)

[Siggi Cherem (dart-lang), 2017/07/05 18:42:07]

Sounds good - I'll track this separately. Your comment on the error message on
the other file also made me realize that it would be nice to match the same term
here as what we use in error messages for our users.

+  var program = (await generateKernel([source], options))?.program;

[Paul Berry, 2017/06/29 18:51:07]

I would feel a lot safer if we could create a ProcessedOptions object here and
pass it into generateKernel.  That way there is no danger of generateKernel
accidentally bypassing ProcessedOptions and trying to read data out of the raw
options object by mistake.

[Siggi Cherem (dart-lang), 2017/06/30 04:12:02]

Done.

+  if (program == null) return null;
+
+  if (program.mainMethod == null) {
+    options.onError(new SimpleError("No 'main' method found."));
     return null;
   }
 
-  if (!await fs.entityForUri(source).exists()) {
-    return report("Entry-point file not found: $source");
-  }
-
-  var pOptions = new ProcessedOptions(options);
-
-  if (!await pOptions.validateOptions()) return null;
-
-  try {
-    TranslateUri uriTranslator = await pOptions.getUriTranslator();
-
-    var dillTarget = new DillTarget(new Ticker(isVerbose: false), uriTranslator,
-        new VmFastaTarget(new TargetFlags(strongMode: options.strongMode)));
-    var summary = await pOptions.sdkSummaryProgram;
-    if (summary != null) {
-      dillTarget.loader.appendLibraries(summary);
-    }
-
-    var kernelTarget =
-        new KernelTarget(options.fileSystem, dillTarget, uriTranslator);
-    kernelTarget.read(source);
-
-    await dillTarget.buildOutlines();
-    await kernelTarget.buildOutlines();
-    Program program = await kernelTarget.buildProgram(trimDependencies: true);
-
-    if (kernelTarget.errors.isNotEmpty) {
-      kernelTarget.errors.forEach(report);
-      return null;
-    }
-
-    if (program.mainMethod == null) {
-      return report("No 'main' method found.");
-    }
-
-    if (!options.compileSdk) {
-      // TODO(sigmund): ensure that the result is not including
-      // sources for the sdk, only external references.
-    }
-    return program;
-  } on InputError catch (e) {
-    options.onError(new SimpleError(e.format()));
-    return null;
-  }
+  return program;
 }
 
 /// Generates a kernel representation for a build unit.
 ///
 /// Intended for modular compilation.
 ///
 /// The build unit by default contains only the source files in [sources]

[ahe, 2017/07/05 13:29:39]

The term "build unit" isn't clearly defined.

[Siggi Cherem (dart-lang), 2017/07/05 18:42:07]

Done.

 /// (including library and part files), but if
 /// [CompilerOptions.chaseDependencies] is true, it may include some additional

[ahe, 2017/07/05 13:29:39]

Only document the behavior of CompilerOptions.chaseDependencies once.

[Siggi Cherem (dart-lang), 2017/07/05 18:42:07]

Done.

 /// source files.  All of the library files are transformed into Dart Kernel
 /// Library objects.
 ///
 /// By default, the compilation process is hermetic, meaning that the only files
 /// which will be read are those listed in [sources],
 /// [CompilerOptions.inputSummaries], and [CompilerOptions.sdkSummary].  If a
 /// source file attempts to refer to a file which is not obtainable from these
 /// URIs, that will result in an error, even if the file exists on the
 /// filesystem.
 ///
 /// When [CompilerOptions.chaseDependencies] is true, this default behavior
 /// changes, and any dependency of [sources] that is not listed in
 /// [CompilerOptions.inputSummaries] and [CompilerOptions.sdkSummary] is treated
 /// as an additional source file for the build unit.
 ///
 /// Any `part` declarations found in [sources] must refer to part files which
 /// are also listed in the build unit sources, otherwise an error results.  (It
 /// is not 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 {
-  var fs = options.fileSystem;
-  report(String msg) {
-    options.onError(new SimpleError(msg));
-    return null;
-  }
-
-  if (!options.chaseDependencies) {
-    // TODO(sigmund): add support, most likely we can do so by adding a wrapper
-    // on top of filesystem that restricts reads to a set of known files.
-    report("hermetic mode (chaseDependencies = false) is not implemented");
-    return null;
-  }
-
-  for (var source in sources) {
-    if (!await fs.entityForUri(source).exists()) {
-      return report("Entry-point file not found: $source");
-    }
-  }
-
-  var pOptions = new ProcessedOptions(options);
-
-  if (!await pOptions.validateOptions()) return null;
-
-  try {
-    TranslateUri uriTranslator = await pOptions.getUriTranslator();
-
-    var dillTarget = new DillTarget(new Ticker(isVerbose: false), uriTranslator,
-        new VmFastaTarget(new TargetFlags(strongMode: options.strongMode)));
-    var summary = await pOptions.sdkSummaryProgram;
-    if (summary != null) {
-      dillTarget.loader.appendLibraries(summary);
-    }
-
-    // TODO(sigmund): this is likely not going to work if done naively: if
-    // summaries contain external references we need to ensure they are loaded
-    // in a specific order.
-    for (var inputSummary in await pOptions.inputSummariesPrograms) {
-      dillTarget.loader.appendLibraries(inputSummary);
-    }
-
-    await dillTarget.buildOutlines();
-
-    var kernelTarget =
-        new KernelTarget(options.fileSystem, dillTarget, uriTranslator);
-    sources.forEach(kernelTarget.read);
-    await kernelTarget.buildOutlines();
-
-    Program program = await kernelTarget.buildProgram(trimDependencies: true);
-
-    if (kernelTarget.errors.isNotEmpty) {
-      kernelTarget.errors.forEach(report);
-      return null;
-    }
-
-    return program;
-  } on InputError catch (e) {
-    options.onError(new SimpleError(e.format()));
-    return null;
-  }
+  return (await generateKernel(sources, options))?.program;

[Paul Berry, 2017/06/29 18:51:07]

Similar issue here.

[Siggi Cherem (dart-lang), 2017/06/30 04:12:02]

Done.

 }