Report MIXIN_TYPEDEF_CYCLE.

Report MIXIN_TYPEDEF_CYCLE.

R=brianwilkerson@google.com, jwren@google.com
BUG=

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

[jwren, 2013-07-10 22:08:31]

On 2013/07/10 21:27:36, scheglov wrote:

LGTM

[Brian Wilkerson, 2013-07-10 22:15:21]

I need more context to understand what this code ought to be doing. Where in the
spec is this problem discussed?

https://codereview.chromium.org/18293017/diff/1/editor/tools/plugins/com.goog...
File
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/error/CompileTimeErrorCode.java
(right):

https://codereview.chromium.org/18293017/diff/1/editor/tools/plugins/com.goog...
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/error/CompileTimeErrorCode.java:856:
* 9.1 Mixin Application: It is a compile-time error if <i>S</i> does not denote
a class available
This spec quote doesn't look right.

https://codereview.chromium.org/18293017/diff/1/editor/tools/plugins/com.goog...
File
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/internal/verifier/ErrorVerifier.java
(right):

https://codereview.chromium.org/18293017/diff/1/editor/tools/plugins/com.goog...
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/internal/verifier/ErrorVerifier.java:4677:
for (InterfaceType mixin : element.getMixins()) {
Should this also be looking at the supertype? It seems strange to limit the
check to mixins.

[scheglov, 2013-07-10 23:08:50]

PTAL

https://codereview.chromium.org/18293017/diff/1/editor/tools/plugins/com.goog...
File
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/error/CompileTimeErrorCode.java
(right):

https://codereview.chromium.org/18293017/diff/1/editor/tools/plugins/com.goog...
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/error/CompileTimeErrorCode.java:856:
* 9.1 Mixin Application: It is a compile-time error if <i>S</i> does not denote
a class available
On 2013/07/10 22:15:22, Brian Wilkerson wrote:
> This spec quote doesn't look right.

Hm...
Well, it seems that this should be just case for
TYPE_ALIAS_CANNOT_REFERENCE_ITSELF.

https://codereview.chromium.org/18293017/diff/1/editor/tools/plugins/com.goog...
File
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/internal/verifier/ErrorVerifier.java
(right):

https://codereview.chromium.org/18293017/diff/1/editor/tools/plugins/com.goog...
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/internal/verifier/ErrorVerifier.java:4677:
for (InterfaceType mixin : element.getMixins()) {
On 2013/07/10 22:15:22, Brian Wilkerson wrote:
> Should this also be looking at the supertype? It seems strange to limit the
> check to mixins.

We already report error for the supertype.

[Brian Wilkerson, 2013-07-11 00:46:19]

This still looks strange to me. I understand why we don't want to allow cycles
in the inheritance graph, but it seems like there might be some combination of
extends, with and inherits that could be missed by not having the whole graph
checked in one place.

Nevertheless, I think this is better than without it, so LGTM.

[scheglov, 2013-07-11 01:34:26]

Committed patchset #2 manually as r24902 (presubmit successful).