Improve error reporting from Kernel scripts

* Ensure that we have a .dill file and it has the correct version before
  we start decoding it in dump.dart.

* Throw errors, not strings.  Ensure that they have a useful
  toString().

* Do not print usage except when the command is invoked
  incorrectly (wrong number of arguments).  Once we get into the
  command it's less likely that it's been invoked incorrectly and more
  likely that something else has gone wrong.

  Because these utilities are invoked from other scripts (like the
  fasta command), printing their usage for problems other than
  invoking them correctly doesn't match the way that they were invoked
  and it's confusing.

Change-Id: I7832383594d2b3719a0a7a7392ba4685717a79d2
Reviewed-on: https://dart-review.googlesource.com/c/78206
Commit-Queue: Kevin Millikin <kmillikin@google.com>
Reviewed-by: Jens Johansen <jensj@google.com>

pkg/front_end/lib/src/base/processed_options.dart

@@ -316,7 +316,9 @@ class ProcessedOptions {
     if (_sdkSummaryComponent == null) {
       if (sdkSummary == null) return null;
       var bytes = await loadSdkSummaryBytes();
-      _sdkSummaryComponent = loadComponent(bytes, nameRoot);
+      if (bytes != null && bytes.isNotEmpty) {
+        _sdkSummaryComponent = loadComponent(bytes, nameRoot);
+      }
     }
     return _sdkSummaryComponent;
   }

pkg/kernel/bin/count_breakdown.dart

