Skip to content

Class modifiers for anonymous mixin applications #2830

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
lrhn opened this issue Feb 9, 2023 · 7 comments
Closed

Class modifiers for anonymous mixin applications #2830

lrhn opened this issue Feb 9, 2023 · 7 comments
Labels
class-modifiers Issues related to "base", "final", "interface", and "mixin" modifiers on classes and mixins.

Comments

@lrhn
Copy link
Member

lrhn commented Feb 9, 2023

An anonymous mixin application class is the result of a mixin application that isn't named by a declaration.
It's every mixin application class other than the final resulting class of a class C = S with M1, ..., Mn; declaration, where the mixin application of Mn to S with M1,...,M{n-1} is the class named C.

Proposed semantics

  • Let A be an anonymous mixin application class with superclass S and mixin M.
  • If either of S or M is declared as sealed (and therefore necessarily in the same library),
    • then A is also marked sealed (which implies being abstract).
  • Otherwise A is marked abstract (like all anonymous mixin application classes are today), and
    • If either of S or M is declared final (and therefore necessarily in the same library)
      • then A is also marked final.
    • Otherwise, if either S or M is declared as base,
      • then A is marked base.

Background and motivation.

TL;DR: This adds modifiers to the anonymous mixin application classes which satisfy and propagate the modifiers of the superclass and mixin, which allows us to completely ignore that ananymous

We have rules for required class modifiers for subclasses that make sense for declared subclasses, but we haven't addressed anonymous mixin application classes which has no declaration of their own.
We should make sure the classes of such mixin applications satisfy the rules we have for classes, and that they behave usefully where necessary, in particular around exhaustiveness, since users can't attach modifiers to them directly.

Examples of anonymous mixin declaration classes:

class C extends S with M1, M2 { ... } // S&M1 and S&M1&M2 are anonymous superclasses of C.
class D = S with M1, M2; // superclass of D is anonymous application class S&M1.

These declarations introduce five new classes:

  • C:S&M1 : S with M1 from the C declaration
  • C:S&M1&M2: C:S&M1 with M2 from the C declaration.
  • C itself, has C:S&M1&M2 as superclass, aka the "mixin application" C:S&M1&M2 with { .... }
  • D:S&M1: S with M1 from the D declaration.
  • D itself which is the named mixin application class D:S&M1 with M2.

The only non-mixin-application class is C.
The only non-anonymous mixin application class is the one named D.

