Still a backdoor in "base" modifier #2755
Comments
munificent
added
the
class-modifiers
|
Alternatively, we could just say that once you've removed the "implements" capability, it can't be re-added, even within the same library. So we'd remove the "outside of the library where it is declared" part from the implements rule and make it:
|
I'd prefer this approach: If we allow the property to be violated in the same library and then somehow enforce a suitable constraint in other libraries, a developer would still need to be aware of the non-subclass-subtype in the same library in order to understand which guarantees they actually have, and the source code doesn't give the developer any hints about this fact. |
|
I agree. As I've understood the current specification's intent:
The "and cannot remove it" is the crucial part here, and where we cannot just look at the local superclass for guidance. If a class or mixin is marked If a class or mixin is marked (If it's On the other hand, just because one of your transitive superclasses is So: // lib_a.dart
base class Foo {
int get _id => ...;
void same(Foo other) => _id == (other is Bar ? other.id : other._id);
}
@reopen
class Bar extends Foo {
int get id => _id;
}
// lib_b.dart
import "lib_a.dart";
class Baz extends Bar {} // open and implementable.
class Qux implements Bar, Baz {
int get id => 42;
} // All is well.So, we can't just enforce the restrictions of every super-interface, we need to find the boundary in the super-class/interface tree between this library and other libraries. If what you do is valid against the first class from another library in the super-tree paths, then it's fine. I tried doing something in #2730. Possibly over-engineered. I don't think the interface restrictions need to be that complicated. Thinking of it again, we might be able to model this with a single flag, let's call it a synthetic
That is, if you create a subtype of a type from another library which doesn't supply a public interface, or of a local subtype of such a type, you also cannot expose an interface, even inside the same library. (Can the On the other hand, I think disallowing re-adding I don't think it'd be a big problem in practice to have the restriction. The use-cases for it aren't going to be plentiful, and there'll probably be workarounds. But it's inconsistent (why can you I don't expect to be able to predict all use-cases. That's why I don't want to prevent specific combinations, even if they go counter to other specific use-cases that we can imagine. I'm satisfied with giving users tools that provide local restrictions on a single declaration (as it applies to any other library looking at it), and letting the total behavior of a library emerge from the combination of modifiers on all the classes. The modifiers is a tool to build guarantees, but they don't provide global guarantees by themselves. Subclasses in the same library can disprove any guarantee that you might think is implied by the superclass modifier. |
|
Here is an attempt to list some properties that we could have. Let
Based on these properties, Similarly,
We can maintain these properties as follows:
To ensure the soundness of rule (2), we report the following errors:
In other words, "If you don't say |
|
If we allow reopening (aka. having fewer restrictions than a supertype in the same library, which we currently do and I think we should keep doing), we do need to decide what that means when the same superinterface can be reached in more than one way. Take the example: // lib_a.dart
base class Foo {}
@reopen
class Bar extends Foo {}
// lib_b.dart
import "lib_a.dart";
class Baz extends Foo implements Bar {}
class Qux implements Bar, Baz {}Here Is it OK for The On the other hand, the existence of So maybe the rule we really need is that:
That is, if a |
|
The way the proposal works, a library author may choose to craft an invariant about their library based on the modifiers they place on the declarations in it. If a library author sets up any of these invariants, code outside of the library should not be able to break it. That's what I really care about. I'm comfortable with the library author breaking their own invariants. It's their library. They chose to author the invariant in the first place, and they can discard it if they want. For me, the key questions are:
For Of course, the library author may choose to extend their own For I believe what Lasse proposed is correct in that it closes the loopholes. But I can't imagine being able to explain it to a user. I like @stereotype441's general point that complex semantics are OK if they are in service of an intent that users understand. But I don't think even the intent here is as clean and simple as it is in things like type promotion and inference. If a library author writes: base class B {}
class C extends B {}Do they actually intend that another library should be able to do: base class D extends B {}
class E implements C {}
class F implements B, C {}But not: class F implements B {}I feel like we're going too far out on a limb with the design here. It might be the right design, but it's complex and I don't have concrete user data I can point to justify it. In the absence of empirical evidence pushing us towards a complex design, my inclination is to pick a simpler, more restrictive rule that is easier to understand and explain while still closing the loopholes. We can always loosen it later if users want. But we can't go the other way if we ship a complex, confusing, but permissive rule. I don't usually like to start with something restrictive, but in this case, it's a restriction the user is opting in to. If they find the rules around How does this sound: It is a compile-time error to:
The changed rules are so that a library can't inherit from its own class while // lib_a.dart
base class A {}
// lib_b.dart
import 'lib_a.dart';
// OK:
base class B extends A {}
// Error according modified rule but not original one:
class C extends B {}
// If the above was allowed, then you could do:
class D implements C {}
// And now you have a class that is a subtype of A but not a subclass.The new rule closes the loophole described in this issue: The rules are somewhat more restrictive. You can still do: base class A {}
base class B extends A {}
class C implements A {}So you can inherit from a base class A {}
class B extends A {}I haven't totally wrapped my head around these proposed rules yet. I feel like I need to write a program to test every possible hierarchy or something. But I think they might work? |
|
The rule change is intended to ensure that all subclasses of a And if you implement any I think it's strict. Maybe too strict, but as you say, we might be able to loosen it in the future. As long as the loosening has to happen inside the same library as the original The rules do not mention So, it's a compile-time error for a class or mixin declaration S in library L to:
The change, to using "has a superinterface" instead of a direct implements clause, allows us to look through In practice, you only need to go through superclasses in the same library, finding the frontier into other libraries, because the moment you see a declaration from another library, it must be consistent with its super-interfaces, and its restriction cannot be ignored in the current library. The actual implementation can still be done step-wise, like the "inherited-base" above. A class and/or mixin declaration S in library L is unimplementable if:
It's a compile-time error if a class or mixin declaration is unimplementable, and is not marked This can be computed per-library, going from super-class to subclass in the order we normally resolve classes. The declaration and use never refers to whether a declaration in another library is unimplementable, because for those we can use the modifiers. Inside a library, we can ignore modifiers, but we can't ignore restrictions inherited from other libraries. Being unimplementable means having such a restriction from anther library. |
|
[Edit Jan 6: Mixin applications are now considered type by type, and Here is a table that shows the constraints we need, as far as I can see. Consider a declaration of a class, mixin, or mixin class named The table shows the requirements around the use of On top of these rules, a declaration carrying (Of course, I'd much prefer that we did not have a Treatment of mixin applicationsWe need to consider the treatment of a mixin application, because a mixin application can appear as the superclass of another class. Hence, the symbol In short, when the superclass written as Constraints on subtypes of interface entitiesThis table is applicable when
Constraints on subtypes of base entitiesThis table is applicable when
Constraints on subtypes of final entitiesThis table is applicable when
Relative to @munificent's rules, I think the following 26 cases aren't errors, but they are errors according to the tables above. If we include the modifier The comments in the libraries whose name is of the form // Library 'n005lib.dart'.
interface class A { // Or `mixin class`.
void foo() {}
}
class B1 extends A {}
base class B2 extends A {}
interface mixin M {
void foo() {}
}
class B3 extends Object with M {} // Or `with A` if mixin class.
base class B4 extends Object with M {} // Or `with A` if mixin class.// Library 'n005.dart'.
import 'n005lib.dart';
class C1 extends B1 {} // Inherits `A.foo`.
base class C2 extends B2 {} // Inherits `A.foo`.
class C3 extends B3 {} // Inherits `M.foo`.
base class C4 extends B4 {} // Inherits `M.foo`.// Library 'n006lib.dart'.
base class A {
void _foo() {}
}
class B1 implements A {}
interface class B2 implements A {}
base mixin M {
void foo() {}
}
class B3 implements M {}
interface class B4 implements M {}
mixin B5 on M {}
interface mixin B6 on M {}// Library 'n006.dart'.
import 'n006lib.dart';
// Not a subclass of any class in 'n006lib.dart'.
/*base*/ class C1 implements B1 {}
/*base*/ class C2 implements B2 {} // Ditto.
/*base*/ class C3 implements B3 {} // Ditto.
/*base*/ class C4 implements B4 {} // Ditto.
/*base*/ class C5 implements B5 {} // Ditto.
/*base*/ class C6 implements B6 {} // Ditto.// Library 'n007lib.dart'.
final class A {
void _foo() {}
void foo() {}
}
class B1 extends A {}
interface class B2 extends A {}
base class B3 extends A {}
final mixin M {
void _foo() {}
void foo() {}
}
class B4 extends Object with M {} // `Object with M` is not final, right?
interface class B5 extends Object with M {} // Ditto.
mixin B6 on M {}
interface mixin B7 on M {}
base mixin B8 on M {}// Library 'n007.dart'.
import 'n007lib.dart';
// Inherits `A.foo`.
/*base*/ class C1 extends B1 {}
// Inherits `A.foo`.
base class C3 extends B3 {}
// Inherits `M.foo`.
/*base*/ class C4a extends B4 {}
// Implements `M`, does not have a `_foo`.
/*base*/ class C4b implements B4 {}
// Implements `M`, does not have a `_foo`.
/*base*/ class C5 implements B5 {}
// Implements `M`, does not have a `_foo`.
/*base*/ class C6a implements B6 {}
// Implements `M`, does not have a `_foo`.
/*base*/ class C6b extends C5 with B6 {}
// Implements `M`, does not have a `_foo`.
/*base*/ class C7 implements B7 {}
// Implements `M`, does not have a `_foo`.
base class C8 extends C5 with B8 {}// Library 'n008lib.dart'.
final class A {
void _foo() {}
void foo() {}
}
class B1 implements A {}
interface class B2 implements A {}
final mixin M {
void _foo() {}
void foo() {}
}
class B3 extends Object with M {} // `Object with M` not final.
interface class B4 extends Object with M {} // Ditto.
mixin B5 on M {}
interface mixin B6 on M {}// Library 'n008.dart'.
import 'n008lib.dart';
// Inherits `A.foo`. Implements `A`, does not have a `_foo`.
/*base*/ class C1a extends B1 {}
// Implements `A`, does not have a `_foo`.
/*base*/ class C1b implements B1 {}
// Implements `A`, does not have a `_foo`.
/*base*/ class C2 implements B2 {}
// Inherits `M.foo`. Implements `M`, does not have a `_foo`.
/*base*/ class C3 extends B3 {}
// Implements `M`, does not have a `_foo`.
/*base*/ class C4 implements B4 {}
// Implements `M`, does not have a `_foo`.
/*base*/ class C5 implements B5 {}
// Implements `M`, does not have a `_foo`.
/*base*/ class C6 implements B6 {} |
|
Not sure I agree on the tables here. In particular, there is no reason to disallow You can declare a mixin which must be applied to a The constraint of the OK, I did my own tables: Dart class modifiers, constraintsAssume we want to make non-implementability (implied by You can reallow extension/mixin-application in a subclass of your own library, if you want to. Direct restrictions from modifiers, inside same library:
Direct restrictions from modifiers, from other library:
Transitive restrictions from superclass modifiers, from other library. Restrictions apply for every superinterface from another library:
Looking at the rightmost columns, it's pretty clear that everything falls into two groups:
If any superclass does not allow Unified definitionA class and/or mixin declaration has a number of flags, representing capabilities, derived from modifiers on the class and flags of superclasses. The flags and their possible values are:
If a class or mixin declaration A refers to a declaration B in the same library, then the flags are inferred as follows:
If A and B are in different libraries, restrictions matter more:
If B has multiple relations to other types A, then the flags are merged, to the most restrictive among the possible outcomes (:x: is more restrictive than :house:, which is again more restrictive than :heavy_check_mark:). Missing combinations require modifiers which are not allowed (a non |
|
We've talked a bunch about various use cases for being able to control a class's capabilities. In particular, how controlling them (if we're careful!) might let library authors ensure certain invariants about how their code is used. But the proposal itself doesn't directly encode those invariants. Partially that's because we aren't sure if we exhaustively know all of the invariants a user might want to and be able to encode. (Also, there are entirely valid software engineering use cases for capability control that don't create rigid statically enforced invariants. Just like you might define an I have a hunch that when it comes to
That sounds like I'm just restating what So here's a stab at defining Can't inheritThe "can't inherit" invariant means "T's code can only be reused within the same library as T". The rule is:
There's no transitive rule because once a subtype has stopped inheriting from T, there's no way to any further subtypes to restart inheriting from it. The chain has been broken. Thus, this one is simple. Must inheritThe strict "must inherit" invariant is "any instance of T inherits from T". A library author can opt in to a looser invariant of "any instance of T inherits from some class in T's library" by choosing to implement a "must-inherit" class from their own library. The rules are:
Both constraintsA library author might want both constraints. This way, they can ensure that every instance of their type was constructed by one of their generative constructors, has the private members they expect, and doesn't have any unexpected method overrides. The only way for it to be true that "every subtype of T inherits from T" and "every subtype of T does not inherit from T" is if there are no subtypes of T, which is what SummaryI think this is similar what Lasse is proposing by defining unimplementable, but my hope was that maybe formulating it like this made it easier to connect the syntax to the intended user goal instead of the invariants emerging (we hope!) from a set of mechanical rules. Note that this doesn't mean that the only use case for these modifiers is these invariants. You might mark a class |
|
I think setting up user-understandable goals is good. The phrasing of the actual semantic rules don't have to follow that (I think phrasing in terms of adding or removing capabilities instead of restrictions is easier to work with, because capabilities saying "can" can't conflict, but "must" rules can). It should be possible to connect the dots from the intent to the specification. (The definition of "implementation library" doesn't account for mixins, so it needs to be extended. A single class declaration can add superclasses with code from more than one library, and it can have - or lack - implementation of more than one interface from the same library). So the intent is:
You can ignore those intents inside the same library. It's still all about "follow the implementation". |
Core Guarantees@munificent, that's a very nice characterization of the core of the desired constraints! Those two invariants ("can't inherit" and "must inherit") are also equivalent, I think, to the things that I was checking in the tables I wrote here: The first one is "no other library inherits implementation declared by this class" (to avoid that others have detailed dependencies on 'our' implementation), and conversely that "every object of type I think it would be nice if it requires a Tables and Holes@lrhn, I think the two rulesets we have are converging (mine, yours). The most obvious difference is that you are using transitive constraints, and I'm only using constraints on the direct superinterface relationship. I'm maintaining the soundness with respect to the provided guarantees by ensuring that every class/mixin declaration preserves the required properties. This could potentially make your rules more flexible than mine, while still maintaining some useful guarantees (because I don't allow any "bad apples" in the middle of a path in the superinterface graph). The other side of the coin is that local rules are simpler (and perhaps more "self-documenting"), because they never rely on searching through the entire superinterface graph. Some situations where your rules allow something that I consider unsafe are the following: // Situation 1, one library L1.
// Table: direct restrictions, same library, row `interface`.
interface class A { void foo() {}}
class B1 extends A {}
base class B2 extends A {}
interface mixin M { void foo() {}}
class B3 extends Object with M {}
base class B4 extends Object with M {}In this situation, a class outside L1 can be a subclass of // Situation 2, one library L2.
// Table: direct restrictions, same library, row `final`.
final class A { void foo() {}}
base class B1 extends A {}
final mixin M { void foo() {}}
base class B2 extends Object with M {}Again, a class outside L2 can be a subclass of (As always, if you want to destroy the guarantees then use Turning to another row, I'm surprised to see that a mixin // Situation 3, libraries L3a and L3b.
// Table: direct restrictions, other library, row `final`.
// Library L3a, file 'l3a.dart'.
final class A {
void foo() {}
void _foo() {}
}
base class A2 extends A {} // Or `implements`.
// Library L3b.
import 'l3a.dart'; // Declares `final class A ...` and a subtype `base class A2`.
final mixin M1 on A { void bar() {}}
base mixin M2 on A { void bar() {}}
final class B1 extends A2 with M1 {} // Or `M2`.
base class B2 extends A2 with M1 {} // Or `M2`.
final class C1a implements B1 {...}
base class C1b implements B1 {...}
base class C2 implements B2 {...}This means that we have obtained a subtype of the final class Similarly, the declarations named It looks like there is a need for the old rule 5 from here, that is, the rule which is marked (New). I can't see how that rule is derivable from the tables and text given here. Finally, I can't see how the treatment of // Situation 4, libraries L4a and L4b.
// Library L4a, file 'l4a.dart'.
base class A { void _foo() {}}
// Library L4b, file 'l4b.dart'.
import 'l4a.dart';
sealed class B extends A {}
base class C implements B {}The class Updated RulesHere is an adjusted version of the rules that I mentioned earlier. SealednessI believe the treatment of I think we should stick to the rule that properties are shown explicitly, which means that This means that That rule is orthogonal to the rules about Note that Proposed Constraint RulesThe tables below are similar to the ones I've mentioned earlier, but adjusted to handle mixin applications explicitly. We consider a class or mixin or mixin class The tables below show the constraints on modifiers. 'Any' indicates that Treatment of mixin applicationsWe need to consider the treatment of a mixin application, because a mixin application can appear as the superclass of another class. Hence, the symbol In short, when the superclass written as Constraints on subtypes of interface entitiesThis table is applicable when
For example: // Library L1.
interface class A {}
base class B1 extends A {} // Error, must be `interface` or `final`.
// Library L2.
import 'L1';
interface class B2 extends A {} // Error, can't extend A at all.
base class B3 implements A {} // OK, with 'any' modifiers you want.Constraints on subtypes of base entitiesThis table is applicable when
Constraints on subtypes of final entitiesThis table is applicable when
|
The The reason you can subtype The mixin declaration changes nothing. If you cannot produce an extensible superclass implementing SealedThe way we have treated If you do: final class A1 { }
base class B1 extends A1 {} // Warning, reallowing extension
interface class A2 {}
base class B2 extends A2 {} // Warning, reallowing extension
sealed class A3 { }
base class B3 extends A3 {} // No warning, there was no promise to disallow extension *transitively*
final class Z4 {}
sealed class A4 extends Z4 {}
base class B4 extends A4 {} // Warning, reallowing extension of *Z4*.the Or, in other words, we are being explicit. Every modifier (or lack of modifier) states precisely what other libraries can do with that declaration. We don't need to combine We also treat interface class A {}
// Warning about exposing implementation of `interface` clas.
class A2 extends A {}
// No warning, removing `interface` doesn't matter when we don't inherit implementation
class A3 implements A {}
base class B {}
// Both warns about reopening for `implements`, whether you inherit implementation or not.
class B2 extends B {}
class B3 implements B {}And inside the same library, we can still allow you to ignore all the restrictions in subclasses. Can can choose to require subtypes of "cannot implement" classes to be Whether those individual class restrictions add up to a consistent invariant depends on how you combine them. We must just make sure that you cannot also ignore constraints from other libraries when you ignore the constraints of your own library, which is why I have the transitive rules. |
|
@lrhn wrote:
Exactly, and that's the reason why I'm writing 'obtained a subtype', rather than something like 'inherited a method implementation'.
Yes, and consequently the declaration of But the broader issue is: If we actually establish a guarantee (modulo We cannot expect the maintainer of a library to study every detail of the subtype/inheritance hierarchies in that library in order to detect whether or not they can trust any particular guarantees. If we let this happen silently then |
Agree. I did not intend that guarantee to exist. We could make it, but I didn't find it necessary. Being able to create a type which is a subtype of F is not a problem, if the type is empty, and it doesn't provide an implementable interface. One of the reasons I don't want to make too many restrictions is that it makes it easier to handle Someone defines a I'd very much want to be able to create a mixin This relies on |
|
@lrhn wrote, about sealed declarations:
Well, Here is your final class A1 { }
base class B1 extends A1 {} // Warning, reallowing extension
interface class A2 {}
base class B2 extends A2 {} // Warning, reallowing extension
sealed class A3 { }
base class B3 extends A3 {} // No warning, there was no promise to disallow extension *transitively*
final class Z4 {}
sealed class A4 extends Z4 {}
base class B4 extends A4 {} // Warning, reallowing extension of *Z4*.Those warnings occur in exactly the situations where my tables said they should occur. I just don't agree that it's a good idea to omit The situation is the same with the Wow! Apart from the bit about representing every property explicitly, I just don't see any disagreement! 😄 (OK, I'd prefer to not have |
|
I'm considering the modifier rules independently of the lint+ The modifiers alone restricting what other libraries can do (extend, implement, mix-in) to the declared class or mixin. We have the desired guarantee that if a library exposes a We also decided to add the extra rule that any subclass of a I think we agree on the result of this, but let me write it up. Then we also want a lint which warns if you "reopen" a declaration in violation of its expected spirit. Anyway, another writeup:Modifier semanticsWe define four modifiers on With no modifier, other classes can freely extend classes, mix-in mixins and implement their interface (as long as all other requirements are satisfied, like a superclass needing to have a public generative constructor). With a modifier, one of
The restrictions do not apply to declarations in the same library. That is: A class or mixin declaration can extend, mix-in or implement another class or mixin declaration in the same library without regard for any Per the last language-team meeting, we add the "
This applies to all superinterfaces, not just immediate ones, including the ones from We assign an intent to each modifier, which relates to the concrete member implementation of the declaration. It's the primary use-case supported by that modifier.
We then add further rules to ensure that these intents are satisfied, unless explicitly broken by another declaration inside the same library. That is, the intents are not promises, they are use-cases that we support, and that you can achieve by following simple rules for adding modifiers on declarations in the same library. We make sure that other libraries cannot break these use-case without the original library's consent and cooperation. The The
Because of the This is one rule which is made easier by the Further, to close the mixin-loophole:
This ensures that if a mixin has an LintingA subclass can ignore modifiers of superclasses in the same library. They can also declare their own modifier almost independently, only restricted by the If a subclass has a less restrictive modifier than the superclass (other than a These only apply when subclass and superclass are inside the same library.
This warns if the intent of a There is no rule to warn if a We can, but might not want to, add:
This would warn if a The lint warning can be silenced by adding a The lint does not distinguish between public and library-private declarations. It could be argued that a private declaration cannot expose anything to other libraries. However, if you can removing a restriction in a private declaration, without any warning, it's not possible to determine whether that was an accident or a deliberate choice, and then it will be harder to warn correctly for any public subclasses of the private class. |
|
@lrhn, this looks very good! There's just one thing left, which is about orthogonality and consistency. I think the specification (and the feature) would be simpler and more consistent if we treat I'll refer to your proposal as the current proposal, and the proposal I'm making here as the proposal about orthogonal sealedness. With orthogonal sealedness we would then do as follows:
There is no interaction between The properties This is different from the current proposal where the use of Here are some reasons why this is an improvement: We agreed already a long time ago that it would be helpful for a reader of the code if every class modifier is present syntactically in every case where it is enabled semantically. With orthogonal sealedness that is true for With the current proposal, it may be necessary to introduce an artificial supertype in order to obtain a specific set of properties for a given sealed class. With orthogonal sealedness it can be declared directly: // Current proposal.
sealed class A extends Abase {...} // To enable the `base` rules in the subtype hierarchy ...
base class Abase {} // ... we introduce this extra class, just so we can say `base`.
// With orthogonal sealedness.
sealed base class A {...} // Same thing: Simple and readable.Another issue is that a sealed class can't reopen anything in the current proposal, but we can do that with orthogonal sealedness: // With orthogonal sealedness.
final class A {}
@reopen // Relax the `final` discipline to a `base` discipline.
sealed base class B extends A {...}
base class C1 extends B {...} // We get a heads-up if we forget `base`.
base class C2 implements B {...} // Same freedom as in the current proposal: can use `implements`.Here's a way to express approximately the same thing using the current proposal: // Current proposal.
final class A {}
// Can't express reopening to `base`, do it repeatedly below.
sealed class B extends A {...}
@reopen
base class C1 extends B {...}
@reopen
base class C2 implements B {...}It would again be possible to use an artificial |
Based on Lasse's proposal at: https://gist.github.com/lrhn/981d4528fbb995c23afa0f9436209537 Fix #2755. Fix #2757.
Based on Lasse's proposal at: https://gist.github.com/lrhn/981d4528fbb995c23afa0f9436209537 Fix #2755. Fix #2757.
|
I believe we have fully closed the door with the current design. |
The intent with marking a class
baseis that there's no way to create an instance that is a subtype of that class that doesn't directly inherit it. The proposal tries to ensure that by requiring that concrete subclasses of thebasetype can't themselves expose an interface. Instead, they have to be markedbaseorfinalthemselves. But that rule can be ignored within the current library. That lets you get an interface to a class you don't control like so:Cis a subtype ofAbut does not extendAor inherit its implementation.I was hoping to have the static semantic rules specified just in terms of the immediate supertypes of the declaration, but I can't think of a local restriction that closes this hole while still allowing you to implement classes marked
baseinside your own library.Maybe the right fix is to change the rules about requiring
baseorfinalon subtypes to:Does that sound right? @lrhn @kallentu @stereotype441
The text was updated successfully, but these errors were encountered: