Skip to content
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

base class modifier loophole #2451

Closed
stereotype441 opened this issue Sep 1, 2022 · 8 comments
Closed

base class modifier loophole #2451

stereotype441 opened this issue Sep 1, 2022 · 8 comments
Labels
patterns Issues related to pattern matching.

Comments

@stereotype441
Copy link
Member

Regarding the base class modifier defined here, it seems to me that the purpose of marking class A as a "base" class is to ensure that any instance of a class that subclasses A in the program is guaranteed to have A in its superclass chain. (This, for example, makes it safe to put private members in A, and access them via a target other than this).

But as I understand the current spec proposal, there's a loophole. If my library contains:

abstract base class A {
  _foo() {}
}

Then what's to stop some other library from doing this?

abstract class B extends A {}
class C implements B {}

To close this loophole I think we should say that if a class A in library L has the base class modifier, then any subclass of A outside of L must have A somewhere in its supertype chain.

@stereotype441 stereotype441 added the patterns Issues related to pattern matching. label Sep 1, 2022
@stereotype441
Copy link
Member Author

Note: it's tempting to fix this proposal by instead saying that if a class C extends a class A, and A has the base modifier, then C also has the base modifier (whether it's specified explicitly or not). But that doesn't close the loophole, because restrictions are ignored inside a library, so the problematic example above would be accepted.

@stereotype441
Copy link
Member Author

@lrhn
Copy link
Member

lrhn commented Sep 1, 2022

Prohibition against implementation would need to be inherited, otherwise it doesn't work.

Which means it's not a property of a class, it's a property of an inheritance tree, so the "base class" name doesn't really match.

That can be implemented either by implicitly inheriting the base-ness, or by requiring any subclass to declare itself as base as well.

@stereotype441
Copy link
Member Author

That can be implemented either by implicitly inheriting the base-ness, or by requiring any subclass to declare itself as base as well.

That's not sufficient, because we allow you to violate the prohibition within the same library. Which means that if we did it by requiring the subclass to declare itself as base, or implicitly inheriting the base-ness, this would be allowed:

// lib1.dart
abstract base class A {
  _foo() {}
}
// lib2.dart
import 'lib1.dart';

abstract base class B extends A {} // allowed because also "base"
class C implements B {} // allowed because same library as B

@lrhn
Copy link
Member

lrhn commented Sep 2, 2022

We could disallow the C class by saying that it implements the base A class from another library, and therefore cannot be allowed. You can still implement your own base classes, as long as they're only based from your library.

We'd probably have to do, at least, that. An implements B is an implicit implements A, and you're not allowed to implement A.

So, it's not that subclasses are implicitly base classes, no need for inheriting the trait, they don't have to be (but can be independently), it's that implementing them also implements the base class A interface, which they can't outside of the library declaring A.

@lrhn
Copy link
Member

lrhn commented Sep 7, 2022

Thinking about this more, the goal of the base marker is to ensure that all classes implementing the type also inherits the implementation.
The definition should follow that goal.

Primitively, it could be something like:

If the declaration of a class C is marked as base, then

  • any class which implements C must also extend C (have C in its superclass chain).
  • any mixin which implements C must also have an on-type which implements C.

If the declaration of a mixin M is marked as base, then

  • any class which implements M must also extend a class which mixes in M (have an application of M in its superclass chain).
  • any mixin which implements M must also have an on-type which implements M.

Likely well have the usual caveats of allowing subclasses in the same library to break that requirement. However, if those classes are public, we must decide on whether extending those is sufficient to satisfy extending C.
Let's say we allow that, which means the rule is then that to implement the interface, you must inherit implementation of the interface from some code in the same library as the interface declaration, not necessarily the declaring class/mixin itself

Then the rules become:

If the declaration of a class or mixin, D, is marked as base, then any other class which implements D (has the interface of D, or a type instantiation of D if generic, as a declared or inherited superinterface) must either

  • be declared in the same library as D, or
  • extend a class which implements D, or
  • be a mixin application of a mixin which implements D.

If the declaration of a class or mixin, D, is marked as base, then any other mixin which implements D must either

  • be declared in the same library as D, or
  • have an on type which implements D.

Together these rules transitively guarantee that any object implementing D has inherited code from some class or mixin declared in the same library as D.

The base is not inherited, so you can make subclasses, and extend them. You can't implement the subclass interface without inheriting implementation from another class/mixin which also implements D.

We may want to restrict the requirement to concrete classes, so you can define abstract class Foo implements D { ... more ...} and not be required to extend D, not until you try to implement Foo. But that means we can't use transitive rules like above. If we allow Foo here, then extends Foo technically satisfies the requirements.

A more direct version:

Any concrete class declaration which has D as a superinterface, must have either a class implementing D, or an application of a mixin implementing D, which was declared in the same library as D, in its superclass chain.

The restriction on mixins (which are inherently abstract) was there to help you not create a mixin which won't work when applied. We can retain that restriction.

@stereotype441
Copy link
Member Author

@lrhn I'm fine with any of these variations

@munificent
Copy link
Member

I'm going to go ahead and close this out because the newer proposal that supersedes the old base/interface one closes this loophole.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patterns Issues related to pattern matching.
Projects
None yet
Development

No branches or pull requests

3 participants