[dart2js] Add back support for '--dump-info' flag.

We had kept the 'dump-info' flag to support '--dump-info=binary' vs json output. But this leads to a confusing experience so instead I've added back in support for '--dump-info' on its own.

This will emit binary data if the flag is passed with no type. Which will require using dart2js_info tools.

Change-Id: Ibf51d3e453375ad143383246dd97e628c1b2a049
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/409821
Commit-Queue: Nate Biggs <[email protected]>
Reviewed-by: Mayank Patke <[email protected]>

Author: natebiggs

pkg/compiler/lib/src/dart2js.dart

@@ -17,7 +17,8 @@ import 'commandline_options.dart';
 import 'common/ram_usage.dart';
 import 'compiler.dart' as default_compiler show Compiler;
 import 'io/mapped_file.dart';
-import 'options.dart' show CompilerOptions, CompilerStage, FeatureOptions;
+import 'options.dart'
+    show CompilerOptions, CompilerStage, DumpInfoFormat, FeatureOptions;
 import 'source_file_provider.dart';
 import 'util/command_line.dart';
 import 'util/util.dart' show stackTraceFilePrefix;
@@ -344,17 +345,17 @@ Future<api.CompilationResult> compile(
   }
 
   void setDumpInfo(String argument) {
-    passThrough(Flags.dumpInfo);
-    if (argument == Flags.dumpInfo || argument == "${Flags.dumpInfo}=json") {
-      return;
-    }
-    if (argument == "${Flags.dumpInfo}=binary") {
+    final hasEnumFormatMatch = DumpInfoFormat.values.any(
+      (e) => '${Flags.dumpInfo}=${e.name}' == argument,
+    );
+    if (argument == Flags.dumpInfo || hasEnumFormatMatch) {
       passThrough(argument);
       return;
     }
     _helpAndFail(
       "Unsupported dump-info format '$argument', "
-      "supported formats are: json or binary",
+      "supported formats: "
+      "${DumpInfoFormat.values.map((e) => e.name).join(', ')}",
     );
   }
 

pkg/compiler/lib/src/dump_info.dart

@@ -75,7 +75,7 @@ class DumpInfoJsAstRegistry {
           !options.stage.emitsDumpInfo &&
           !options.stage.shouldWriteDumpInfoData;
 
-  bool get useBinaryFormat => options.useDumpInfoBinaryFormat;
+  bool get useBinaryFormat => options.dumpInfoFormat == DumpInfoFormat.binary;
 
   void registerEntityAst(Entity? entity, js_ast.Node code) {
     if (_disabled) return;
@@ -1637,7 +1637,7 @@ class DumpInfoTask extends CompilerTask implements InfoReporter {
   final bool useBinaryFormat;
 
   DumpInfoTask(this.options, this.measurer, this.outputProvider, this.reporter)
-    : useBinaryFormat = options.useDumpInfoBinaryFormat,
+    : useBinaryFormat = options.dumpInfoFormat == DumpInfoFormat.binary,
       super(measurer);
 
   @override

pkg/compiler/lib/src/options.dart

@@ -4,6 +4,7 @@
 
 library;
 
+import 'package:collection/collection.dart';
 // ignore: implementation_imports
 import 'package:front_end/src/api_unstable/dart2js.dart' as fe;
 
@@ -126,8 +127,7 @@ enum CompilerStage {
     return CompilerStage.values.map((p) => '`${p._stageFlag}`').join(', ');
   }
 
-  static CompilerStage fromFlag(String? stageFlag) {
-    if (stageFlag == null) return CompilerStage.all;
+  static CompilerStage _fromFlagString(String stageFlag) {
     for (final stage in CompilerStage.values) {
       if (stageFlag == stage._stageFlag) {
         return stage;
@@ -138,6 +138,24 @@ enum CompilerStage {
       'Supported values are: $validFlagValuesString',
     );
   }
+
+  /// Can be used from outside the compiler to determine which stage will run
+  /// based on provided flag.
+  ///
+  /// Used for internal build systems.
+  static CompilerStage fromFlag(String? stageFlag) {
+    return stageFlag == null ? CompilerStage.all : _fromFlagString(stageFlag);
+  }
+
+  static CompilerStage fromOptions(CompilerOptions options) {
+    final stageFlag = options._stageFlag;
+    if (stageFlag == null) {
+      return options._dumpInfoFormatOption != null
+          ? CompilerStage.dumpInfoAll
+          : CompilerStage.all;
+    }
+    return _fromFlagString(stageFlag);
+  }
 }
 
 /// A [FeatureOption] is both a set of flags and an option. By default, creating
@@ -290,6 +308,8 @@ abstract class DiagnosticOptions {
   bool showPackageWarningsFor(Uri uri);
 }
 
+enum DumpInfoFormat { binary, json }
+
 /// Object for passing options to the compiler. Superclasses are used to select
 /// subsets of these options, enabling each part of the compiler to depend on
 /// as few as possible.
@@ -470,9 +490,10 @@ class CompilerOptions implements DiagnosticOptions {
   /// a standalone task (without re-emitting JS).
   Uri? _dumpInfoDataUri;
 
-  /// Whether to use the new dump-info binary format. This will be the default
-  /// after a transitional period.
-  bool useDumpInfoBinaryFormat = false;
+  /// Which format the user has chosen to emit dump info in if any.
+  DumpInfoFormat? _dumpInfoFormatOption;
+  DumpInfoFormat get dumpInfoFormat =>
+      _dumpInfoFormatOption ?? DumpInfoFormat.json;
 
   /// If set, SSA intermediate form is dumped for methods with names matching
   /// this RegExp pattern.
@@ -712,7 +733,7 @@ class CompilerOptions implements DiagnosticOptions {
   late final CompilerStage stage = _calculateStage();
 
   CompilerStage _calculateStage() =>
-      _cfeOnly ? CompilerStage.cfe : CompilerStage.fromFlag(_stageFlag);
+      _cfeOnly ? CompilerStage.cfe : CompilerStage.fromOptions(this);
 
   Uri? _outputUri;
   Uri? outputUri;
@@ -895,9 +916,11 @@ class CompilerOptions implements DiagnosticOptions {
         options,
         '${Flags.dumpInfoDataUri}=',
       )
-      ..useDumpInfoBinaryFormat = _hasOption(
+      .._dumpInfoFormatOption = _extractEnumOption(
         options,
-        "${Flags.dumpInfo}=binary",
+        Flags.dumpInfo,
+        DumpInfoFormat.values,
+        emptyValue: DumpInfoFormat.binary,
       )
       ..dumpSsaPattern = _extractStringOption(
         options,
@@ -1270,6 +1293,22 @@ int? _extractIntOption(List<String> options, String prefix) {
   return (option == null) ? null : int.parse(option);
 }
 
+/// Extracts an enum value for the flag given by [prefix].
+///
+/// [emptyValue] is used to provide a default value when the flag is given but
+/// with no '=' value provided.
+T? _extractEnumOption<T extends Enum>(
+  List<String> options,
+  String prefix,
+  List<T> values, {
+  T? emptyValue,
+}) {
+  if (emptyValue != null && _hasOption(options, prefix)) return emptyValue;
+  String? option = _extractStringOption(options, '$prefix=', null);
+  if (option == null) return null;
+  return values.firstWhereOrNull((e) => e.name == option);
+}
+
 bool _hasOption(List<String> options, String option) {
   return options.contains(option);
 }

pkg/compiler/lib/src/serialization/task.dart

@@ -388,7 +388,7 @@ class SerializationTask extends CompilerTask {
     source.registerAbstractValueDomain(abstractValueDomain);
     return DumpInfoProgramData.readFromDataSource(
       source,
-      includeCodeText: !_options.useDumpInfoBinaryFormat,
+      includeCodeText: _options.dumpInfoFormat != DumpInfoFormat.binary,
     );
   }
 }