Being anonymous, an anonymous mixin application cannot be referenced from anywhere other than the context where it is written. The only role of an anonymous mixin application is to be the superclass of one other class, which is always in the same library as the mixin application itself.
(We let compilers combine/canonicalize equivalent mixin applications, so they'd only have one S&M1 class. That's not technically in the language specification, but nobody cares, and you can't tell without using dart:mirrors, at least since we seem to actively avoid that class becoming the LUB of two subclasses.)

The current semantics make every anonymous mixin application be implicitly abstract, and we preserve that (it's implied by sealed).

We extend that to apply other modifiers as necessary and/or useful.

We mark the mixin application final if either superclass is final (and not sealed),
and base if either superclass is base (and not final or sealed).
The marker is just there to satisfy the "every subclass of a base or final declaration must itself be base, final or sealed" rule, and to not implicitly reopen a super-class (to make the expected "implicit-reopen" lint not have to special-case anonymous mixin application classes).

The propagation of sealed is desirable because of how exhaustiveness works.
If you declare:

sealed class MyBase {}
class A extends MyBase {}
class B extends MyBase with _Helper {}

you'd probably expect A and B to exhaust MyBase.
However, if MyBase with _Helper is not sealed, exhausting MyBase won't necessarily be taken as exhausting MyBase with _Helper, in which case case A _ => ..., case B _ => ... wont be exhausting MyBase.
When MyBase with _Helper is sealed, and it has only one subclass (B), exhausting B will exhaust MyBase with _Helper too, using the same algorithm as for sealed classes with other explicitly sealed direct subclasses.

We also propagate sealed from the mixin. It's probably more speculative, but take:

sealed mixin M { ... }
class M1 extends A with M {}
class M2 extends B with M {}

This declares a sealed type with precisely two declared subclasses in the same library. Again we'd expect M1 and M2 to exhaust M. And again, if we don't treat A with M, the actual immediate subclass of M, as sealed, exhausting M1 won't exhaust A with M, which means that there is a subclass of M which the algorithm may have to assume can have other subclasses.

We could also just say that the exhaustiveness checking algorithm on sealed classes bypasses anonymous mixin application classes and go directly to the non-anonymous subclass.

That's effectively what we do by saying that the anonymous class is sealed, but with the added benefit that it allows us to avoid special-casing and also desugar composite mixin applications while preserving their behavior.

We can, and sometimes do, treat anonymous mixin applications as if they were declared by freshly-private-named mixin application declarations of precisely one mixin, so the above would be:

abstract class _$C_S_M1 = S with M1;
abstract class _$C_S_M1_M2 = _$C_S_M1 with M2;
class C extends _$C_S_M1_M2 {}
abstract class _$D_S_M1 = S with M1;
class D = _$D_S_M1 with M2; // superclass of D is anonymous application class S&M1.

All mixin applications are then of the form class C = S with M implements I...I;.
We prefer to be able to continue doing that rewrite, and applying the abstract/base/final/sealed modifiers as described above, rather than skipping the anonymous classes internally in the algorithm, ensures that rewriting the mixin applications into their explicit form preserves semantics precisely.

@leafpetersen @kallentu @munificent @eernstg @stereotype441 @natebosch @jakemac53

EDIT: Made mixin application classes be only base if sufficient, to avoid reopening.

@lrhn lrhn added the class-modifiers Issues related to "base", "final", "interface", and "mixin" modifiers on classes and mixins. label Feb 9, 2023
@eernstg
Copy link
Member

eernstg commented Feb 9, 2023

Nice!

copybara-service bot pushed a commit to dart-lang/sdk that referenced this issue Feb 22, 2023
… on clause

Implements the proposed spec for dart-lang/language#2830 and along the way, fixes another change with anonymous mixins erroring in on-clauses.

Change-Id: I15ed7370f9099d9de89d80f5844db76f58fbd216
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/284481
Reviewed-by: Johnni Winther <[email protected]>
Commit-Queue: Kallen Tu <[email protected]>
@lrhn
Copy link
Member Author

lrhn commented Feb 27, 2023

I suggest we just assume this definition to be the desired behavior for now. I'll write it into the specification when I find some time.

We need something like this, to ensure that base class C extends S with M {} where S is base does not report an error when S&M is not also base. We need these for the same reasons the current specification adds abstract to all anonymous mixin application classes, because it's safe and simple, and makes you not have to worry about the intermediate classes.

The goal is that users can safely ignore the intermediate classes when thinking about their class modifiers.

If there are any missing edge cases, we'll handle them from here.

@eernstg
Copy link
Member

eernstg commented Feb 27, 2023

Sounds good!

Implicitly adding the required class modifiers to mixin applications is probably good for the comprehensibility of the
class modifier related rules and their implementation, and we might even want to mention those class modifiers in error messages involving any such anonymous class.

One more thing comes to mind, though: In S with M (considered as a general notation that includes every mixin application, as in the initial comment), if M has the modifier interface then it seems reasonable to implicitly add that modifier to S with M as well.

// Library 'a.dart'.

interface mixin M {
  void foo() { print('Should not be inherited outside this library'); }
}

class A extends Object with M {}

// Library 'b.dart'.
import 'a.dart';

class B extends A {} // Inherits `M.foo`, which shouldn't happen.

void main() {
  B().foo();
}

If we do not consider Object with M to be interface then A extends a class which isn't interface, so there's no reopen lint on A, but Object with M mixes in M but isn't marked interface, so Object with M should get a reopen lint. That's not particularly actionable.

If we do consider Object with M to be interface then we get a reopen lint on A, and it can be fixed by adding interface to A, and then B will be a compile-time error.

So the point is that we'd want to consider A to be the culprit, and we want to consider the superclass of A to be Object with M, and this will work if we propagate interface into mixin applications.

@lrhn
Copy link
Member Author

lrhn commented Feb 27, 2023

I can see the point.

Assume:

interface class I {}
class C extends I {}

If the point if interface is that you cannot inherit implementation from I (outside of the library), then you should also not be able to inherit that implementation through C. We'd probably show a "reopen" warning on C if it's not marked interface or worse.

The same applies to mixin applications which inherit the implementation.
A class C extends Object with M {} should usually have the same behavior as class C extends M {}.

That does mean that the logic becomes:

  • If either S or M is sealed, then the AMAC (anonymous mixin application class) is also sealed.
  • Otherwise the AMAC is abstract, and:
    • If either S or M is final, then the AMAC is final.
    • Otherwise if one of S and M is base and the other is interface, then the AMAC is final
    • Otherwise if either is base then the AMACA is base.
    • Otherwise if either is interface then the AMACA is interface.

More complicated than before, but it's still just a simple partial order LUB.

@eernstg
Copy link
Member

eernstg commented Mar 12, 2023

I think it may be a bit easier to check the rules if they are expressed without 'otherwise'.

S & M in the following stands for S with M or for S1 with M1, ... Mk, M for some k > 0. In other words, S & M is a mixin application, and it includes the case where S is itself a mixin application.

  • S with M is final if S is final; if M is final; or if one of S and M is base and the other is interface.
  • S with M is base if S and M are both base; or if one of them is base and the other has no modifier.
  • S with M is interface if both S and M are interface; or if one of them is interface and the other has no modifier.

@leafpetersen
Copy link
Member

@lrhn @eernstg Can this be closed out?

@lrhn
Copy link
Member Author

lrhn commented May 8, 2023

Yes. The specification changed to only be defined in terms of declarations, so the implicit intermediate classes introduced here do not need to have any modifiers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
class-modifiers Issues related to "base", "final", "interface", and "mixin" modifiers on classes and mixins.
Projects
None yet
Development

No branches or pull requests

3 participants