Initial support for mixins in dartc.

Initial support for mixins in dartc.


R=brianwilkerson@google.com
BUG=

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

[Brian Wilkerson, 2013-01-08 16:40:06]

Unfortunately, I think there is one underlying problem that causes several
places in the code to be wrong. Specifically, given the following declarations:

class A { mA() {} }
class B extends A { mB() {} }
class C extends Object with B { mC() {} }

it is the case that C defines exactly two methods: mC and mB. The code appears
to assume that it also defines mA, which it doesn't.

Members defined in supertypes of a mixin type are not included in classes
derived from the application of the mixin. The interfaces implemented by the
mixin are not implemented by the application of the mixin (unless they are
specified explicitly).

I have tried to mark all of the places where the code violates this semantic,
but you should look over the rest because I could easily have missed something.

https://codereview.chromium.org/11776037/diff/1/compiler/java/com/google/dart...
File compiler/java/com/google/dart/compiler/resolver/ClassAliasElement.java
(right):

https://codereview.chromium.org/11776037/diff/1/compiler/java/com/google/dart...
compiler/java/com/google/dart/compiler/resolver/ClassAliasElement.java:1: //
Copyright (c) 2012, the Dart project authors. Please see the AUTHORS file
nit: copyright year

https://codereview.chromium.org/11776037/diff/1/compiler/java/com/google/dart...
File
compiler/java/com/google/dart/compiler/resolver/ClassElementImplementation.java
(right):

