Rewire DDC to use the analyzer task model

Rewire DDC to use the analyzer task model

This is WIP - not quite ready for review.

Not all tests are passing yet.  Most checker and inference tests are passing, but about a quarter are failing - need to look.

The non-runtime changes under lib along with test/testing.dart are the only real changes.

We seem to be losing some type info - lots of new casts - but I see at least one removed.

R=brianwilkerson@google.com, jmesserly@google.com, leafp@google.com

Committed: https://github.com/dart-lang/dev_compiler/commit/6fe74a269f7877b6aae3d03bfd9be9e664968cf8

[vsm, 2015-09-18 22:57:28]

vsm@google.com changed reviewers:
+ leafp@google.com

[vsm, 2015-09-18 22:57:45]

https://codereview.chromium.org/1355893003/diff/1/pubspec.yaml
File pubspec.yaml (right):

https://codereview.chromium.org/1355893003/diff/1/pubspec.yaml#newcode40
pubspec.yaml:40: path: ../dart/sdk/pkg/analyzer/
This needs to be replaced with a new analyzer version when we're ready.

[vsm, 2015-09-18 23:32:12]

https://codereview.chromium.org/1355893003/diff/10001/lib/src/checker/rules.dart
File lib/src/checker/rules.dart (right):

https://codereview.chromium.org/1355893003/diff/10001/lib/src/checker/rules.d...
lib/src/checker/rules.dart:202: if (expr is SimpleIdentifier) {
Brian / Leaf - any suggestions on the right code here?  Without this, we're
hitting the missing type call before.  It looks like cases where the staticType
hasn't been registered with the AstNode but is on the element.

This fixes a number of "failures".

[Jennifer Messerly, 2015-09-19 00:18:45]

jmesserly@google.com changed reviewers:
+ jmesserly@google.com

[Jennifer Messerly, 2015-09-19 00:18:46]

Conceptually this is super cool. :)

We should track down why JS code qualify declined so much though.

https://codereview.chromium.org/1355893003/diff/1/lib/runtime/dart/_foreign_h...
File lib/runtime/dart/_foreign_helper.js (right):

https://codereview.chromium.org/1355893003/diff/1/lib/runtime/dart/_foreign_h...
lib/runtime/dart/_foreign_helper.js:121: return dart.as(a + b, core.String);
what happened here? This returns String:

String JS_STRING_CONCAT(String a, String b) {
  // This body is unused, only here for type analysis.
  return JS('String', '# + #', a, b);
}

Do we not implement JS builtin anymore? If not, can we at least just trust it?

https://codereview.chromium.org/1355893003/diff/10001/lib/runtime/dart/collec...
File lib/runtime/dart/collection.js (right):

