Move the 'verify' flag to buildProgram().

Move the 'verify' flag to buildProgram().

Also make 'errors' typed.

R=ahe@google.com, paulberry@google.com, sigmund@google.com
BUG=

Committed: https://github.com/dart-lang/sdk/commit/bff1e003a8f690418fb33578597662fc91dc145a

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

lgtm

https://codereview.chromium.org/2887163003/diff/1/pkg/front_end/lib/src/fasta...
File pkg/front_end/lib/src/fasta/kernel/kernel_target.dart (right):

https://codereview.chromium.org/2887163003/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/kernel/kernel_target.dart:104: final List<String>
errors = <String>[];
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)

https://codereview.chromium.org/2887163003/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/kernel/kernel_target.dart:720:
errors.addAll(verifyErrors.map((error) => error.toString()));
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:57:49]

Description was changed from

==========
Move the 'verify' flag to buildProgram().

Also make 'errors' typed.

R=ahe@google.com, paulberry@google.com, sigmund@google.com
BUG=
==========

to

==========
Move the 'verify' flag to buildProgram().

Also make 'errors' typed.

R=ahe@google.com, paulberry@google.com, sigmund@google.com
BUG=

Committed:
https://github.com/dart-lang/sdk/commit/bff1e003a8f690418fb33578597662fc91dc145a
==========

[scheglov, 2017-05-17 20:57:50]

Committed patchset #1 (id:1) manually as
bff1e003a8f690418fb33578597662fc91dc145a (presubmit successful).

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

https://codereview.chromium.org/2887163003/diff/1/pkg/front_end/lib/src/fasta...
File pkg/front_end/lib/src/fasta/kernel/kernel_target.dart (right):

https://codereview.chromium.org/2887163003/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/kernel/kernel_target.dart:104: final List<String>
errors = <String>[];
On 2017/05/17 20:05:35, Siggi Cherem (dart-lang) wrote:
> 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)

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

https://codereview.chromium.org/2887163003/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/kernel/kernel_target.dart:720:
errors.addAll(verifyErrors.map((error) => error.toString()));
On 2017/05/17 20:05:35, Siggi Cherem (dart-lang) wrote:
> minor style suggestion: I generally prefer '$error' over error.toString()
> becayse it's shorter and syntax highlighting makes it obvious this is a
string.

Done.

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

https://codereview.chromium.org/2887163003/diff/1/pkg/front_end/lib/src/fasta...
File pkg/front_end/lib/src/fasta/kernel/kernel_target.dart (right):

https://codereview.chromium.org/2887163003/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/kernel/kernel_target.dart:104: final List<String>
errors = <String>[];
On 2017/05/17 20:58:08, scheglov wrote:
> On 2017/05/17 20:05:35, Siggi Cherem (dart-lang) wrote:
> > 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)
> 
> Which one? :-)
> Sometimes these objects are strings, something InputError, sometimes
> VerificationError.

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]

https://codereview.chromium.org/2887163003/diff/1/pkg/front_end/lib/src/fasta...
File pkg/front_end/lib/src/fasta/kernel/kernel_target.dart (right):

https://codereview.chromium.org/2887163003/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/kernel/kernel_target.dart:104: final List<String>
errors = <String>[];
What's the purpose of this change?

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

https://codereview.chromium.org/2887163003/diff/1/pkg/front_end/lib/src/fasta...
File pkg/front_end/lib/src/fasta/kernel/kernel_target.dart (right):

https://codereview.chromium.org/2887163003/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/kernel/kernel_target.dart:104: final List<String>
errors = <String>[];
On 2017/05/19 10:47:16, ahe wrote:
> What's the purpose of this change?

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]

https://codereview.chromium.org/2887163003/diff/1/pkg/front_end/lib/src/fasta...
File pkg/front_end/lib/src/fasta/kernel/kernel_target.dart (right):

https://codereview.chromium.org/2887163003/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/kernel/kernel_target.dart:104: final List<String>
errors = <String>[];
On 2017/05/19 15:53:53, scheglov wrote:
> On 2017/05/19 10:47:16, ahe wrote:
> > What's the purpose of this change?
> 
> 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.

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.