https://codereview.chromium.org/11776037/diff/1/compiler/java/com/google/dart...
compiler/java/com/google/dart/compiler/resolver/ClassElementImplementation.java:384:
for (InterfaceType mix : getMixins()) {
Semantics: can't add supertypes of mixins.

https://codereview.chromium.org/11776037/diff/1/compiler/java/com/google/dart...
File compiler/java/com/google/dart/compiler/resolver/ClassElementUnion.java
(right):

https://codereview.chromium.org/11776037/diff/1/compiler/java/com/google/dart...
compiler/java/com/google/dart/compiler/resolver/ClassElementUnion.java:80:
return ImmutableList.of();
This seems wrong, but I'm not sure I understand this class well enough to be
sure. Let's talk.

https://codereview.chromium.org/11776037/diff/1/compiler/java/com/google/dart...
File compiler/java/com/google/dart/compiler/resolver/ClassScope.java (right):

https://codereview.chromium.org/11776037/diff/1/compiler/java/com/google/dart...
compiler/java/com/google/dart/compiler/resolver/ClassScope.java:82: element =
scope.findElement(inLibrary, name, examinedTypes);
Semantics: we can't look in supertypes of a mixin.

https://codereview.chromium.org/11776037/diff/1/compiler/java/com/google/dart...
File compiler/java/com/google/dart/compiler/resolver/Elements.java (right):

https://codereview.chromium.org/11776037/diff/1/compiler/java/com/google/dart...
compiler/java/com/google/dart/compiler/resolver/Elements.java:335: MethodElement
result = lookupFieldElementGetter(mixinType.getElement(), name);
Semantics: can't look in supertypes of a mixin.

https://codereview.chromium.org/11776037/diff/1/compiler/java/com/google/dart...
compiler/java/com/google/dart/compiler/resolver/Elements.java:374: MethodElement
result = lookupFieldElementSetter(mixinType.getElement(), name);
Semantics: can't look in supertypes of a mixin.

https://codereview.chromium.org/11776037/diff/1/compiler/java/com/google/dart...
compiler/java/com/google/dart/compiler/resolver/Elements.java:867: if
(hasClassMember(mixinType.getElement(), name)) {
Semantics: can't look in supertypes of a mixin.

https://codereview.chromium.org/11776037/diff/1/compiler/java/com/google/dart...
compiler/java/com/google/dart/compiler/resolver/Elements.java:930:
addAllMembers(visited, allMembers, mix.getElement());
Semantics: can't look in supertypes of a mixin.

https://codereview.chromium.org/11776037/diff/1/compiler/java/com/google/dart...
File compiler/java/com/google/dart/compiler/resolver/SupertypeResolver.java
(right):

https://codereview.chromium.org/11776037/diff/1/compiler/java/com/google/dart...
compiler/java/com/google/dart/compiler/resolver/SupertypeResolver.java:164:
topLevelContext.onError(mixNode, ResolverErrorCode.BLACK_LISTED_IMPLEMENTS,
mixNode);
Does this need to be a different error message?

https://codereview.chromium.org/11776037/diff/1/compiler/java/com/google/dart...
compiler/java/com/google/dart/compiler/resolver/SupertypeResolver.java:172: if
(!classElement.isInterface()) {
I think isInterface will always be false. Can we remove the test, at least for
mixins?

https://codereview.chromium.org/11776037/diff/1/compiler/java/com/google/dart...
File
compiler/java/com/google/dart/compiler/type/InterfaceTypeImplementation.java
(right):

https://codereview.chromium.org/11776037/diff/1/compiler/java/com/google/dart...
compiler/java/com/google/dart/compiler/type/InterfaceTypeImplementation.java:145:
for (InterfaceType type : getElement().getMixins()) {
Semantics: I didn't look to see how this method is being used, but I don't think
we can treat mixins as additional interfaces so this is probably wrong.

https://codereview.chromium.org/11776037/diff/1/compiler/java/com/google/dart...
File compiler/java/com/google/dart/compiler/type/TypeAnalyzer.java (right):

https://codereview.chromium.org/11776037/diff/1/compiler/java/com/google/dart...
compiler/java/com/google/dart/compiler/type/TypeAnalyzer.java:3216:
superTypes.addAll(currentClass.getElement().getMixins());
Semantics: can't look in supertypes of a mixin.

https://codereview.chromium.org/11776037/diff/1/compiler/java/com/google/dart...
File compiler/java/com/google/dart/compiler/type/Types.java (right):

https://codereview.chromium.org/11776037/diff/1/compiler/java/com/google/dart...
compiler/java/com/google/dart/compiler/type/Types.java:218:
types.addAll(getSuperTypes(mixin));
Semantics: can't look in supertypes of a mixin.

https://codereview.chromium.org/11776037/diff/1/compiler/java/com/google/dart...
compiler/java/com/google/dart/compiler/type/Types.java:561: InterfaceType result
= checkedAsInstanceOf(asSupertype(ti, mixin), element,
Semantics: can't look in supertypes of a mixin.

https://codereview.chromium.org/11776037/diff/1/editor/tools/plugins/com.goog...
File
editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/model/DartClassTypeAlias.java
(right):

https://codereview.chromium.org/11776037/diff/1/editor/tools/plugins/com.goog...
editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/model/DartClassTypeAlias.java:20:
public interface DartClassTypeAlias extends CompilationUnitElement,
SourceManipulation,
Will we need to be able to access the list of mixin names separately?

https://codereview.chromium.org/11776037/diff/1/editor/tools/plugins/com.goog...
File
editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/utilities/bindings/BindingUtils.java
(right):

https://codereview.chromium.org/11776037/diff/1/editor/tools/plugins/com.goog...
editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/utilities/bindings/BindingUtils.java:102:
* A table mapping class type alias names to a list of function type aliases
defined with that
nit: "function" --> "class"

https://codereview.chromium.org/11776037/diff/1/editor/tools/plugins/com.goog...
editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/utilities/bindings/BindingUtils.java:1418:
supertypes.addAll(targetType.getMixins());
Semantics: In the one place I can find where this is used, this will cause us to
look at the supertypes of the mixins, which we can't do.

[scheglov, 2013-01-09 00:59:39]

PTAL

https://codereview.chromium.org/11776037/diff/1/compiler/java/com/google/dart...
File compiler/java/com/google/dart/compiler/resolver/ClassAliasElement.java
(right):

https://codereview.chromium.org/11776037/diff/1/compiler/java/com/google/dart...
compiler/java/com/google/dart/compiler/resolver/ClassAliasElement.java:1: //
Copyright (c) 2012, the Dart project authors. Please see the AUTHORS file
On 2013/01/08 16:40:06, Brian Wilkerson wrote:
> nit: copyright year

Done.

https://codereview.chromium.org/11776037/diff/1/compiler/java/com/google/dart...
File
compiler/java/com/google/dart/compiler/resolver/ClassElementImplementation.java
(right):

https://codereview.chromium.org/11776037/diff/1/compiler/java/com/google/dart...
compiler/java/com/google/dart/compiler/resolver/ClassElementImplementation.java:384:
for (InterfaceType mix : getMixins()) {
On 2013/01/08 16:40:06, Brian Wilkerson wrote:
> Semantics: can't add supertypes of mixins.

Done.

https://codereview.chromium.org/11776037/diff/1/compiler/java/com/google/dart...
File compiler/java/com/google/dart/compiler/resolver/ClassScope.java (right):

https://codereview.chromium.org/11776037/diff/1/compiler/java/com/google/dart...
compiler/java/com/google/dart/compiler/resolver/ClassScope.java:82: element =
scope.findElement(inLibrary, name, examinedTypes);
On 2013/01/08 16:40:06, Brian Wilkerson wrote:
> Semantics: we can't look in supertypes of a mixin.

Bad naming.
Here not actually "supertype", but "mixin".

I also will change it to use MixinScope, which does not ask super-class and
interfaces.

https://codereview.chromium.org/11776037/diff/1/compiler/java/com/google/dart...
File compiler/java/com/google/dart/compiler/resolver/Elements.java (right):

https://codereview.chromium.org/11776037/diff/1/compiler/java/com/google/dart...
compiler/java/com/google/dart/compiler/resolver/Elements.java:335: MethodElement
result = lookupFieldElementGetter(mixinType.getElement(), name);
On 2013/01/08 16:40:06, Brian Wilkerson wrote:
> Semantics: can't look in supertypes of a mixin.

Done.

https://codereview.chromium.org/11776037/diff/1/compiler/java/com/google/dart...
compiler/java/com/google/dart/compiler/resolver/Elements.java:374: MethodElement
result = lookupFieldElementSetter(mixinType.getElement(), name);
On 2013/01/08 16:40:06, Brian Wilkerson wrote:
> Semantics: can't look in supertypes of a mixin.

Done.

https://codereview.chromium.org/11776037/diff/1/compiler/java/com/google/dart...
compiler/java/com/google/dart/compiler/resolver/Elements.java:867: if
(hasClassMember(mixinType.getElement(), name)) {
On 2013/01/08 16:40:06, Brian Wilkerson wrote:
> Semantics: can't look in supertypes of a mixin.

Done.

https://codereview.chromium.org/11776037/diff/1/compiler/java/com/google/dart...
compiler/java/com/google/dart/compiler/resolver/Elements.java:930:
addAllMembers(visited, allMembers, mix.getElement());
On 2013/01/08 16:40:06, Brian Wilkerson wrote:
> Semantics: can't look in supertypes of a mixin.

Done.

https://codereview.chromium.org/11776037/diff/1/compiler/java/com/google/dart...
File compiler/java/com/google/dart/compiler/resolver/SupertypeResolver.java
(right):

https://codereview.chromium.org/11776037/diff/1/compiler/java/com/google/dart...
compiler/java/com/google/dart/compiler/resolver/SupertypeResolver.java:164:
topLevelContext.onError(mixNode, ResolverErrorCode.BLACK_LISTED_IMPLEMENTS,
mixNode);
On 2013/01/08 16:40:06, Brian Wilkerson wrote:
> Does this need to be a different error message?

Done.

https://codereview.chromium.org/11776037/diff/1/compiler/java/com/google/dart...
compiler/java/com/google/dart/compiler/resolver/SupertypeResolver.java:172: if
(!classElement.isInterface()) {
On 2013/01/08 16:40:06, Brian Wilkerson wrote:
> I think isInterface will always be false. Can we remove the test, at least for
> mixins?

AFAIK we decided to don't remove anything from DartC, at least until is does not
conflict with current spec.

Removed for mixin.

https://codereview.chromium.org/11776037/diff/1/compiler/java/com/google/dart...
File
compiler/java/com/google/dart/compiler/type/InterfaceTypeImplementation.java
(right):

https://codereview.chromium.org/11776037/diff/1/compiler/java/com/google/dart...
compiler/java/com/google/dart/compiler/type/InterfaceTypeImplementation.java:145:
for (InterfaceType type : getElement().getMixins()) {
On 2013/01/08 16:40:06, Brian Wilkerson wrote:
> Semantics: I didn't look to see how this method is being used, but I don't
think
> we can treat mixins as additional interfaces so this is probably wrong.

Done.

https://codereview.chromium.org/11776037/diff/1/compiler/java/com/google/dart...
File compiler/java/com/google/dart/compiler/type/TypeAnalyzer.java (right):

https://codereview.chromium.org/11776037/diff/1/compiler/java/com/google/dart...
compiler/java/com/google/dart/compiler/type/TypeAnalyzer.java:3216:
superTypes.addAll(currentClass.getElement().getMixins());
On 2013/01/08 16:40:06, Brian Wilkerson wrote:
> Semantics: can't look in supertypes of a mixin.

Done.

https://codereview.chromium.org/11776037/diff/1/compiler/java/com/google/dart...
File compiler/java/com/google/dart/compiler/type/Types.java (right):

https://codereview.chromium.org/11776037/diff/1/compiler/java/com/google/dart...
compiler/java/com/google/dart/compiler/type/Types.java:218:
types.addAll(getSuperTypes(mixin));
On 2013/01/08 16:40:06, Brian Wilkerson wrote:
> Semantics: can't look in supertypes of a mixin.

Done.

https://codereview.chromium.org/11776037/diff/1/compiler/java/com/google/dart...
compiler/java/com/google/dart/compiler/type/Types.java:561: InterfaceType result
= checkedAsInstanceOf(asSupertype(ti, mixin), element,
On 2013/01/08 16:40:06, Brian Wilkerson wrote:
> Semantics: can't look in supertypes of a mixin.

Done.

https://codereview.chromium.org/11776037/diff/1/editor/tools/plugins/com.goog...
File
editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/model/DartClassTypeAlias.java
(right):

https://codereview.chromium.org/11776037/diff/1/editor/tools/plugins/com.goog...
editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/model/DartClassTypeAlias.java:20:
public interface DartClassTypeAlias extends CompilationUnitElement,
SourceManipulation,
On 2013/01/08 16:40:06, Brian Wilkerson wrote:
> Will we need to be able to access the list of mixin names separately?

  Yes.

typedef M = A with B;

  We need to rename M, navigate to it, etc.

https://codereview.chromium.org/11776037/diff/1/editor/tools/plugins/com.goog...
File
editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/utilities/bindings/BindingUtils.java
(right):

https://codereview.chromium.org/11776037/diff/1/editor/tools/plugins/com.goog...
editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/utilities/bindings/BindingUtils.java:102:
* A table mapping class type alias names to a list of function type aliases
defined with that
On 2013/01/08 16:40:06, Brian Wilkerson wrote:
> nit: "function" --> "class"

Done.

https://codereview.chromium.org/11776037/diff/1/editor/tools/plugins/com.goog...
editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/utilities/bindings/BindingUtils.java:1418:
supertypes.addAll(targetType.getMixins());
On 2013/01/08 16:40:06, Brian Wilkerson wrote:
> Semantics: In the one place I can find where this is used, this will cause us
to
> look at the supertypes of the mixins, which we can't do.

We don't use this method.
Commented out.

[Brian Wilkerson, 2013-01-09 16:17:41]

LGTM once comments have been addressed

https://codereview.chromium.org/11776037/diff/13001/compiler/java/com/google/...
File compiler/java/com/google/dart/compiler/resolver/ClassScope.java (right):

https://codereview.chromium.org/11776037/diff/13001/compiler/java/com/google/...
compiler/java/com/google/dart/compiler/resolver/ClassScope.java:65: MixinScope
scope = new MixinScope(intfElement,
I think this wanted to still be ClassScope and you wanted to change it to
MixinScope on line 79.

https://codereview.chromium.org/11776037/diff/13001/compiler/java/com/google/...
File compiler/java/com/google/dart/compiler/resolver/Elements.java (right):

https://codereview.chromium.org/11776037/diff/13001/compiler/java/com/google/...
compiler/java/com/google/dart/compiler/resolver/Elements.java:380: result =
lookupFieldElementSetter(mixinType.getElement(), name);
I believe that this needs to invoke lookupFieldElementSetter0.

https://codereview.chromium.org/11776037/diff/13001/compiler/java/com/google/...
File
compiler/java/com/google/dart/compiler/type/InterfaceTypeImplementation.java
(right):

https://codereview.chromium.org/11776037/diff/13001/compiler/java/com/google/...
compiler/java/com/google/dart/compiler/type/InterfaceTypeImplementation.java:126:
for (InterfaceType intrface : getMixins()) {
nit: "intrface" --> "mixin"

https://codereview.chromium.org/11776037/diff/13001/editor/tools/plugins/com....
File
editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/utilities/bindings/BindingUtils.java
(right):

https://codereview.chromium.org/11776037/diff/13001/editor/tools/plugins/com....
editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/utilities/bindings/BindingUtils.java:1417:
//    supertypes.addAll(targetType.getMixins());
Please add a comment that adding the mixins might be wrong so that if someone
decides to uncomment this method they'll know that it might not be doing the
right thing.