language: Make it an error to have two mixin applications of the same mixin

[rakudrama]

It very difficult to reason correctly about a class that mixes in the same mixin class twice along the inheritance chain. Most correct uses of the mixin require a lot of super. calls to avoid logic errors. See https://github.com/dart-lang/sdk/issues/26775 for an example of how difficult it is to simply correctly use a field of a mixin class.

I think we should make the repeated mixing-in of the same class an error. We can lift the error when a compelling use cases can be found. Most code that uses a mixin twice has unintended behaviour, e.g. https://github.com/dart-lang/sdk/issues/28771

• [ ] Make repeated mixins an error in Analyzer
• [ ] Make repeated mixins an error in VM
• [ ] Make repeated mixins an error in dart2js

We still don't have a clear sketch of how to implement this correctly and efficiently in JavaScript without doing a lot of work to clone code. Any work in this area will be delayed until after Kernel is working fully. JavaScript idioms for mixins (e.g. Object.assign to prototype) not handle the repeated mixin case correctly. We should help our customers not write code that will not be correctly compiled for multiple quarters, if at all.

[lrhn]

I'm not sure most mixins use super calls at all. They just add methods that act on other (abstract) methods. Look for example on Iterable, FixedLengthListMixin or HtmlMixin (the last one is from package:analysis_server). They just add methods, the ones in Iterable all do virtual calls to the abstract iterator getter, but not a super call.

There are some mixins that do use super calls. Those need to work, and I don't think it's that much harder to make them work, whether they are mixed in once or twice, in different class chains or in the same one. In either case, they need to work on different dynamic supertypes, and yes, that might require copying. I think Dart mixins are fundamentally about copying the methods into a new class. Any implementation that works without copying is doing an optimization, and it's great when that works, just as any other space-saving optimization. Still, two mixin applications, of the same mixin or different ones, do work on top of different supertype chains, and their super invocations see different superclasses.

I can see that it's easier when you can just compile the mixin function once, and store its super-dependency in a single private JS member that it can access by name, and that doesn't work if you have the same method twice in the same class chain. Maybe you'll need to make the function into a closure that closes over the name it uses for its super-reference. Maybe you can find something smarter, but it should be possible.

[eernstg]

I can follow Lasse's argument in the direction that "we can make it work", but I tend to think that we should also be aware of the the cognitive load created by the need to manage multiple copies of a given instance variable, as well as managing identical overriding method implementations (potentially causing multiple "copies" of the same side-effects on reachable state). This is a non-trivial cost that we incur from having multiple applications of the same mixin, and it also affects clients who just know that a given object has type MyMixin, but may not know that there are three of them..

One thing that usually comes up is that we may also mix in different instantiations of the same generic class (with MyMixin<int>, MyMixin<String>, directly or indirectly), but that shouldn't be much worse than implements.

[lrhn]

From what I'm gathering, the problem is mixing in the same (private?) field more than once. Would it be sufficient to not allow a mixin to override a private field with another? (Or public too, if necessary)

I see no reason to restrict mixins that doesn't have fields. Also, the restriction written in this issue doesn't preclude:

class X {
   int _x = 42;
   foo() => _x;
}
class Y {
  bar() => super._x;
}
class W = X with Y, X;  // Not mixing in X twice, just mixin it in once on top of itself.

So, are we sure the requirements here are both precise and sufficient?

[leafpetersen]

@rakudrama Any thoughts about Lasse's comments?

@kmillikin Do you expect that kernel will fix the implementation problems for dart2js?

[rakudrama]

Lasse has a good point that the problem I characterized as two mixin applications of the same class also occurs with mixing a class on top of itself. We should ban that too.

Public fields are no different to private fields. The mess can occur in a library or across several libraries.

Multiple mixins with fields rarely do what the programmer expects. At best they introduce inaccessible fields.

Code-only mixins would shadow an implementation with the same implementation. I'm not sure when this would be useful or how to use it in a non-obscure way. If the class is not abstract and implements the mixin's interface, what is to be gained?

At first sight you might think that UnmodifiableListView with IterableMixin replaces the efficient list length with the iteration-based one. Perhaps you want to avoid the some property of the specialized subtype's implemention. However, it does not work. get iterator returns a ListIterator, this calls get length because it is assumed that Lists can do that efficiently, resulting in unbounded recursion. My conclusion with this experiment is that for a mixin to be applied twice requires the mixin and all classes using it to be designed for that.

Mixins containing only abstract methods are interfaces and could be re-affirmed via implements.

[kmillikin]

Kernel doesn't have a ready solution to the code duplication problem for dart2js.

Currently for the VM, we have a mixin elimination transformation that introduces normal classes instead of mixin applications and copies the fields and methods from the mixin. Then we can resolve super calls in mixins so that super calls are always direct calls.

Unfortunately this transformation is not modular --- it needs access to method bodies and field initilizers for mixin classes in other libraries and requires recompilation if those bodies change.

[eernstg]

The reason why I prefer to ban multiple applications of the same mixin in the same class is conceptual and semantic, so it's just as bad when one copy is a superclass and another one is a mixin as it is when they are both mixins. So I agree with Stephen that they should be treated the same.

For the conceptual part, I don't think it is very helpful to be able to "be" something more than once without having a notion of roles. You can be an employee with several employers, so you have several employee roles, but if phoneNumber is a property of an employee then you'd want to say "give me your phoneNumber as an employee of that employer", not just "give me your phoneNumber".

In more concrete Dart terms, let's assume that class C has 3 copies of mixin M and you wish to invoke a specific copy of some method M.foo(). We may use the well-known workaround where we declare methods like foo1() => super.foo(); in a subclass of the first copy of M, foo2() => super.foo(); for the second copy of M, etc. But it's not a very convincing way to "be an M three times", because we can't use the interface for M, we have to use the new names foo1/2/3.

On top of that, using 3 mixins to model 3 employments is far too rigid to work well in real life, we'd want to create and cancel employments dynamically. Multiple copies of a mixin just doesn't do the job, it's much more likely to work if we use separate objects (say, instances of an Employment role class), and we can even make it look the same with forwarders like foo1() => myEmployment1.foo();.

Another potential justification could be encapsulated implementation: We could chain up the methods of the mixin such that an invocation of myC.foo() would invoke every implementation of foo because each of them calls super.foo(). In this case neither C itself nor its clients need to access each copy of M.foo individually. This could make sense if foo has side effects, because they would then occur 3 times, which means that we don't have 3 copies of M for nothing. Still, it's not likely to be hugely useful..

My impression is that the conceptual value of this feature is so low (probably negative) that it's a bad idea to invest a lot of effort into solving whatever technical difficulties it may raise.

Do you have a lot of cool use cases?

[leafpetersen]

@lrhn It's too late to do this for Dart 2. We should look at whether the new mixin syntax can avoid repeating this problem.