@@ -18,8 +18,9 @@ void usage() {
 }
 
 main(List<String> args) {
-  CommandLineHelper.requireExactlyOneArgument(true, args, usage);
-  Component component = CommandLineHelper.tryLoadDill(args[0], usage);
+  CommandLineHelper.requireExactlyOneArgument(args, usage,
+      requireFileExists: true);
+  Component component = CommandLineHelper.tryLoadDill(args[0]);
   TypeCounter counter = new TypeCounter();
   component.accept(counter);
   counter.printStats();

pkg/kernel/bin/dump.dart

@@ -24,8 +24,8 @@ void usage() {
 
 main(List<String> args) {
   CommandLineHelper.requireVariableArgumentCount([1, 2], args, usage);
-  CommandLineHelper.requireFileExists(args[0], usage);
-  var binary = CommandLineHelper.tryLoadDill(args[0], usage);
+  CommandLineHelper.requireFileExists(args[0]);
+  var binary = CommandLineHelper.tryLoadDill(args[0]);
   writeComponentToText(binary,
       path: args.length > 1 ? args[1] : null,
       showOffsets: const bool.fromEnvironment("showOffsets"));

pkg/kernel/bin/eval.dart

@@ -19,7 +19,8 @@ void usage() {
 }
 
 main(List<String> args) {
-  CommandLineHelper.requireExactlyOneArgument(true, args, usage);
-  Component component = CommandLineHelper.tryLoadDill(args[0], usage);
+  CommandLineHelper.requireExactlyOneArgument(args, usage,
+      requireFileExists: true);
+  Component component = CommandLineHelper.tryLoadDill(args[0]);
   new Interpreter(component).run();
 }

pkg/kernel/bin/size_breakdown.dart

@@ -20,7 +20,8 @@ void usage() {
 }
 
 main(args) {
-  CommandLineHelper.requireExactlyOneArgument(true, args, usage);
+  CommandLineHelper.requireExactlyOneArgument(args, usage,
+      requireFileExists: true);
   List<int> bytes = new File(args[0]).readAsBytesSync();
   try {
     Component p = new Component();

pkg/kernel/bin/split.dart

@@ -23,8 +23,9 @@ void usage() {
 }
 
 main(args) async {
-  CommandLineHelper.requireExactlyOneArgument(true, args, usage);
-  Component binary = CommandLineHelper.tryLoadDill(args[0], usage);
+  CommandLineHelper.requireExactlyOneArgument(args, usage,
+      requireFileExists: true);
+  Component binary = CommandLineHelper.tryLoadDill(args[0]);
 
   int part = 1;
   binary.libraries.forEach((lib) => lib.isExternal = true);

pkg/kernel/bin/type_check.dart

@@ -20,8 +20,9 @@ void usage() {
 }
 
 void main(List<String> args) {
-  CommandLineHelper.requireExactlyOneArgument(true, args, usage);
-  final binary = CommandLineHelper.tryLoadDill(args[0], usage);
+  CommandLineHelper.requireExactlyOneArgument(args, usage,
+      requireFileExists: true);
+  final binary = CommandLineHelper.tryLoadDill(args[0]);
   ErrorFormatter errorFormatter = new ErrorFormatter();
   new StrongModeTypeChecker(errorFormatter, binary)..checkComponent(binary);
   if (errorFormatter.numberOfFailures > 0) {

pkg/kernel/lib/binary/ast_from_binary.dart

@@ -23,9 +23,14 @@ class ParseError {
 }
 
 class InvalidKernelVersionError {
-  final String message;
+  final int version;
 
-  InvalidKernelVersionError(this.message);
+  InvalidKernelVersionError(this.version);
+
+  String toString() {
+    return 'Unexpected Kernel version ${version} '
+        '(expected ${Tag.BinaryFormatVersion}).';
+  }
 }
 
 class CanonicalNameError {
@@ -81,7 +86,7 @@ class BinaryBuilder {
       : _disableLazyReading = disableLazyReading;
 
   fail(String message) {
-    throw new ParseError(message,
+    throw ParseError(message,
         byteIndex: _byteOffset, filename: filename, path: debugPath.join('::'));
   }
 
@@ -266,7 +271,7 @@ class BinaryBuilder {
         return new TypeLiteralConstant(type);
     }
 
-    throw 'Invalid constant tag $constantTag';
+    throw fail('unexpected constant tag: $constantTag');
   }
 
   Constant readConstantReference() {
@@ -305,7 +310,7 @@ class BinaryBuilder {
     } else if (tag == Tag.Something) {
       return true;
     } else {
-      throw fail('Invalid Option tag: $tag');
+      throw fail('unexpected option tag: $tag');
     }
   }
 
@@ -394,7 +399,9 @@ class BinaryBuilder {
     while (_byteOffset > 0) {
       int size = readUint32();
       int start = _byteOffset - size;
-      if (start < 0) throw "Invalid component file: Indicated size is invalid.";
+      if (start < 0) {
+        throw fail("indicated size does not match file size");
+      }
       index.add(size);
       _byteOffset = start - 4;
     }
@@ -409,6 +416,18 @@ class BinaryBuilder {
   ///
   /// The input bytes may contain multiple files concatenated.
   void readComponent(Component component, {bool checkCanonicalNames: false}) {
+    // Check that we have a .dill file and it has the correct version before we
+    // start decoding it.  Otherwise we will fail for cryptic reasons.
+    int offset = _byteOffset;
+    int magic = readUint32();
+    if (magic != Tag.ComponentFile) {
+      throw ArgumentError('Not a .dill file (wrong magic number).');
+    }
+    int version = readUint32();
+    if (version != Tag.BinaryFormatVersion) {
+      throw InvalidKernelVersionError(version);
+    }
+    _byteOffset = offset;
     List<int> componentFileSizes = _indexComponents();
     if (componentFileSizes.length > 1) {
       _disableLazyReading = true;
@@ -451,7 +470,7 @@ class BinaryBuilder {
   void readSingleFileComponent(Component component,
       {bool checkCanonicalNames: false}) {
     List<int> componentFileSizes = _indexComponents();
-    if (componentFileSizes.isEmpty) throw "Invalid component data.";
+    if (componentFileSizes.isEmpty) throw fail("invalid component data");
     _readOneComponent(component, componentFileSizes[0]);
     if (_byteOffset < _bytes.length) {
       if (_byteOffset + 3 < _bytes.length) {
@@ -554,14 +573,12 @@ class BinaryBuilder {
 
     final int magic = readUint32();
     if (magic != Tag.ComponentFile) {
-      throw fail('This is not a binary dart file. '
-          'Magic number was: ${magic.toRadixString(16)}');
+      throw ArgumentError('Not a .dill file (wrong magic number).');
     }
 
     final int formatVersion = readUint32();
     if (formatVersion != Tag.BinaryFormatVersion) {
-      throw fail('Invalid kernel binary format version '
-          '(found ${formatVersion}, expected ${Tag.BinaryFormatVersion})');
+      throw InvalidKernelVersionError(formatVersion);
     }
 
     // Read component index from the end of this ComponentFiles serialized data.
@@ -579,15 +596,12 @@ class BinaryBuilder {
 
     final int magic = readUint32();
     if (magic != Tag.ComponentFile) {
-      throw fail('This is not a binary dart file. '
-          'Magic number was: ${magic.toRadixString(16)}');
+      throw ArgumentError('Not a .dill file (wrong magic number).');
     }
 
     final int formatVersion = readUint32();
     if (formatVersion != Tag.BinaryFormatVersion) {
-      throw new InvalidKernelVersionError(
-          'Invalid kernel binary format version '
-          '(found ${formatVersion}, expected ${Tag.BinaryFormatVersion})');
+      throw InvalidKernelVersionError(formatVersion);
     }
 
     // Read component index from the end of this ComponentFiles serialized data.
@@ -1192,7 +1206,7 @@ class BinaryBuilder {
       case Tag.AssertInitializer:
         return new AssertInitializer(readStatement());
       default:
-        throw fail('Invalid initializer tag: $tag');
+        throw fail('unexpected initializer tag: $tag');
     }
   }
 
@@ -1294,7 +1308,7 @@ class BinaryBuilder {
   VariableDeclaration readVariableReference() {
     int index = readUInt();
     if (index >= variableStack.length) {
-      throw fail('Invalid variable index: $index');
+      throw fail('unexpected variable index: $index');
     }
     return variableStack[index];
   }
@@ -1306,7 +1320,7 @@ class BinaryBuilder {
       case 1:
         return '||';
       default:
-        throw fail('Invalid logical operator index: $index');
+        throw fail('unexpected logical operator index: $index');
     }
   }
 
@@ -1538,7 +1552,7 @@ class BinaryBuilder {
       case Tag.ConstantExpression:
         return new ConstantExpression(readConstantReference());
       default:
-        throw fail('Invalid expression tag: $tag');
+        throw fail('unexpected expression tag: $tag');
     }
   }
 
@@ -1689,7 +1703,7 @@ class BinaryBuilder {
         var function = readFunctionNode();
         return new FunctionDeclaration(variable, function)..fileOffset = offset;
       default:
-        throw fail('Invalid statement tag: $tag');
+        throw fail('unexpected statement tag: $tag');
     }
   }
 
@@ -1832,7 +1846,7 @@ class BinaryBuilder {
         var bound = readDartTypeOption();
         return new TypeParameterType(typeParameterStack[index], bound);
       default:
-        throw fail('Invalid dart type tag: $tag');
+        throw fail('unexpected dart type tag: $tag');
     }
   }
 

pkg/kernel/lib/src/tool/command_line_util.dart

@@ -68,13 +68,13 @@ List<ProgramRoot> parseProgramRoots(List<String> embedderEntryPointManifests) {
 }
 
 class CommandLineHelper {
-  static requireExactlyOneArgument(
-      bool requireExistingFile, List<String> args, void Function() usage) {
+  static requireExactlyOneArgument(List<String> args, void Function() usage,
+      {bool requireFileExists: false}) {
     if (args.length != 1) {
       print("Expected exactly 1 argument, got ${args.length}.");
       usage();
     }
-    requireFileExists(args[0], usage);
+    if (requireFileExists) CommandLineHelper.requireFileExists(args[0]);
   }
 
   static requireVariableArgumentCount(
@@ -86,19 +86,20 @@ class CommandLineHelper {
     }
   }
 
-  static requireFileExists(String file, void Function() usage) {
+  static requireFileExists(String file) {
     if (!new File(file).existsSync()) {
-      print("Argument '$file' isn't an existing file.");
-      usage();
+      print("File $file doesn't exist.");
+      exit(1);
     }
   }
 
-  static Component tryLoadDill(String file, void Function() usage) {
+  static Component tryLoadDill(String file) {
     try {
       return loadComponentFromBinary(file);
     } catch (e) {
-      print("Argument '$file' isn't a dill file that can be loaded.");
-      usage();
+      print("$file can't be loaded.");
+      print(e);
+      exit(1);
     }
     return null;
   }