Unifying compiler context

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

Index: pkg/front_end/lib/src/fasta/compiler_context.dart
diff --git a/pkg/front_end/lib/src/fasta/compiler_context.dart b/pkg/front_end/lib/src/fasta/compiler_context.dart
index 466ecdf1b92149954a012193721704abaa5220bb..f63a582ed02438e9a9348e30c33cfcc7924201e0 100644
--- a/pkg/front_end/lib/src/fasta/compiler_context.dart
+++ b/pkg/front_end/lib/src/fasta/compiler_context.dart
@@ -6,12 +6,12 @@ library fasta.compiler_context;
 
 import 'dart:async' show Zone, runZoned;
 
+import 'package:front_end/compiler_options.dart';
 import 'package:front_end/file_system.dart';
-import 'package:front_end/physical_file_system.dart';
+import 'package:front_end/src/base/processed_options.dart';
 import 'package:front_end/src/fasta/fasta_codes.dart';
 import 'package:kernel/ast.dart' show Source;
-
-import 'compiler_command_line.dart' show CompilerCommandLine;
+import 'command_line_reporting.dart' as command_line_reporting;
 
 import 'colors.dart' show computeEnableColors;
 
@@ -21,13 +21,28 @@ import 'severity.dart' show Severity;
 
 final Object compilerContextKey = new Object();
 
+/// Shared context used throughout the compiler.
+///
+/// The compiler works with a single instance of this class. To avoid
+/// passing it around as an argument everywhere, it is stored as a zone-value.
+///
+/// For convenience the static getter [CompilerContext.current] retrieves the
+/// context stored in the current zone.
 class CompilerContext {
-  final FileSystem fileSystem = PhysicalFileSystem.instance;
-
-  final CompilerCommandLine options;
-
+  // TODO(sigmund): move here methods in ProcessedOptions that don't seem

[ahe, 2017/07/18 10:40:51]

"move" -> "Move".

Consider: "Move methods in ProcessedOptions here that don't ...

[Siggi Cherem (dart-lang), 2017/07/18 22:50:43]

Done.

+  // appropriate as "options", or consider merging ProcessedOptions entirely
+  // within this class, and depend only on the raw options here.
+  final ProcessedOptions options;
+
+  /// Sources seen by the compiler.
+  ///
+  /// This is populated as the compiler loads files, and it is used for error

[ahe, 2017/07/18 10:40:51]

loads -> reads

[Siggi Cherem (dart-lang), 2017/07/18 22:50:44]

Done.

+  /// reporting and to generate source location information in the compiled
+  /// programs.
   final Map<String, Source> uriToSource = <String, Source>{};
 
+  FileSystem get fileSystem => options.fileSystem;
+
   bool enableColorsCached = null;
 
   CompilerContext(this.options);
@@ -39,21 +54,23 @@ class CompilerContext {
   /// Report [message], for example, by printing it.
   void report(LocatedMessage message, Severity severity) {
     options.report(message, severity);
+    // TODO: default to > command_line_reporting.report;

[ahe, 2017/07/18 10:40:50]

I'm not sure we should submit this CL with this TODO.

[ahe, 2017/07/18 10:40:51]

No name on TODO.
"default" -> "Default"
";" -> "."

Write out what ">" means

[Siggi Cherem (dart-lang), 2017/07/18 22:50:43]

Sorry, I forgot to delete this line. It was a note-to-self to come back to it
before finishing the CL. I implemented the logic but forgot to remove this note.

We now default correctly to command_line_reporting.report in ProcessedOptions.

   }
 
   /// Report [message], for example, by printing it.
   void reportWithoutLocation(Message message, Severity severity) {
     options.reportWithoutLocation(message, severity);
+    // TODO: default to > command_line_reporting.reportWithoutLocation;

[ahe, 2017/07/18 10:40:50]

Ditto.

[Siggi Cherem (dart-lang), 2017/07/18 22:50:44]

Done.

   }
 
   /// Format [message] as a text string that can be included in generated code.
   String format(LocatedMessage message, Severity severity) {
-    return options.format(message, severity);
+    return command_line_reporting.format(message, severity);

[Siggi Cherem (dart-lang), 2017/07/18 00:11:08]

Peter - the default format has the notion of colors, but other than that it
didn't seem very command-line specific. Should we have it in a separate file
than `report`?

[ahe, 2017/07/18 10:40:51]

I think command_line_reporting uses a format that's suitable for command-line
applications.

It's not suitable for an IDE as it includes a source snippet.

It's not suitable for output in a log, or in a web page.

Have I overlooked some non-command-line use cases where this format is useful?

[Siggi Cherem (dart-lang), 2017/07/18 22:50:43]

I see, I don't have other use cases in mind at the moment. I overlooked at the
fact that format was also used in messages generated in the output program.

   }
 
   /// Format [message] as a text string that can be included in generated code.
   String formatWithoutLocation(Message message, Severity severity) {
-    return options.formatWithoutLocation(message, severity);
+    return command_line_reporting.formatWithoutLocation(message, severity);
   }
 
   static CompilerContext get current {
@@ -68,12 +85,23 @@ class CompilerContext {
     return context;
   }
 
-  /// Perform [action] in a [Zone] where [cl] will be available as
+  /// Perform [action] in a [Zone] where [this] will be available as
   /// `CompilerContext.current.options`.
-  static dynamic withGlobalOptions(
-      CompilerCommandLine cl, dynamic action(CompilerContext c)) {
-    CompilerContext c = new CompilerContext(cl);
-    return runZoned(() => action(c), zoneValues: {compilerContextKey: c});
+  T runInContext<T>(T action(CompilerContext c)) {
+    return runZoned(() => action(this), zoneValues: {compilerContextKey: this});
+  }
+
+  /// Perform [action] in a [Zone] where [options] will be available as
+  /// `CompilerContext.current.options`.
+  static T runWithOptions<T>(
+      ProcessedOptions options, T action(CompilerContext c)) {
+    return new CompilerContext(options).runInContext(action);
+  }
+
+  static T runWithDefaultOptions<T>(T action(CompilerContext c)) {
+    var options = new CompilerOptions();
+    var pOptions = new ProcessedOptions(options);

[ahe, 2017/07/18 10:40:51]

Consider:

var options = new ProcessedOptions(new CompilerOptions());

Just to avoid having a variable named pOptions. Otherwise, rename it to
processedOptions.

[Siggi Cherem (dart-lang), 2017/07/18 22:50:44]

Done.

+    return new CompilerContext(pOptions).runInContext(action);
   }
 
   static bool get enableColors {