[Capability Modifiers] Specify mixin interactions with older language versions.

[leafpetersen]

The current version of the capability modifiers proposal says this about interactions with older language versions:

It is a compile-time error to:

...

 Mix in a class not marked `mixin` outside of the library where it is
    declared, unless the class declaration is in a library whose language
    version is older than the version this feature ships in.

I believe that the intent is that after this change, assuming it is shipped in language version N:

• It will be an error to use a class as a mixin when the class is declared in a library from language version >=N, regardless of the language version of the library in which the mixin application is performed
• It will not be an error to use a class as a mixin when the class is declared in a library from language version <N, regardless of the language version of the library in which the mixin application is performed.

This means that if C is a class not declared with mixin, and C is mixed in then the behavior looks as follows (assuming that the declaration and use library are distinct):

Language version of declaration library	Language version of the use library	Outcome
<N	<N	Ok
<N	>= N	Ok
>= N	<N	Error
>= N	>= N	Error

Is this the behavior we want?

• The first and last rows are a given.
• The second row implies that an "opted in" library can continue using classes from old libraries as mixins without them being declared as such. This is probably the right decision, since a consumer of an old library can't force the old library to make a choice yet, and the old library may presumably choose to allow the class to be mixed in.
• The third row implies that an "opted out" library may not continue using classes from new libraries as mixins. This is probably the most contentious bit.
  • It has the advantage that it allows compilers to assume that classes in opted in libraries will never be mixed in.
  • It has the disadvantage that it will naturally lead package authors to make inadvertent breaking changes when they upgrade to the new language version and neglect to mark all of their classes as mixin classes.

On balance, I think I'm comfortable with this choice. If we're all in agreement on this, we should make this more explicit in the specification and be sure that these combinations are tested.

cc @munificent @eernstg @lrhn @kallentu @stereotype441 @jakemac53 @natebosch @mit-mit

[leafpetersen]

cc @srawlins who raised this question

[natebosch]

yes, I think this is the behavior we want.

• it will naturally lead package authors to make inadvertent breaking changes when they upgrade to the new language version and neglect to mark all of their classes as mixin classes.

The breaking change effectively happens either way. I think this decision mainly impacts when the breaking impact will be discovered, and what mitigation steps might be available to the impacted users.

From the standpoint of google3 I think it's better that the failure happens as early as possible. We'd like a signal for someone authoring a migration that a downstream consumer is using their class as a mixin, even if the consumer isn't migrated.

@mit-mit might have a different preference for the pub side. I still lean towards making the breakage visible earlier. No matter what we decide, a package that migrates after its consumers is going to potentially break them on publish. I don't see much value, from the package publisher perspective, in having a consumer pool which is differentiated by when they might notice that I missed a mixin.

[leafpetersen]

The breaking change effectively happens either way. I think this decision mainly impacts when the breaking impact will be discovered, and what mitigation steps might be available to the impacted users.

This is a good point. It's probably much more useful to be able to immediately see the effect of your change, rather than have it roll in as downstream consumers upgrade their own language versions.

[lrhn]

I also think this is the behavior we want.

If a class was not intended as a mixin, and was used as it anyway, that use can break at any time. And as @munificent says, it'll happen at the latest when the using library updates their language version. Breaking early is preferable.

If a class is intended as a mixin, hopefully the declaring package has tests which performs the mixin, and they'll notice the need for the mixin keyword immediately when they update the language version.
I don't expect to see anyone forget a mixin keyword on a class intended as a mixin.

[eernstg]

I like the fact that the table here can be summarized as follows: The declaration decides.

In more detail: We consider a legacy class declaration to have an implicit mixin modifier. Now, it's OK to mix in a mixin class and it's an error to mix in a class without the mixin modifier. In other words, it doesn't matter where the mixin application occurs, the decision is made by the declaration, and it can be detected by looking at the class declaration and its language version alone.

This means that legacy code can break when updating some of its dependencies (in this sense it differs from null safety). However, if the owners of a widely used library wish to avoid this breakage then they can add mixin to every class that permits this modifier.

Would it be useful to have tool support for finding those class declarations?

[leafpetersen]

I also think this is the behavior we want.

