Move the 'verify' flag to buildProgram().

pkg/front_end/lib/src/fasta/kernel/kernel_target.dart

Index: pkg/front_end/lib/src/fasta/kernel/kernel_target.dart
diff --git a/pkg/front_end/lib/src/fasta/kernel/kernel_target.dart b/pkg/front_end/lib/src/fasta/kernel/kernel_target.dart
index 6d929ed07cf6d0da5336ea7299a3c6c6e2427709..59722bfa24ae88faff8d1c862df3de0eb0eed250 100644
--- a/pkg/front_end/lib/src/fasta/kernel/kernel_target.dart
+++ b/pkg/front_end/lib/src/fasta/kernel/kernel_target.dart
@@ -101,7 +101,7 @@ class KernelTarget extends TargetImplementation {
   SourceLoader<Library> loader;
   Program _program;
 
-  final List errors = [];
+  final List<String> errors = <String>[];

[Siggi Cherem (dart-lang), 2017/05/17 20:05:35]

btw - in the future I would like to pass in some kind of
error-handler/diagnostic-reporter that directly takes error objects. that way
formatting and other details to convert them into strings is handled elsewhere. 

That will make it easier to integrate different ways to report errors (e.g. like
analyzers --machine format)

[scheglov, 2017/05/17 20:58:08]

Which one? :-)
Sometimes these objects are strings, something InputError, sometimes
VerificationError.

[Siggi Cherem (dart-lang), 2017/05/17 21:08:31]

likely a new one :)

maybe FastaMessage or some new iteration of it? (it should be something for
which we have all error messages and error codes in the messages.yaml file.)

[ahe, 2017/05/19 10:47:16]

What's the purpose of this change?

[scheglov, 2017/05/19 15:53:53]

I think typed collections are better then untyped.
Now we are more consistent about what objects are put into errors.
Less cognitive load, less toString()s in code.

[ahe, 2017/05/22 09:14:10]

I'd appreciate if you tried to gather information about the reasons for why
stuff is written as it is before you start apply your personal subjective
standards to code. We can't just go around changing code because we feel that it
should be typed, private, or sorted by name. It doesn't scale when we all apply
subjective standards to code and just have it oscillate between various
stylistic choices. The only way to avoid this to accept that people sometimes
have different opinions than you, and try to go with the existing style in a
piece of code.

In this case, you throw away essential information by calling toString or format
(currently the Uri and offset, but long-term also the error code).

You do identify an issue: that this list contains various kinds of objects, but
this isn't a fix that helps us address that issue.

 
   final TypeBuilder dynamicType =
       new KernelNamedTypeBuilder("dynamic", null, -1, null);
@@ -119,8 +119,9 @@ class KernelTarget extends TargetImplementation {
   void addError(file, int charOffset, String message) {
     Uri uri = file is String ? Uri.parse(file) : file;
     InputError error = new InputError(uri, charOffset, message);
-    print(error.format());
-    errors.add(error);
+    String formatterMessage = error.format();
+    print(formatterMessage);
+    errors.add(formatterMessage);
   }
 
   SourceLoader<Library> createLoader() =>
@@ -275,7 +276,7 @@ class KernelTarget extends TargetImplementation {
   }
 
   @override
-  Future<Program> buildProgram() async {
+  Future<Program> buildProgram({bool verify: false}) async {
     if (loader.first == null) return null;
     if (errors.isNotEmpty) {
       return handleInputError(null, null, isFullProgram: true);
@@ -287,6 +288,7 @@ class KernelTarget extends TargetImplementation {
       loader.finishNativeMethods();
       runBuildTransformations();
 
+      if (verify) this.verify();
       errors.addAll(loader.collectCompileTimeErrors().map((e) => e.format()));
       if (errors.isNotEmpty) {
         return handleInputError(null, null, isFullProgram: true);
@@ -299,10 +301,9 @@ class KernelTarget extends TargetImplementation {
     }
   }
 
-  Future<Null> writeProgram(Uri uri, {bool verify: false}) async {
+  Future<Null> writeProgram(Uri uri) async {
     if (loader.first == null) return null;
     try {
-      if (verify) this.verify();
       await writeLinkedProgram(uri, _program, isFullProgram: true);
     } on InputError catch (e) {
       return handleInputError(uri, e, isFullProgram: true);
@@ -715,7 +716,8 @@ class KernelTarget extends TargetImplementation {
   }
 
   void verify() {
-    errors.addAll(verifyProgram(_program));
+    var verifyErrors = verifyProgram(_program);
+    errors.addAll(verifyErrors.map((error) => error.toString()));

[Siggi Cherem (dart-lang), 2017/05/17 20:05:35]

minor style suggestion: I generally prefer '$error' over error.toString()
becayse it's shorter and syntax highlighting makes it obvious this is a string.

[scheglov, 2017/05/17 20:58:08]

Done.

     ticker.logMs("Verified program");
   }
 }