https://codereview.chromium.org/1355893003/diff/10001/lib/runtime/dart/collec...
lib/runtime/dart/collection.js:63: if (dart.notNull(dart.dcall(core.identical,
core.identityHashCode, hashCode)) && dart.notNull(dart.dcall(core.identical,
core.identical, equals))) {
we seem to have lost a bunch of types -- lots more dcalls

https://codereview.chromium.org/1355893003/diff/10001/lib/src/checker/rules.dart
File lib/src/checker/rules.dart (right):

https://codereview.chromium.org/1355893003/diff/10001/lib/src/checker/rules.d...
lib/src/checker/rules.dart:202: if (expr is SimpleIdentifier) {
On 2015/09/18 23:32:12, vsm wrote:
> Brian / Leaf - any suggestions on the right code here?  Without this, we're
> hitting the missing type call before.  It looks like cases where the
staticType
> hasn't been registered with the AstNode but is on the element.
> 
> This fixes a number of "failures".

FYI, at least one place this used to happen was the "methodName" field of calls.
It happens because Analyzer doesn't need to compute the function type there, so
they don't (unless it's a tear off). However DDC has always relied on being able
to treat method calls as "tear off" + function call.

[Jennifer Messerly, 2015-09-19 00:23:38]

Anyway, I'm just guessing, but that was the one place I remember we were doing
magic with SimpleIdentifiers. Could be a wrong guess.

https://codereview.chromium.org/1355893003/diff/10001/lib/src/checker/rules.dart
File lib/src/checker/rules.dart (right):

https://codereview.chromium.org/1355893003/diff/10001/lib/src/checker/rules.d...
lib/src/checker/rules.dart:202: if (expr is SimpleIdentifier) {
On 2015/09/19 00:18:46, John Messerly wrote:
> On 2015/09/18 23:32:12, vsm wrote:
> > Brian / Leaf - any suggestions on the right code here?  Without this, we're
> > hitting the missing type call before.  It looks like cases where the
> staticType
> > hasn't been registered with the AstNode but is on the element.
> > 
> > This fixes a number of "failures".
> 
> FYI, at least one place this used to happen was the "methodName" field of
calls.
> It happens because Analyzer doesn't need to compute the function type there,
so
> they don't (unless it's a tear off). However DDC has always relied on being
able
> to treat method calls as "tear off" + function call.

To clarify that:

Given `obj.method(args)`

analyzer would only care about the .staticType result for the whole
MethodInvocation, as well as `obj` and each of `args`. It would also compute the
Element for the method invocation.

DDC, however, would ask for the staticType of `.method` SimpleIdentifier too.
And expect it to be a function type with the signature (argTypes -> resultType).

See this line:
https://github.com/dart-lang/dev_compiler/blob/94d823b08e1a6420823bc7bc894fa4...

[Jennifer Messerly, 2015-09-19 00:27:51]

https://codereview.chromium.org/1355893003/diff/1/lib/runtime/dart/_foreign_h...
File lib/runtime/dart/_foreign_helper.js (right):

https://codereview.chromium.org/1355893003/diff/1/lib/runtime/dart/_foreign_h...
lib/runtime/dart/_foreign_helper.js:121: return dart.as(a + b, core.String);
On 2015/09/19 00:18:46, John Messerly wrote:
> what happened here? This returns String:
> 
> String JS_STRING_CONCAT(String a, String b) {
>   // This body is unused, only here for type analysis.
>   return JS('String', '# + #', a, b);
> }
> 
> Do we not implement JS builtin anymore? If not, can we at least just trust it?

Ah, could be we lost this code:
https://github.com/dart-lang/dev_compiler/blob/94d823b08e1a6420823bc7bc894fa4...

note, that code's not exactly right either, it only happens to work for
dart:core types. Ideally we'd lookup the "type string" in the context where it
occurs, likely when we're doing name resolution.

Either that or just trust JS annotations, and infer their type from downward
inference. We have to trust them anyway, since we aren't typechecking the JS
code :). So hmm, maybe my vote is actually just to trust and ignore that type
string.

[vsm, 2015-09-21 15:47:13]

On 2015/09/19 00:23:38, John Messerly wrote:
> Anyway, I'm just guessing, but that was the one place I remember we were doing
> magic with SimpleIdentifiers. Could be a wrong guess.
> 
>
https://codereview.chromium.org/1355893003/diff/10001/lib/src/checker/rules.dart
> File lib/src/checker/rules.dart (right):
> 
>
https://codereview.chromium.org/1355893003/diff/10001/lib/src/checker/rules.d...
> lib/src/checker/rules.dart:202: if (expr is SimpleIdentifier) {
> On 2015/09/19 00:18:46, John Messerly wrote:
> > On 2015/09/18 23:32:12, vsm wrote:
> > > Brian / Leaf - any suggestions on the right code here?  Without this,
we're
> > > hitting the missing type call before.  It looks like cases where the
> > staticType
> > > hasn't been registered with the AstNode but is on the element.
> > > 
> > > This fixes a number of "failures".
> > 
> > FYI, at least one place this used to happen was the "methodName" field of
> calls.
> > It happens because Analyzer doesn't need to compute the function type there,
> so
> > they don't (unless it's a tear off). However DDC has always relied on being
> able
> > to treat method calls as "tear off" + function call.
> 
> To clarify that:
> 
> Given `obj.method(args)`
> 
> analyzer would only care about the .staticType result for the whole
> MethodInvocation, as well as `obj` and each of `args`. It would also compute
the
> Element for the method invocation.
> 
> DDC, however, would ask for the staticType of `.method` SimpleIdentifier too.
> And expect it to be a function type with the signature (argTypes ->
resultType).
> 
> See this line:
>
https://github.com/dart-lang/dev_compiler/blob/94d823b08e1a6420823bc7bc894fa4...

Thanks - John!  I think this is exactly the case where I'm seeing
MissingTypeErrors.

[vsm, 2015-09-21 15:49:07]