If a class was not intended as a mixin, and was used as it anyway, that use can break at any time. And as @munificent says, it'll happen at the latest when the using library updates their language version. Breaking early is preferable.

Note that this means that we will need to add mixin to every class which can be used as a mixin in the core libraries before we ship Dart 3 unless we exempt the core libraries from this behavior. Or unless the class is clearly documented as not being intended for use as a mixin per here.

[lrhn]

I'd prefer to say that a class is not intended as a mixin unless it's documented as such.
That means {List,Set,Map}Mixin, and nothing else.

If we really want to be absolutely sure not to break existing code which mixes in other classes, which are not documented as being intended as mixins, then I'd prefer to allow legacy libraries to mix in non-mixin classes from the platform libraries, but not from anywhere else.

[leafpetersen]

Technically, our breaking change policy puts the burden on us to document things that aren't intended to be mixed in, rather than saying that anything not documented as intended to be mixed in shouldn't be. It feels a little off to violate this, especially without having at least scraped pub to see what people are doing in practice.

I doubt we can actually get these changes in before Dart 3 alpha, which we've promised covers all of the non-language versioned breaking changes. So that leaves allowing older language versions to mixin classes from the SDK core libraries.

[munificent]

If we're all in agreement on this, we should make this more explicit in the specification and be sure that these combinations are tested.

Yup, the behavior you describe is what I had in mind. I'll update the proposal to spell it out more clearly.

Technically, our breaking change policy puts the burden on us to document things that aren't intended to be mixed in, rather than saying that anything not documented as intended to be mixed in shouldn't be.

For what it's worth, "Effective Dart" has long said that you should only mix in classes explicitly documented as supporting it.

I'm happy to defer to the core lib folks to decide which classes they want to put mixin on.

[leafpetersen]

I'm happy to defer to the core lib folks to decide which classes they want to put mixin on.

I think we're saying there is no choice to be made here, unless we make modifications to what I laid out in comment 1. Everything which can be a mixin, must be.

[natebosch]

I doubt we can actually get these changes in before Dart 3 alpha, which we've promised covers all of the non-language versioned breaking changes.

Would it be worth considering faking it for the core libraries in Dart 3 alpha? We could implement a restriction against mixing in classes from the SDK outside a hardcoded list perhaps?

So that leaves allowing older language versions to mixin classes from the SDK core libraries.

If we are special casing the SDK, is it out of the question to special case it the other way, and make mixing in of SDK classes breaking only after changing language version, but mixing in of non-SDK classes breaking regardless of language version?

[leafpetersen]

So that leaves allowing older language versions to mixin classes from the SDK core libraries.

If we are special casing the SDK, is it out of the question to special case it the other way, and make mixing in of SDK classes breaking only after changing language version, but mixing in of non-SDK classes breaking regardless of language version?

I'm not sure what you're asking. What I'm proposing there is:

Language version of declaration library	Language version of the use library	Outcome
non SDK, <N	<N	Ok
non SDK, <N	>= N	Ok
non SDK, >= N	<N	Error
SDK, >= N	<N	Ok
non SDK, >= N	>= N	Error
SDK, >= N	>= N	Error

What is it that you would do differently?

[leafpetersen]

I doubt we can actually get these changes in before Dart 3 alpha, which we've promised covers all of the non-language versioned breaking changes.

Would it be worth considering faking it for the core libraries in Dart 3 alpha? We could implement a restriction against mixing in classes from the SDK outside a hardcoded list perhaps?

Maybe. It's actually not out of the question that the real implementation will be in place by Dart 3 alpha, in which case the mixin part could be ok - I think we could get mixin onto the right classes in time. But I question whether we could triage all of the things that should have other modifiers in time.

[natebosch]

I'm not sure what you're asking. What I'm proposing there is:

Ah yes, that is what I was hoping for.

[lrhn]

I think we're saying there is no choice to be made here, unless we make modifications to what I laid out in comment 1. > Everything which can be a mixin, must be.

We're definitely not going to add mixin to every class that could potentially be used as a mixin with the current semantics. That would be a way to say that they are intended as mixins, which they are not. It would be wrong.

We have the prefer_mixin lint which reports any mixing in of a class, other than those on a short allowed-list.

If doing the modifications mentioned here is the only alternative, we must do that.

