Implement override checks.

Implement override checks.

First attempt committed as: https://code.google.com/p/dart/source/detail?r=9108

Committed: https://code.google.com/p/dart/source/detail?r=9151

[karlklose, 2012-06-20 13:51:31]

LGTM so far.

https://chromiumcodereview.appspot.com/10575033/diff/1/dart/lib/compiler/impl...
File dart/lib/compiler/implementation/resolver.dart (right):

https://chromiumcodereview.appspot.com/10575033/diff/1/dart/lib/compiler/impl...
dart/lib/compiler/implementation/resolver.dart:262: // But a field in this
class.
... or accessor ...?

[ahe, 2012-06-22 12:50:33]

Hi Karl,

Thank you for taking a look. I have uploaded a new version which I think is
correct (including status files).

However, I still need to make proper errors.

Cheers,
Peter

https://chromiumcodereview.appspot.com/10575033/diff/1/dart/lib/compiler/impl...
File dart/lib/compiler/implementation/resolver.dart (right):

https://chromiumcodereview.appspot.com/10575033/diff/1/dart/lib/compiler/impl...
dart/lib/compiler/implementation/resolver.dart:262: // But a field in this
class.
On 2012/06/20 13:51:31, karlklose wrote:
> ... or accessor ...?

Done.

[ahe, 2012-06-25 12:53:00]

Florian, Lasse: Karl already reviewed the resolution logic, but the diagnostic
changes are fairly substantial, so I would appreciate if you could take a look
as well.

[ahe, 2012-06-26 10:01:20]

Hi Søren,

I think Lasse and Florian are busy. Perhaps you have time to take a look?

Cheers,
Peter

[Søren Gjesse, 2012-06-26 10:46:46]

lgtm

https://chromiumcodereview.appspot.com/10575033/diff/16001/dart/lib/compiler/...
File dart/lib/compiler/implementation/warnings.dart (right):

https://chromiumcodereview.appspot.com/10575033/diff/16001/dart/lib/compiler/...
dart/lib/compiler/implementation/warnings.dart:188: static final
NO_STATIC_OVERRIDE = const MessageKind(
Maybe the messages which are related and used together when reporting context
information should be together in an array e.g.:

static final NO_STATIC_OVERRIDE = [
    const MessageKind(...),
    const MessageKind(...)]

Then this array can be used where the message reporting takes both the actual
error message and some context information message - I guess in some cases the
context information could be several messages.

[floitsch, 2012-06-26 10:59:33]

LGTM.

[ahe, 2012-06-26 11:07:05]

https://chromiumcodereview.appspot.com/10575033/diff/16001/dart/lib/compiler/...
File dart/lib/compiler/implementation/warnings.dart (right):

https://chromiumcodereview.appspot.com/10575033/diff/16001/dart/lib/compiler/...
dart/lib/compiler/implementation/warnings.dart:188: static final
NO_STATIC_OVERRIDE = const MessageKind(
Great idea!

On 2012/06/26 10:46:46, Søren Gjesse wrote:
> Maybe the messages which are related and used together when reporting context
> information should be together in an array e.g.:
> 
> static final NO_STATIC_OVERRIDE = [
>     const MessageKind(...),
>     const MessageKind(...)]
> 
> Then this array can be used where the message reporting takes both the actual
> error message and some context information message - I guess in some cases the
> context information could be several messages.

[ahe, 2012-06-27 09:04:47]

I'll submit this TBR. Please take a look at these two patch sets which are all
that changed:

https://chromiumcodereview.appspot.com/10575033/diff2/22001:25001/dart/lib/co...
https://chromiumcodereview.appspot.com/10575033/diff2/22001:25001/dart/lib/co...

This CL was reverted because it triggered an assertion:
http://build.chromium.org/p/client.dart/builders/dart2js-linux-release-checke...

The problem is about this line in Resolver.resolveClass:

    if (element.isResolved) return;

It should be:

    if (element.isResolved || element.isBeingResolved) return;

Otherwise we'll attempt to resolve a class twice. However, since
element.isBeingResolved is being set by the *caller* of  Resolver.resolveClass,
it is not enough to make that one change. Fortunately, I think it is cleaner
with all the checking and setting of isResolved and isBeingResolved in the same
location.

[floitsch, 2012-06-27 12:01:21]

LGTM.