On 2015/09/19 00:27:51, John Messerly wrote:
>
https://codereview.chromium.org/1355893003/diff/1/lib/runtime/dart/_foreign_h...
> File lib/runtime/dart/_foreign_helper.js (right):
> 
>
https://codereview.chromium.org/1355893003/diff/1/lib/runtime/dart/_foreign_h...
> lib/runtime/dart/_foreign_helper.js:121: return dart.as(a + b, core.String);
> On 2015/09/19 00:18:46, John Messerly wrote:
> > what happened here? This returns String:
> > 
> > String JS_STRING_CONCAT(String a, String b) {
> >   // This body is unused, only here for type analysis.
> >   return JS('String', '# + #', a, b);
> > }
> > 
> > Do we not implement JS builtin anymore? If not, can we at least just trust
it?
> 
> Ah, could be we lost this code:
>
https://github.com/dart-lang/dev_compiler/blob/94d823b08e1a6420823bc7bc894fa4...
> 
> note, that code's not exactly right either, it only happens to work for
> dart:core types. Ideally we'd lookup the "type string" in the context where it
> occurs, likely when we're doing name resolution.
> 
> Either that or just trust JS annotations, and infer their type from downward
> inference. We have to trust them anyway, since we aren't typechecking the JS
> code :). So hmm, maybe my vote is actually just to trust and ignore that type
> string.

There is similar code in Analyzer I think.  It might only be affecting the
propagated type instead of the static one though.  Need to go look.

[Leaf, 2015-09-21 17:15:59]

I have a CL in progress that may help with some of this, but there are some
unresolved things to be sorted out still (e.g. how to deal with the JS types).

https://codereview.chromium.org/1355893003/diff/1/lib/runtime/dart/_foreign_h...
File lib/runtime/dart/_foreign_helper.js (right):

https://codereview.chromium.org/1355893003/diff/1/lib/runtime/dart/_foreign_h...
lib/runtime/dart/_foreign_helper.js:121: return dart.as(a + b, core.String);
On 2015/09/19 00:27:51, John Messerly wrote:
> On 2015/09/19 00:18:46, John Messerly wrote:
> > what happened here? This returns String:
> > 
> > String JS_STRING_CONCAT(String a, String b) {
> >   // This body is unused, only here for type analysis.
> >   return JS('String', '# + #', a, b);
> > }
> > 
> > Do we not implement JS builtin anymore? If not, can we at least just trust
it?
> 
> Ah, could be we lost this code:
>
https://github.com/dart-lang/dev_compiler/blob/94d823b08e1a6420823bc7bc894fa4...
> 
> note, that code's not exactly right either, it only happens to work for
> dart:core types. Ideally we'd lookup the "type string" in the context where it
> occurs, likely when we're doing name resolution.
> 
> Either that or just trust JS annotations, and infer their type from downward
> inference. We have to trust them anyway, since we aren't typechecking the JS
> code :). So hmm, maybe my vote is actually just to trust and ignore that type
> string.

Yes, I have't pulled this over yet.  I was a little unsure about what to do
here, since the comments indicate that the code was pulled over from the
analyzer, so I wanted to avoid duplicating it if it was already present. 
Clearly it's not kicking in if it is present though.  Does anyone know the
history of that code and the comment?

https://codereview.chromium.org/1355893003/diff/10001/lib/src/checker/rules.dart
File lib/src/checker/rules.dart (right):

https://codereview.chromium.org/1355893003/diff/10001/lib/src/checker/rules.d...
lib/src/checker/rules.dart:202: if (expr is SimpleIdentifier) {
On 2015/09/19 00:23:38, John Messerly wrote:
> On 2015/09/19 00:18:46, John Messerly wrote:
> > On 2015/09/18 23:32:12, vsm wrote:
> > > Brian / Leaf - any suggestions on the right code here?  Without this,
we're
> > > hitting the missing type call before.  It looks like cases where the
> > staticType
> > > hasn't been registered with the AstNode but is on the element.
> > > 
> > > This fixes a number of "failures".
> > 
> > FYI, at least one place this used to happen was the "methodName" field of
> calls.
> > It happens because Analyzer doesn't need to compute the function type there,
> so
> > they don't (unless it's a tear off). However DDC has always relied on being
> able
> > to treat method calls as "tear off" + function call.
> 
> To clarify that:
> 
> Given `obj.method(args)`
> 
> analyzer would only care about the .staticType result for the whole
> MethodInvocation, as well as `obj` and each of `args`. It would also compute
the
> Element for the method invocation.
> 
> DDC, however, would ask for the staticType of `.method` SimpleIdentifier too.
> And expect it to be a function type with the signature (argTypes ->
resultType).
> 
> See this line:
>
https://github.com/dart-lang/dev_compiler/blob/94d823b08e1a6420823bc7bc894fa4...