Let's reiterate our constraints.

• We must not break existing code, when it's run on a 3.0 SDK.
• We want to allow existing code to be migrated to Dart 3.0 and not lose functionality. (No forced breaking changes. This includes the classes which are currently both extended and mixed in.)
• The classes IterableMixin, ListMixin, SetMixin, MapMixin and StringConversionSinkMixin should be the only classes in the core platform libraries (I don't know about the web libraries) that will be made into mixins. No other class is intended as a mixin. They likely have to keep being usable as classes as well.
• We may want to add class modifiers to platform classes (other than the ones that are already un-implementable). But that is potentially breaking, so see first item. We likely need to allow legacy libraries to ignore those modifiers as well. (I don't expect to get away with making Future final, even if legacy code can ignore it. Some things would be too breaking to the ecosystem, no matter how convenient they would be for us.)

Preserving the ability for a single declaration to be both mixed in and extended can be done in several ways:

• A mixin class declaration.
• Allowing you to extend a mixin declaration with no superinterface declaration. (Allow mixins in "extends" clauses #1942)
• Allowing you to mix in a class declaration with no superclass and generative constructor declaration (aka. what we have now).

The last one is dangerous, because it is too easy to accidentally allow a class to be used as a mixin, and then it becomes a breaking change to add a superclass or generative constructor. That's why we want to get rid of it.

The second one is less dangerous, because extending Object with the mixin was always possible. Adding a super-interface constraint is a breaking change to the mixin, whether it's used with extends, so you won't accidentally just break using it with extends. This is an option, if we want to avoid mixin class declarations.

(I'd prefer to combine IterableMixin and IterableBase into one mixin class IterableBase (or just mixin class Iterable), and let the IterableMixin name be a deprecated alias for IterableBase. The one thing that prevents me from doing that is that IterableBase has a const constructor, and a mixin cannot declare a constructor, and I have no way to get a const default constructor. Is there some way we could do that, say const .... mixin class Foo {} would get a const default constructor? We'll likely need the modifier for "default constructor"-syntax anyway, like const class Foo(int x).... A const class would need to have only final fields, and if they have initializers, those must be constants too, and its default constructor will be const. The trick will work for the other four, where the ...Base class constructor is not const.)

[leafpetersen]

My understanding of where we left this (in combination with the resolution of #2725 ) was that we would do what I suggest above and make it not an error to mixin a normal (non-mixin) class from the 3.N core libraries from pre 3.0 code. In this way, we can choose to put mixin only on classes which we wish to be used as mixins going forward, without adding a new non-language versioned breaking change in 3.0 (after having announced that all breaking changes were in as of January).

The current implementation does not do what I describe above - it makes it an error.

I'd like to propose that we change the implementation to behave as described above before flipping the flag.

Thoughts on this? cc @dart-lang/language-team

[lrhn]

If we allow non-mixin classes from the platform SDK to be mixed in by pre-feature libraries,
what should the conditions be for which classes can be used that way?

• Those which satisfy the pre-feature requirements (no constructor, Object as superclass), or
• those which satisfy the post-feature requirements for being a mixin class (no non-trivial generative constructor, Object as superclass, not declared by mixin application declaration)?

The latter allows us to add some constructors without breaking legacy code, and I think the only use of mixin application declarations (class C = S with M;) that could possibly be used as mixins, are classes that will be mixin classes anyway.

[munificent]

The latter allows us to add some constructors without breaking legacy code

Then the latter sounds good to me.

[lrhn]

And we definitely should issue warnings where someone is mixing in a non-mixin class (or extending a non-base/open class, etc.) from the platform libraries.

[leafpetersen]

Tests out for the proposed semantics out for review here: https://dart-review.googlesource.com/c/sdk/+/287665 . If this encapsulates our desired behavior I will update the spec to reflect this.

[lrhn]

I already added words to the #2889 PR.
Check if it makes sense.

declarations in pre-feature libraries can ignore some
base, interface and final modifiers on some declarations
in platform libraries, and can mix in non-mixin classes from platform libraries,
as long as such a class has Object as superclass and declares no constructors.

(The "some" comes from not being able to ignore it on, e.g., int.)

[munificent]

The tests have landed. Can this be closed now?