Mark top-level error field as static.

Mark top-level error field as static.

Closes https://github.com/dart-lang/sdk/issues/30431

R=sjindel@google.com

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

[ahe, 2017-08-16 08:44:40]

Description was changed from

==========
Mark top-level error field as static.
==========

to

==========
Mark top-level error field as static.

Closes https://github.com/dart-lang/sdk/issues/30431
==========

[ahe, 2017-08-16 08:44:52]

ahe@google.com changed reviewers:
+ sjindel@google.com

[sjindel, 2017-08-16 10:27:01]

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

https://codereview.chromium.org/2999033002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/kernel/verifier.dart:49: String
checkLocation(TreeNode node, String name) {
How are these changes related?

[ahe, 2017-08-16 11:59:15]

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

https://codereview.chromium.org/2999033002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/kernel/verifier.dart:49: String
checkLocation(TreeNode node, String name) {
On 2017/08/16 10:27:01, sjindel wrote:
> How are these changes related?

Without the change on line 79, Uri.parse would throw a NSM error as #errors
doesn't have a fileUri.

But that got me thinking that it would be a good idea to verify that all nodes
have location information, unless they're synthetic.

[ahe, 2017-08-16 12:01:47]

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

https://codereview.chromium.org/2999033002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/kernel/verifier.dart:49: String
checkLocation(TreeNode node, String name) {
On 2017/08/16 11:59:15, ahe wrote:
> On 2017/08/16 10:27:01, sjindel wrote:
> > How are these changes related?
> 
> Without the change on line 79, Uri.parse would throw a NSM error as #errors
> doesn't have a fileUri.
> 
> But that got me thinking that it would be a good idea to verify that all nodes
> have location information, unless they're synthetic.

It would throw a NSM error *before* I modified kernel_target.dart.

If you're asking why I'm making changes to the verifier, then the answer is that
this should prevent me or others from repeating the same mistake.

[sjindel, 2017-08-16 12:48:39]

On 2017/08/16 12:01:47, ahe wrote:
>
https://codereview.chromium.org/2999033002/diff/1/pkg/front_end/lib/src/fasta...
> File pkg/front_end/lib/src/fasta/kernel/verifier.dart (right):
> 
>
https://codereview.chromium.org/2999033002/diff/1/pkg/front_end/lib/src/fasta...
> pkg/front_end/lib/src/fasta/kernel/verifier.dart:49: String
> checkLocation(TreeNode node, String name) {
> On 2017/08/16 11:59:15, ahe wrote:
> > On 2017/08/16 10:27:01, sjindel wrote:
> > > How are these changes related?
> > 
> > Without the change on line 79, Uri.parse would throw a NSM error as #errors
> > doesn't have a fileUri.
> > 
> > But that got me thinking that it would be a good idea to verify that all
nodes
> > have location information, unless they're synthetic.
> 
> It would throw a NSM error *before* I modified kernel_target.dart.
> 
> If you're asking why I'm making changes to the verifier, then the answer is
that
> this should prevent me or others from repeating the same mistake.

Got it.
lgtm

[ahe, 2017-08-22 14:49:33]

Patchset #2 (id:20001) has been deleted

[ahe, 2017-08-22 14:53:31]

PTAL

I failed verify_test.dart and realized that I should extend it to test for this
case. Unfortunately, the test was written in such a way that we weren't sure
that the correct problem was being reported.

A number of negativeTest would incorrectly create a top-level field that wasn't
static, but actually test for something else. So I had to add a matcher.

[ahe, 2017-08-24 10:23:32]

Samir, did you have any comments on the latest patch set I added?

[sjindel, 2017-08-24 10:36:06]

On 2017/08/24 10:23:32, ahe wrote:
> Samir, did you have any comments on the latest patch set I added?

It looks good overall, but I wonder if we could avoid repeating the error
messages?
This would break tests if we change error messages where they are emitted, but
not here also.

Otherwise lgtm

[ahe, 2017-08-24 10:44:30]

On 2017/08/24 10:36:06, sjindel wrote:
> On 2017/08/24 10:23:32, ahe wrote:
> > Samir, did you have any comments on the latest patch set I added?
> 
> It looks good overall, but I wonder if we could avoid repeating the error
> messages?
> This would break tests if we change error messages where they are emitted, but
> not here also.

I agree. Eventually, I'd like to use the error codes we use in the front-end.
But I didn't wanna go down that route now.
 
> Otherwise lgtm

Thank you!

[ahe, 2017-08-29 10:39:07]

Description was changed from

==========
Mark top-level error field as static.

Closes https://github.com/dart-lang/sdk/issues/30431
==========

to

==========
Mark top-level error field as static.

Closes https://github.com/dart-lang/sdk/issues/30431

R=sjindel@google.com

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

[ahe, 2017-08-29 10:39:08]

Committed patchset #4 (id:80001) manually as
6eff945874618a5a59cf62eb6d9356c6774abd3c (presubmit successful).