I didn't port that line over because I was a little worried about how it would
interact, but I'll look into it.  Should be fine, I don't know any reason not to
have the type there.

[vsm, 2015-09-21 17:21:34]

On 2015/09/21 17:15:59, Leaf wrote:
> I have a CL in progress that may help with some of this, but there are some
> unresolved things to be sorted out still (e.g. how to deal with the JS types).
> 
>
https://codereview.chromium.org/1355893003/diff/1/lib/runtime/dart/_foreign_h...
> File lib/runtime/dart/_foreign_helper.js (right):
> 
>
https://codereview.chromium.org/1355893003/diff/1/lib/runtime/dart/_foreign_h...
> lib/runtime/dart/_foreign_helper.js:121: return dart.as(a + b, core.String);
> On 2015/09/19 00:27:51, John Messerly wrote:
> > On 2015/09/19 00:18:46, John Messerly wrote:
> > > what happened here? This returns String:
> > > 
> > > String JS_STRING_CONCAT(String a, String b) {
> > >   // This body is unused, only here for type analysis.
> > >   return JS('String', '# + #', a, b);
> > > }
> > > 
> > > Do we not implement JS builtin anymore? If not, can we at least just trust
> it?
> > 
> > Ah, could be we lost this code:
> >
>
https://github.com/dart-lang/dev_compiler/blob/94d823b08e1a6420823bc7bc894fa4...
> > 
> > note, that code's not exactly right either, it only happens to work for
> > dart:core types. Ideally we'd lookup the "type string" in the context where
it
> > occurs, likely when we're doing name resolution.
> > 
> > Either that or just trust JS annotations, and infer their type from downward
> > inference. We have to trust them anyway, since we aren't typechecking the JS
> > code :). So hmm, maybe my vote is actually just to trust and ignore that
type
> > string.
> 
> Yes, I have't pulled this over yet.  I was a little unsure about what to do
> here, since the comments indicate that the code was pulled over from the
> analyzer, so I wanted to avoid duplicating it if it was already present. 
> Clearly it's not kicking in if it is present though.  Does anyone know the
> history of that code and the comment?

It may be from propagatedType computation, not staticType - don't remember for
sure.

We also "improved" upon it somewhat. E.g., there is also some syntax
differentiating nullable and non-nullable return values.  

> 
>
https://codereview.chromium.org/1355893003/diff/10001/lib/src/checker/rules.dart
> File lib/src/checker/rules.dart (right):
> 
>
https://codereview.chromium.org/1355893003/diff/10001/lib/src/checker/rules.d...
> lib/src/checker/rules.dart:202: if (expr is SimpleIdentifier) {
> On 2015/09/19 00:23:38, John Messerly wrote:
> > On 2015/09/19 00:18:46, John Messerly wrote:
> > > On 2015/09/18 23:32:12, vsm wrote:
> > > > Brian / Leaf - any suggestions on the right code here?  Without this,
> we're
> > > > hitting the missing type call before.  It looks like cases where the
> > > staticType
> > > > hasn't been registered with the AstNode but is on the element.
> > > > 
> > > > This fixes a number of "failures".
> > > 
> > > FYI, at least one place this used to happen was the "methodName" field of
> > calls.
> > > It happens because Analyzer doesn't need to compute the function type
there,
> > so
> > > they don't (unless it's a tear off). However DDC has always relied on
being
> > able
> > > to treat method calls as "tear off" + function call.
> > 
> > To clarify that:
> > 
> > Given `obj.method(args)`
> > 
> > analyzer would only care about the .staticType result for the whole
> > MethodInvocation, as well as `obj` and each of `args`. It would also compute
> the
> > Element for the method invocation.
> > 
> > DDC, however, would ask for the staticType of `.method` SimpleIdentifier
too.
> > And expect it to be a function type with the signature (argTypes ->
> resultType).
> > 
> > See this line:
> >
>
https://github.com/dart-lang/dev_compiler/blob/94d823b08e1a6420823bc7bc894fa4...
> 
> I didn't port that line over because I was a little worried about how it would
> interact, but I'll look into it.  Should be fine, I don't know any reason not
to
> have the type there.

I believe this is the cause of all the MissingTypeErrors I was seeing.

[Jennifer Messerly, 2015-09-21 18:19:00]

https://codereview.chromium.org/1355893003/diff/1/lib/runtime/dart/_foreign_h...
File lib/runtime/dart/_foreign_helper.js (right):

