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

pkg/front_end/lib/kernel_generator.dart

Index: pkg/front_end/lib/kernel_generator.dart
diff --git a/pkg/front_end/lib/kernel_generator.dart b/pkg/front_end/lib/kernel_generator.dart
index e0ba4c782e326b5dcb2b06f745b1e2e2acdbd823..91c73c24bc2278db2bd4eb650c5c5556f555c49f 100644
--- a/pkg/front_end/lib/kernel_generator.dart
+++ b/pkg/front_end/lib/kernel_generator.dart
@@ -8,16 +8,9 @@ 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].
@@ -31,62 +24,25 @@ import 'src/simple_error.dart';
 /// 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));
-    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);
-    }
+  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;
 
-    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()));
+  if (program.mainMethod == null) {
+    options.onError(new SimpleError("No 'main' method found."));
     return null;
   }
+
+  return program;
 }
 
 /// Generates a kernel representation for a build unit.
@@ -116,71 +72,11 @@ Future<Program> kernelForProgram(Uri source, CompilerOptions options) async {
 /// 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.

 }