Mixin declaration allowing super-invocations. about language

[munificent]

I'm worried the on keyword is a little too opaque. Given that implements and extends are already pretty long, what do you think of requires? In the strawman, you consistently use "requirement" to refer to those, so it seems like that is the natural word for what's being described.

A possible slightly change would be to use the word "class" instead of the word "mixin" to declare mixins, and have that automatically define both a mixin and a class, and then to enforce the rules for mixins only when the identifier is used as a mixin.

In practice, most classes that we see today used as mixins are intended to only be used as mixins. Many even put "Mixin" in the class name. So it doesn't seem like having dual-use types is a highly valuable affordance.

By prohibiting using the mixin as a class, it means the author of the mixin doesn't have to worry about accidentally breaking users when they change the mixin in a way that makes it no longer a usable class (like adding an abstract method, etc.). It's roughly analogous to sealed classes, final methods etc. It lets the author narrow the scope of what future changes may break users.

Also, I think it gives users a clearer expectation of how to use the type. It lets the API author give users a more "on rails" experience.

It might be worth adding a way to explicitly opt in to defining a "mixin and class" type, but my feeling is that it's not a good default behavior.

[Hixie]

For the on issue: Maybe we could create a survey that presents people with a snippet of code, and ask them what they think it means (without reference to this proposal). If we put out several such surveys, we could find out which syntax idea is clearest. Maybe @InMatrix has some more rigorous ideas for how to do this. It seems like something eminently testable empirically.

For the backwards-compatibility issue: Could we instrument the analyzer to catch such cases, and anonymously report them? Flutter in particular has an analytics mechanism we could reuse for this. Obviously if all the uses are in the Flutter framework then there's no breakage concern.

[Hixie]

This seems like a reasonable proposal.

A possible slightly change would be to use the word "class" instead of the word "mixin" to declare mixins, and have that automatically define both a mixin and a class, and then to enforce the rules for mixins only when the identifier is used as a mixin. It seems like that would get all the benefits of the proposal while simultaneously avoiding having a backwards-incompatible change in syntax. Is there a reason I'm missing why that would not be possible?

[leafpetersen]

A possible slightly change would be to use the word "class" instead of the word "mixin"
A possible slightly change would be to use the word "class" instead of the word "mixin"

It's definitely possible, with various caveats. It has some downsides though.

One (mentioned in the proposal) is simply that allowing classes to be used as mixins makes it harder for library writers to control what is a breaking change. So having a separate construct for the separate intended use provides some benefits independent of anything else.

Another downside (or advantage of the new syntax, if you prefer) is that the class syntax doesn't tell the client which interfaces should be satisfied by the superclass, and which can be satisfied in other ways (e.g by the mixin itself, or by the class that the mixin is being mixed into).

Compare:

class A extends B implements C, D {...}

versus

mixin M on B, C implements D { ...}

the latter makes it clear that M should be mixed onto something satisfying B and C. For A, it's not clear what subset of B, C, and D are required by the superclasses. This is especially confusing if a super called method is provided by (say) both C and D. Does that mean that both C and D must be in the superclass chain? Or exactly one? Or at least one?

There are also compilation benefits to knowing whether something is intended to be used as a mixin or a class, particularly in a modular setting. If a compiler needs to be prepared for the possibility that any suitable class might be used as a mixin, then it is more restricted in the set of optimizations that it can do (such as inlining super calls).

[leafpetersen]

I'm worried the on keyword is a little too opaque. Given that implements and extends are already pretty long, what do you think of requires?

This was considered. I don't have strong feelings either way. The on keyword makes a lot of sense to me, since I think of applying a mixin as mixing it "on" to the super class, but thinking of it in terms of what the mixin "requires" of the superclass also makes sense.

@lrhn might have more thoughts. It's something I'd definitely be interested in hearing from other folks about.

[Hixie]