https://codereview.chromium.org/1355893003/diff/1/lib/runtime/dart/_foreign_h...
lib/runtime/dart/_foreign_helper.js:121: return dart.as(a + b, core.String);
On 2015/09/21 17:15:59, Leaf wrote:
> On 2015/09/19 00:27:51, John Messerly wrote:
> > On 2015/09/19 00:18:46, John Messerly wrote:
> > > what happened here? This returns String:
> > > 
> > > String JS_STRING_CONCAT(String a, String b) {
> > >   // This body is unused, only here for type analysis.
> > >   return JS('String', '# + #', a, b);
> > > }
> > > 
> > > Do we not implement JS builtin anymore? If not, can we at least just trust
> it?
> > 
> > Ah, could be we lost this code:
> >
>
https://github.com/dart-lang/dev_compiler/blob/94d823b08e1a6420823bc7bc894fa4...
> > 
> > note, that code's not exactly right either, it only happens to work for
> > dart:core types. Ideally we'd lookup the "type string" in the context where
it
> > occurs, likely when we're doing name resolution.
> > 
> > Either that or just trust JS annotations, and infer their type from downward
> > inference. We have to trust them anyway, since we aren't typechecking the JS
> > code :). So hmm, maybe my vote is actually just to trust and ignore that
type
> > string.
> 
> Yes, I have't pulled this over yet.  I was a little unsure about what to do
> here, since the comments indicate that the code was pulled over from the
> analyzer, so I wanted to avoid duplicating it if it was already present. 
> Clearly it's not kicking in if it is present though.  Does anyone know the
> history of that code and the comment?

yup, see here:
https://github.com/dart-lang/sdk/blob/f6e0a8f30b32af16f9fede4b65698ebfacb28d9...

note that it only uses propagatedType. We also tweaked our version: restricted
to only dart:_foreign_helper and it supports the dart2js syntax of "bottom".

Either way, the lookup is pretty bad, it only works for types in dart:core.

[vsm, 2015-09-22 18:06:28]

On 2015/09/21 18:19:00, John Messerly wrote:
>
https://codereview.chromium.org/1355893003/diff/1/lib/runtime/dart/_foreign_h...
> File lib/runtime/dart/_foreign_helper.js (right):
> 
>
https://codereview.chromium.org/1355893003/diff/1/lib/runtime/dart/_foreign_h...
> lib/runtime/dart/_foreign_helper.js:121: return dart.as(a + b, core.String);
> On 2015/09/21 17:15:59, Leaf wrote:
> > On 2015/09/19 00:27:51, John Messerly wrote:
> > > On 2015/09/19 00:18:46, John Messerly wrote:
> > > > what happened here? This returns String:
> > > > 
> > > > String JS_STRING_CONCAT(String a, String b) {
> > > >   // This body is unused, only here for type analysis.
> > > >   return JS('String', '# + #', a, b);
> > > > }
> > > > 
> > > > Do we not implement JS builtin anymore? If not, can we at least just
trust
> > it?
> > > 
> > > Ah, could be we lost this code:
> > >
> >
>
https://github.com/dart-lang/dev_compiler/blob/94d823b08e1a6420823bc7bc894fa4...
> > > 
> > > note, that code's not exactly right either, it only happens to work for
> > > dart:core types. Ideally we'd lookup the "type string" in the context
where
> it
> > > occurs, likely when we're doing name resolution.
> > > 
> > > Either that or just trust JS annotations, and infer their type from
downward
> > > inference. We have to trust them anyway, since we aren't typechecking the
JS
> > > code :). So hmm, maybe my vote is actually just to trust and ignore that
> type
> > > string.
> > 
> > Yes, I have't pulled this over yet.  I was a little unsure about what to do
> > here, since the comments indicate that the code was pulled over from the
> > analyzer, so I wanted to avoid duplicating it if it was already present. 
> > Clearly it's not kicking in if it is present though.  Does anyone know the
> > history of that code and the comment?
> 
> yup, see here:
>
https://github.com/dart-lang/sdk/blob/f6e0a8f30b32af16f9fede4b65698ebfacb28d9...
> 
> note that it only uses propagatedType. We also tweaked our version: restricted
> to only dart:_foreign_helper and it supports the dart2js syntax of "bottom".
> 
> Either way, the lookup is pretty bad, it only works for types in dart:core.

Just rebased and reran against the latest analyzer in sdk.   Most dcall
regressions have disappeared.  Still a lot of casts - primarily from JS I think.

[Brian Wilkerson, 2015-09-24 19:41:00]

brianwilkerson@google.com changed reviewers:
+ brianwilkerson@google.com

[Brian Wilkerson, 2015-09-24 19:41:01]

https://codereview.chromium.org/1355893003/diff/10001/lib/src/checker/rules.dart
File lib/src/checker/rules.dart (right):

https://codereview.chromium.org/1355893003/diff/10001/lib/src/checker/rules.d...
lib/src/checker/rules.dart:202: if (expr is SimpleIdentifier) {
> It happens because Analyzer doesn't need to compute the function type there,
> so they don't (unless it's a tear off). However DDC has always relied on being
> able to treat method calls as "tear off" + function call.

Seems to me that we should change analyzer to always compute a function type
there. It would be good for ddc and possibly for other clients as well.

[Leaf, 2015-09-24 19:48:21]

https://codereview.chromium.org/1355893003/diff/10001/lib/src/checker/rules.dart
File lib/src/checker/rules.dart (right):

https://codereview.chromium.org/1355893003/diff/10001/lib/src/checker/rules.d...
lib/src/checker/rules.dart:202: if (expr is SimpleIdentifier) {
On 2015/09/24 19:41:01, Brian Wilkerson wrote:
> > It happens because Analyzer doesn't need to compute the function type there,
> > so they don't (unless it's a tear off). However DDC has always relied on
being
> > able to treat method calls as "tear off" + function call.
> 
> Seems to me that we should change analyzer to always compute a function type
> there. It would be good for ddc and possibly for other clients as well.

I think the change on line 633 of this CL
https://codereview.chromium.org/1352763004/diff/80001/pkg/analyzer/lib/src/ge...
made this change unnecessary for DDC.  I had restricted the logic to strong mode
only, but could make it unconditional.

[Brian Wilkerson, 2015-09-24 21:17:48]

https://codereview.chromium.org/1355893003/diff/10001/lib/src/checker/rules.dart
File lib/src/checker/rules.dart (right):

https://codereview.chromium.org/1355893003/diff/10001/lib/src/checker/rules.d...
lib/src/checker/rules.dart:202: if (expr is SimpleIdentifier) {
Yes. I think I made a comment to that effect in the review of that CL. :-)

I don't mind having different values for the static type information in strong
mode, but I'd prefer not to have the presence or absence of information depend
on the mode.

[vsm, 2015-10-07 21:14:16]

PTAL - all tests pass with leafp's latest CL.  

Generated code looks good - we have a check on redirecting constructors - we
were apparently missing this before.

Still need to update the pubspec.yaml with a new analyzer version once leafp's
cl lands.

[Leaf, 2015-10-07 21:48:38]

lgtm

https://codereview.chromium.org/1355893003/diff/220001/pubspec.yaml
File pubspec.yaml (right):

https://codereview.chromium.org/1355893003/diff/220001/pubspec.yaml#newcode44
pubspec.yaml:44: dependency_overrides:
You'll want to make sure to take this out.

[Jennifer Messerly, 2015-10-07 21:50:27]

LGTM

https://codereview.chromium.org/1355893003/diff/220001/test/checker/checker_t...
File test/checker/checker_test.dart (right):

https://codereview.chromium.org/1355893003/diff/220001/test/checker/checker_t...
test/checker/checker_test.dart:1978:
/*severe:InvalidMethodOverride,severe:InvalidMethodOverride*/m(a) {}
is this an example of a duplicate error message? If so maybe add a TODO
somewhere

[vsm, 2015-10-07 22:02:21]

https://codereview.chromium.org/1355893003/diff/220001/test/checker/checker_t...
File test/checker/checker_test.dart (right):

https://codereview.chromium.org/1355893003/diff/220001/test/checker/checker_t...
test/checker/checker_test.dart:1978:
/*severe:InvalidMethodOverride,severe:InvalidMethodOverride*/m(a) {}
On 2015/10/07 21:50:26, John Messerly wrote:
> is this an example of a duplicate error message? If so maybe add a TODO
> somewhere

It's actually now 2 different override errors - one for the method in Base and
one for the method in I1.  The old code only looked at the first override.

[Brian Wilkerson, 2015-10-07 22:44:18]

LGTM

[vsm, 2015-10-08 20:43:01]

Committed patchset #15 (id:240001) manually as
6fe74a269f7877b6aae3d03bfd9be9e664968cf8 (presubmit successful).