Just to clarify, I definitely like the "mixin" keyword, and I think it's a good idea, for all the reasons given above. I'm greatly concerned about yet another breaking change, however. One option would be to support both but have a lint that discourages the old style.

[Hixie]

(To handle the compiler issue, one option would be to have a compiler mode that just disables the legacy syntax for codebases that know they don't use it.)

[munificent]

The on keyword makes a lot of sense to me, since I think of applying a mixin as mixing it "on" to the super class

My impression from talking about and explaining mixins to lots of users is that almost none of them actually internalize the Bracha-ian notion that a mixin is a function that is applied to a superclass, so "on" probably doesn't illuminate much. Also "on" to me reads like a thing it does not a thing it needs.

I'm greatly concerned about yet another breaking change, however. One option would be to support both but have a lint that discourages the old style.

The doc does state:

It expects to deprecate and remove the ability to derive a mixin from a class declaration, but doesn't require it.

My expectation is that we would add the new syntax, only allow super calls in mixins that use the new syntax but also support the old syntax and semantics for regular non-super-calling mixins. The set of mixins that use super calls is vanishingly small. I wouldn't be entirely surprised if you were literally the author of a majority of them. :)

[leafpetersen]

Just to be sure that we're on the same page, we're not intending to remove support for using classes as non super-mixins , at least yet. We'd like to do so in the future, but that would depend on driving usage low enough, and even then... well, we'll see.

For super-mixins, there is no existing web code that uses them (they're not implemented), and the feature is not available even on the VM without turning on the experimental flag. So it's not breaking for direct consumers of the Dart SDK.

Unfortunately for us, of course, the flutter tools ship with the experimental flag turned on. So this will be a breaking change for flutter. This is actually true whether we introduce new syntax or not: see below for an example of two bad uses of super-mixins that I really, really, really want to disallow, whether we use new syntax or old. But it's certainly true that keeping the old syntax would make this less breaking.

This change is primarily breaking for library writers though. If you wrote the super-mixin correctly, then most of the time uses won't need to change. It's only the definition of super-mixins that will need to change. So most flutter users shouldn't even know this has happened. If their code worked before, it should continue working. The cases that they will be broken are:

• if they wrote their own super-mixins (need to migrate)
• they're using the mixin in an invalid way, but that currently works (need to migrate, would probably need to migrate even if we keep the old syntax).

My hope is that if we get out ahead of this with communication, this can be a minimally disruptive change to flutter users (and to the flutter framework). WDYT?

// This program passes the static analysis, and runs in the dart VM (throwing an error on the missing super call)
class A {
  void bar() {}
  void foo() {}
}

class B {
  void bar() {}
  void foo() {}
}


abstract class M extends A implements B {
  void baz() {
    super.bar();
    super.foo();
  }
}

// Doesn't provide an `A` to `M`.
class C extends B with M {
}

abstract class AbsA implements A {
  void bar();
  void foo();
}

// Doesn't give `M` the required super methods
abstract class D extends AbsA with M {
}


class E extends D {
  void bar() {}
  void foo() {}
}

void main() {
  C().baz();
  E().baz(); // Runtime error
}

[leafpetersen]

Maybe we could create a survey that presents people with a snippet of code

I'd be very happy to see data on this, with the caveat that for lots of reasons we need to move fast on this (not least of which is that the longer we wait, the bigger the chance that we break people). For small syntax changes like this, I think it would be reasonable for us to implement the feature simultaneously with doing surveys, and then just change the keyword if based on data before shipping to users. We'd need to get going on data gathering ASAP though.

Could we instrument the analyzer to catch such cases, and anonymously report them? Flutter in particular has an analytics mechanism we could reuse for this.

I would love to start getting analytics data about language and core library feature usage. Even if it's not feasible in time for this feature, it would be awesome if we could start getting this set up for future use. Not sure what would be involved - I don't think it would be technically hard in the analyzer, but there are lots of tricky issues. cc @devoncarew @bwilkerson

[leafpetersen]

I split off a new issue for discussion of the syntax choice for superclass constraints: #9 .

[bwilkerson]

... getting analytics data about language and core library feature usage.

I would also love to have that data available. There are two issues I can think of:

1. Deciding how to allow users to opt out if they don't want us collecting the data from them.
2. Deciding what data to collect and when to compute it. (There's a danger of skewing the numbers if we count some code bases too frequently.)

[a-siva]

For super-mixins, there is no existing web code that uses them (they're not implemented), and the feature is not available even on the VM without turning on the experimental flag. So it's not breaking for direct consumers of the Dart SDK.

Unfortunately for us, of course, the flutter tools ship with the experimental flag turned on. So this will be a breaking change for flutter. This is actually true whether we introduce new syntax or not: see below for an example of two bad uses of super-mixins that I really, really, really want to disallow, whether we use new syntax or old. But it's certainly true that keeping the old syntax would make this less breaking.

Which experimental flag in the VM are you referring to?

[devoncarew]

start getting analytics data about language and core library feature usage

You could probably get pretty far just analyzing the top n Dart repos on github.

Which experimental flag in the VM are you referring to?

For the analyzer at least, this is enabled via:

analyzer:
  language:
    enableSuperMixins: true

We can get data on how often this is enabled, but I expect it's a very small number of users.

[leafpetersen]

Which experimental flag in the VM are you referring to?

Hmm, is this on by default in the VM? In any case, it's not on by default in the analyzer, and not supported by any web implementation.

[leafpetersen]

I split off an issue here for discussion of the breaking change implications.

[lrhn]

Just to be precise: The super-mixins/enableSuperMixins flag is only supported for backwards compatibility for Flutter. It's not an experimental flag, it's a pre-deprecated compatibility flag for specific existing code. The moment Flutter no longer uses class-based super-mixins, the flag goes away without any further warning.

[eernstg]

We discussed the superclass constraints a number of times. Just for the record, here are some reasons why I think the current proposal offers a bit too much red tape and a bit too little actual help:

Refactoring from classes may fail

The issue here is that the superclass requirement (the on clause, possibly being renamed to requires) is based on a subtype requirement on the actual superclass, and this means that the material obtained from the actual superclass is subject to a more rigid requirement than that of a normal class. Hence, we may be unable to rewrite an existing subclass to a mixin application.

For example, assume that a class A already provides an implementation of a method foo, and then we wish to use that implementation in a class where that method contributes to the satisfaction of an interface I:

class A {
  int foo() => 42;
}

abstract class I {
  int foo();
}

class B extends A implements I {
  int bar() => foo();
  int foo() => super.foo() + 1;
}

If we make an attempt to express the same thing using a mixin and a mixin application, we hit the superclass constraint:

class A {
  int foo() => 42;
}

abstract class I {
  int foo();
}

mixin M on I {
  int bar() => foo();
  int foo() => super.foo() + 1;
}

class B = A with M; // Error, `A` does not implement `I`.

So the problem is that we cannot create mixins by refactoring existing classes, because a normal class will allow an inherited member to contribute to an implementation requirement, but a mixin application will not.

We could try to use mixin M implements I, but that would preclude super.foo().

This is a problem because mixins are intended to help abstracting away multiple identical class bodies, and this means that they should be similarly powerful. Here, 'powerful' just means 'non-stubborn' because there is no technical problem in using a structural rather than nominal superclass constraint.

A subtype check is not sufficient to ensure correct overriding

It would be useful if checking the superclass constraint were sufficient to ensure that a mixin application succeeds: That is, check that the superclass S implements whatever is in the on clause of the mixin, and then the mixin application will work.

(We already know that the superclass constraint does not include information about which methods are implemented and which ones are not implemented. That is, we may make an attempt to apply a given mixin M to a superclass S, and even though S does satisfy the superclass constraint in the declaration of M, we still hit the error that some method is needed for a superinvocation, but S does not have an implementation. But that's a separate topic, this section is about overriding and about the fact that a mixin application may still fail. So in the example below, the superclass provides an implementation where needed.)

Consider the following example:

abstract class I {
  int foo();
}

class A implements I {
  int foo([int x]) => 42;
}

class M on I {
  int foo() => 43;
}

class B = A with M; // Error, `M.foo` cannot override `A.foo`.

So the problem is that int foo([int x]) cannot be overridden by int foo(), and in general, any non-trivial change to the interface of I which is present in the actual superclass may destroy the ability of any declaration in the mixin to override a declaration in I.

The sound "direction" for the constraint in the case of overriding is the reverse direction of the one that is used to satisfy an implements relationship: If the signature in M can override the signature in I and the signature in I can override the signature in whatever superclass we wish to apply M to (call it S), then we can actually trust the mixin application to have only correct override relationships.

So for overriding, the on constraints ought to work like an implemented by rather than like an implements constraints.

What can we do?

One possible approach is to say that we specify both an overriding interface (overrides A, B) and require the inverse implements relation for the superclass relative to those classes (that's going to be structural, or it would be far to rigid); and a superinvocation interface (super I.foo, J.bar ...), implying that they must be implemented and they must be callable as specified in the given interface; and some regular implements relations which may be used when checking invocations on this (but for which we do not require anything from the superclass, those methods may also be provided by subclasses).

That's really heavy, though.

We could also give up on checking superclass overridings as such and only require that superinvocations actually hit an implementation with a suitable signature. This means that we would specify only the methods m(...) for which we do want to call super.m(...), and we can do that using things like requires A.foo, B.bar.

Finally, we could just drop the on/requires clause entirely, insist that superinvocations must be correct as self-sends (that is, we must call something from the signatures of the implements clause, correctly), and then we check implementedness for superinvocations and overridings from scratch at each mixin application.

[lrhn]

The immediate refactoring of B would, as I see it, be:

mixin M on A implements I {
  int bar() => foo();
  int foo() => super.foo() + 1;
}
class B = A with M;

This works and behaves exactly like the original declaration of B, so I'm not sure I see the refactoring issue.

It's true that a sub-type check is not sufficient to statically guarantee that a super-class implementation matches its interface because classes can be abstract and not implement their own interface.
That's why we also require that any member that is super-invoked must also implement the class interface.
That doesn't help with the issue you mention - the super-class being more specific than the interface, and therefore not overridable. We will catch the erroneous override (it's an invalid override independently of whether it comes from a mixin or not) the same way as we catch all other overrides, so I don't think this is a new issue or necessarily mixin related. The mixin application just creates an invalid class, just as if you had written the same class directly.

The current approach is a trade-off between lightness and static guarantees. We don't want something heavier. That includes specifying the actual methods being super-invoked.

Dropping the requirements clause also removes a very important part of documentation. The class is intended to mix-in on a specific super-class, but if you can't specify that, then you don't get any local errors, only remote errors at the application points, and people can start using it on classes it's not intended for.

All in all, I think the current design is a good trade-off.

[eernstg]

The immediate refactoring of B looks good, but it isn't generally applicable; mixin M on A implements I can only be used if M is written in a location where A is known, and only if M is only applied to A and/or subtypes thereof. But if you want to reuse M with different superclasses (A1 ... Ak) that aren't organized such that one of them Aj is a supertype of all the others, then that rewrite won't work.

We will catch the erroneous override .. the same way as we catch all other overrides,

My point is that if we want to specify a superclass interface then the purpose of that superclass interface should be that it creates an encapsulated abstraction: We check a given actual superclass S against the superclass interface of the mixin, and if S satisfies the constraints then mixin application will succeed. Just like all other interfaces.

However, if we insist that actual superclasses must be subtypes of a specified set of types then we are not helping developers: The overrides will succeed if the actual superclass has exactly the specified method signatures; if the actual superclass has more specific method signatures then it is likely to fail (as in my example); and if the actual superclass has less specific method signatures (that is, we have the inverse override relation from the on signatures to the actual superclass signatures) then the overrides would actually be guaranteed to succeed, but that case is prevented a priori because we require subtypes.

In other words: For overriding, we prevent a lot of cases that would work, we allow one case that will work, and we allow a lot of cases that won't work.

To me, that sounds more like red tape than it sounds like a helpful mechanism.

I still think it would be more pragmatically useful to specify an implements clause which defines the signatures that the mixin can use for self-sends, and a requires A.foo, B.bar clause which specifies which implemented signatures the superclass must have, such that they can be subject to superinvocations (that we can check statically), and then we simply don't specify anything that pertains to the override relationships for each declared method in the mixin, which also means that we don't have to worry about the inverse override relation being the correct one to check for, etc.

[leafpetersen]

It's certainly annoying that we don't get encapsulation of the override errors, but as @lrhn points out, we've already lost that in Dart. When you override, you have to check against all implemented interfaces, and for a mixin the override "happens" at the application point.

We could drop the on clause as you say and simply list the methods required, but I'm not sure how we recover the existing patterns that we have in code which in part rely on mixin inference from the superclass constraints.

This design seems to match up pretty well with the few uses we have in practice.

[leafpetersen]

I've added a spec for super mixin type argument inference adapted to this syntax and opened an issue for discussion here. The proposal is here.

[johnniwinther]

With the current proposal we allow super calls that are statically known to fail.

Consider this

abstract class I {
  foo([a]);
}
class A {
  foo() => 42;
}
abstract class B extends A implements I {}
mixin M on I {
   // This will fail because B doesn't have a concrete implementation of I.foo.
   foo([a]) => super.foo(a);
}
class C = B with M; // No compile-time error since B (promises to) implements I.

main() => new C().foo(0); // Results in a super-noSuchMethod error at runtime.

[eernstg]

With the current proposal we allow super calls that are statically known to fail.

I agree that mixin declarations have no checks for concreteness on the methods invoked in superinvocations (so the static checks on M will succeed because the superclass is assumed, and will be required, to implement I, and I declares a foo with a signature which admits the given call). Also, it won't help to check that the actual superclass B implements I: It does, and the method is still unimplemented.

But the proposal says that

it's a compile-time error to [..] apply a mixin containing
super-invocations to a class that doesn't have a concrete implementation of the
super-invoked members compatible with the super-constraint interface.

so I believe that we would actually catch that one at compile-time with the current proposal: B must have a concrete declaration of foo compatible with the abstract declaration foo([a]) from I, and it doesn't (taking 'compatible with' to mean that this concrete declaration fulfills the requirements on foo in a check that verifies B implements I).

[johnniwinther]

Good. I must have missed the mentioning of 'concrete implementation'.

[natebosch]

@rmacnak-google - are the dart:mirrors breaking change concerns from dart-lang/sdk#30342 something that needs to be discussed on this proposal?

[rmacnak-google]

Yes, the language team is responsible for making corresponding changes to the mirror API for every language change.

[yjbanov]

Are there any details on the dart:mirrors breakage? I thought part of the proposal was that super-mixins will have the same behavior unless you opt into the new mixin syntax.

[leafpetersen]

I thought part of the proposal was that super-mixins will have the same behavior unless you opt into the new mixin syntax.

The old super-mixins are going away. Old non-super mixins will continue to be supported.

[leafpetersen]

Yes, the language team is responsible for making corresponding changes to the mirror API for every language change.

This is news to the language team. :) I think it does make sense for dart:mirrors to be on the list of implementations against which we should file issues when we make language changes though. I'll add something to the tracking issue and the template.

[mit-mit]

Closing, this was included in Dart 2.1