-
Notifications
You must be signed in to change notification settings - Fork 207
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
Experiment - Class annotation for disallowing use as a super-type, a.k.a "sealed classes" #11
Comments
CC @eernstg @lrhn @leafpetersen @matanlurey @munificent Not sure how fleshed out the "Problem" issue should be. Happy to flesh out more. "Solution" issue / PR coming soon. |
I think this is great. |
I would love to see something like this, but I'd need a little more flexibility. In the published analyzer packages we define the public interface as an abstract class and have one or more internal classes that implement that interface. I want to be able to mark the public interface as being one that can only be extended/implemented/mixed-in within the same package (without requiring that all implementations be in the same library as the abstract class). I understand that "package" isn't a concept known to the language spec, but it's an important concept for package authors and I think it needs to be taken into account when defining visibility controls. |
I suspect this is one of those features that we wouldn't end up using in the Flutter framework all that much. For example, we wouldn't have used it for ThemeData, since if people want to override it, who are we to argue? Even within the framework itself we extend core classes like Size (with _DebugSize). Ideally there would be no performance implications, since the release mode compiler can do full-program analysis. |
I actually really like this idea as well. Libraries as a barrier on visibility make for awkward large implementations; either you use a lot of parts (yuck, imho), or you have massive individual files (bad for human ergonomics). My thought is that we can implement the most restrictive version of "sealed classes" that "we" "like" first, and then any loosening of restrictions is not a breaking change *. Of course, I won't define "we" or "like," 😄 but the consensus might be that we want one concrete class, or maybe all concrete classes declared in the same library, or maybe even the package-level version; whichever we choose though, none of these choices prohibits the idea of package-level sub-classes in the future. Especially while we define packages firmly, and maybe think about JIT / DDC implications, where all sub-classes are not definitely found in the same library as the super-class declaration. * er... I suppose it is a breaking change in terms of the back-ends, if a back-end depends on optimizing a sealed class with the idea that there is exactly one concrete class. But such a change is still not a breaking changes for users; the change just has to be coordinated with back-ends and static analysis. |
@Hixie Would flutter not be interested in this insofar as announcing that a class has not been written and tested carefully with the sub-class case in mind? You can always write comments today "Do not sub-class," but users still have the ability, and may rely on weird sub-classing behavior until you cut them off. Just a thought. |
We generally avoid checking in code that has not been written and tested carefully with the sub-class case in mind. I don't think we have any docs that say not to subclass something. The one exception would be some mixins and interfaces, where we already prevent subclassing by judicious use of private constructors. |
Huge 👍 for this first version of this feature, and for experimenting. For folks that aren't aware, we are suggesting the
We agree, just it is easier to start with the most restricted ruleset and incrementally make it more open then go in the other direction. For example, there is some clarity around library/package visibility I think we should answer before allowing limited inheritance, but I don't want it to block @srawlins first release. My understanding is you could continue not to use
I think we should try and follow-up, separately, with how the language team and specification wants to tackle this problem (modules and visibility). I've made it very known that our users are having a very hard time figuring out the difference between files/libraries/packages/bazel-targets, and our tooling is not consistent here.
Hence it being optional. But I remember @yjbanov mentioning there were a few classes that when users made them polymorphic (often on accident) the performance severely decreased. I'd hope
What do you mean by this? How is this related to |
It would be nice if there was a way to detect those, but at the end of the day, if someone wants to do it, we don't want to stop them. One option would be for sealing a class to be merely advisory, e.g. |
I am not sure unfortunately that fits with spirit of the language in 2018 (or how other modern programming languages work - it would be like saying the ... though as far as it stays an analyzer-only hint you can |
I personally totally disagree with this:
And agree with Ian:
I do think that this shows something bad:
But I think it should be a warning to do so, not an error. If it's not a warning, what inevitably happens is people fork libraries and change the class to be unsealed. What benefit does that serve anyone?
It is also a substantial amount of work to get something done when the library you're using sealed a class that you need to extend to solve your problem. What do you do if the author refuses to unseal the class? Rewrite the library? Making a class extensible is O(1) for some large constant factor, but if downstream users have to fork the library that's O(n). We're programmers! We should prefer the former to the latter! And as a user, I don't care about any of this "substantial" design/testing, I won't do it. I will test that my own integration works, and if other cases aren't tested, that doesn't matter to me. If I can't get the class unsealed I will fork the library to get what I want, and now I'm really losing out. I do believe that any author of any package is free to add "DO NOT EXTEND/IMPLEMENT," whether on a class-by-class basis, or package-level basis. And I also believe that as a user of such a library, I want static analysis to help me when I've broken those rules. But if I've broken those rules, I want to add a single-version constraint to my pubspec, and as updates to that package come out, I can test it against the updates and widen that range accordingly. So that I can get what I want done without depending on an update from the author, without forking the library, and in a way I know works for me. (maybe even a pre-publish warning, "^x.x.x" is an unsafe constraint because you extend a sealed class). Inside google3 this is better as an error than a warning, so maybe there should be a flag --strict-sealed-types. But overall I think final classes are a mistake. I would call it "overparenting" if the package author is the parent and the downstream users are its children :) Sometimes you gotta let you kids fall down, and that's ok! But it is a good idea to warn them "careful, that's slippery!" |
It seems like me we are debating the semantics and benefits of static type systems and rules more than this particular optional analysis rule (that can be Dart has a number of sealed types ( (From a "what is everyone else doing" perspective, Swift, Kotlin, Java, C#, Rust, and practically every other modern language has the concept of sealed or final and does just fine. Dart 2.0 already has a lot of rules around what you can and can't ignored compared to Dart 1.0, and we also seem to be doing just fine) |
I have libraries, published externally and internally, that are impossible to sub-class correctly from a user's perspective. They store and return private state that is inaccessible to the user, and using anything but the pre-defined classes at best is undefined behavior, and in practice breaks the application. A more practical example today is
Personally this is very regressive, and doesn't work in single-source environments like google3 anyway (and because Dart has nominal types scoped to a library, you can't even use your custom type in shared infrastructure, you'll get (Unfortunately the compilers will not, in that case, though) |
One of my favorite things about Dart is that Dart does not do this. Dart has an edge over the competition here in my mind :)
I get this, and it's valid -- but its also ironic. "Why can't the Dart team trust users to use this feature correctly?" is not far off from "Why can't library authors trust users to subclass correctly?"
Don't Dart2js and flutter already get all the performance benefit they need since they do full program analysis? I imagine though that yes, sealed classes would be something leveragable by DDC and the JIT for better performance, which would be a win. If we have |
Unfortunately Dart also does not have extension methods or default methods or any other mechanic that makes it easy so extend existing classes in a non-breaking fashion, so I don't think its a fair comparison yet. Something like an (enforced)
I mean, if you want to try and convince the language and core team to allow sub-classing
My understanding talking to folks is that because both want to move to a more modular compile, knowing earlier that something is
Unfortunately that would mean that 100% of libraries internally would need be compatible with this flag to take advantage of this optimization. If you look at the difficulties of the Closure compiler rolling out |
Of course, but, while I might be wrong :-), I don't think I'm the only one who wants the extended use case. If we're planning on relaxing the requirements incrementally, I think it's important to define how will we know whether we've relaxed the requirements enough?
I am also very frustrated that we are in a position where some of our terminology is inconsistent. It makes it harder for users to understand what we're talking about, and sometimes causes confusion within the team. As for warning vs error, I personally don't have a strong opinion; there are trade-offs either way. What I really want is a way for clients of the packages I work on to be told when they're doing something we don't expect/want them to do (creating a subtype of a class). If it's impossible, that can lead to one class of problem. If it's possible, then at least we've warned them that we're not going to protect them from breaking changes and can make changes when we need to. |
I think we'll need to gather better evidence that Classes end up on the heap, and My gut feeling is that for actual performance gains we will want frequently allocated small objects not end up on the heap. They have to be unboxed. That means, in addition to Having said that, it is easier to relax restrictions in the future than impose them. If in the future we decide to introduce unboxed types, it would be good to seal today classes that will be unboxed in the future. Otherwise we'll either have to go through a painful migration, or (as usually happens) not deliver any performance gains at all. Ideally, performance would be an explicit goal for Dart. If @Hixie, the way polymorphism works in languages like Dart it can have very non-local effects on the performance of the system. For example, extending the |
This is exactly what
constitutes, though. I want to empower developers:
This avoids all new "rules for thee and not for me." I agree that there are cases where sealed classes make sense -- like String. Where I disagree is that its a maximum benefit to let any class be marked as such. I think the alternative I presented has all the benefits and more. I also think its totally reasonable to say things like, "Dart 3 may require --strict-sealed-types, and we highly encourage you to use it, or your code may not maximally be forward-compatible." Which would help with the increasing-restrictions-over-time-instead-of-relaxing-them problem. |
Maybe this itself deserves another issue then? I don't know if the language team has the bandwidth to tackle this right now then, but we could start to gather information and requirements around modules/visibility for Dart:Next.
I am guessing this will probably means you also can't have arbitrary user code extending/implementing your types (i.e. the strictest sense of
I don't think its a good idea to block the restricted form of this analysis feature on those requirements, though. For one, it will be very possible to loosen the restriction in the future. For another, I think even your suggestion of sealed except for same library will not be enough for some users. Consider:
import 'buffer_default.dart'
if (dart.library.io) import 'buffer_vm.dart';
if (dart.library.html) import 'buffer_js.dart';
// Based on the "library only" sealed proposal, this would be illegal.
@sealed
abstract class DataBuffer {
factory DataBuffer(int size) = DataBufferImpl;
} I think we all want to avoid:
... so starting with the most restrictive sub-set (i.e. "pure" |
There are various problems being described here, and it's not at all clear to me which of these require sealed classes as an answer. For the performance issues in particular, I would focus on tooling solutions: lints pointing to subclassing cases that will impact performance, observatory tooling in the profiler to pin point specific classes whose inheritance might be an issue, etc. For the issue of APIs that weren't designed for extending, I would focus on analyzer warnings, metadata, and documentation. For the issue of Dart being different than other languages, I would do nothing. Dart is it's own language; we shouldn't add features just for parity. We should add them when there's a real need. For Flutter, I suspect we will never seal a class; we encourage people to view the framework as something they should build on. We also make great use of subclassing in tests. We don't want to limit people, we want to empower them. That said, I have no objection to the language feature being added. I'm sure it has value in many cases. |
I should be clear that both suffer from this. I can mark my code This is way more power that an author holds, than being able to override something in a the file its defined in.
I agree here -- if its something you can Ultimately, all sides of this are trying to mitigate risk -- risk of people Its hard to know which risk is more important to mitigate unless we have data. My only caution here, that I'll express as a prior so that it doesn't look like a moving goal post if it comes up in the future, is that probably most people who start to rely on |
A brief comment on performance: A lot of the performance discussion seems to be assuming whole program compilation. My understanding is that this is no longer something we can assume. One of the goals of having an experimental version of this feature is to be able to measure performance benefits. |
I have opened dart-lang/sdk#34135 to try to avoid talking too much about visibility/libraries/packages here. Hopefully interested folks can add their feedback to that thread instead. |
Right, my understanding from talking to Dart2JS and the VM team is that @Hixie's suggestion of:
... Is a very Dart 1-way of viewing Dart (i.e. write whatever you want, we will make it fast). ... it also seems like a waste (to me) of the only CFE-ification of Dart. If every backend needs to implement totally custom whole-program analysis in order to emit fast/efficient code I'm not sure what the language team is supposed to do other than just tweak syntax. |
I think we need to be careful about selection bias here. If we define the most restrictive form, then the analyzer code base (for example) will not be able to make use of it. Any data that is collected will tell you nothing about how analyzer would have used a definition that better meets its use case. |
Not to discourage investments in tooling, but unless we have good reasons to believe that such tooling can be built and will be effective at improving performance, I'd prefer that we attempt some time-tested solutions too. Specifically, with polymorphism having non-local effects on the program, it is not clear how tooling can help with improving performance of separately compiled modules where whole program analysis is not available. However, type system-based solutions are actually able to enforce the necessary contracts across modules. |
Disclaimer: I have not been involved in performance discussions and everything seems simple until you actually are responsible for it. I'm absolutely missing all the information and this is all speculation. I get that there are good reasons for doing this, and I'm all for it, but marking classes as final will never be as fast as finding all methods which, for a particular program, have no overrides. This is a classic link-time optimization, why wouldn't that work for dart? Not to be pessimistic here, but final classes would not gain us performance, it wouldn't even enable modular compilation. Final classes would mitigate the performance hit of not investing developer hours into creating a link-time optimization phase for a modular compilation workflow. Am I wrong here? I don't think its a good way for us to save time, this will push the language to be more and more restrictive in pursuit of performance, because there will always be more classes we could mark It's one thing to do full program type flow analysis at link time, to have no modular compile phase at all. Its another thing to make a set of non-overridden methods and translate them differently when outputting a big binary assembled from multiple smaller ones. end speculation. |
Long thread is long! I want to reply to one point:
Our experience on the language is that once an experiment gets out into the wild, the Dart team no longer has any control over it. Asserts in initializers and supermixins were both "experiments" that the language team had to scramble to some how clean up and specify once we ended up stuck with them. It's not a given that just because we call it an experiment that we can easily modify it if users are relying on it. In this case, because it's just an annotation, it's probably OK, but we should still be cautious. Also, a lot of discussion around how limited the semantics should be and that we can widen them later. Given that, it's not clear to me what this experiment is for. What questions are we trying to answer? How do we get useful feedback from the experiment if the semantics are still up in the air? |
I'd really like to avoid getting too derailed on the performance issue.
I agree with @munificent that it's worth spending some time even on an early prototype to try to have a picture in mind of the end goal. I'm hoping that this issue can be the start of some design work around that. |
I've taken the last two days as a comment period, and now I'm free to move forward without taking the comments into account. 😛 Just kidding; can you imagine!? Okay, this is now a feature request for
(Sorry, GitHub doesn't support intra-page headers; I can't link to the specific headers) There are some comments that I didn't really address above in the new summary. I'll address them here:
This benefits the authors of the sealed class in that they don't have to support the subclass. This benefits the users who chose to sub-class in that they now can do what they want. It serves all of the benefits listed in the summary. This seems like a rather blanket argument. Doesn't it also apply to private members, implementation imports, type checking, ...?
Er... huh? I don't get this. For one thing, @matanlurey argues that making certain Angular Dart classes, which certain teams have sub-classed, extensible, is an incredible amount of work and refactoring, even requiring breaking refactors.
It's definitely nice to work on products whose APIs aren't used a lot in an environment where you are be responsible for helping to migrate all breaking changes. This is not the case for packages used widely in google3. It matters to many users.
This only works in versioned vendor environments.
This language feature (or experimental annotation) does not warrant such an expensive feature for pub.
This is not a feature for the "kids;" it's a feature for the "parents."
Since there's only ~3 levels of relaxation, I don't think this will be hard to know. I think a few months of experimentation and comments will give us a good idea. I'm not prepared to accept today an agreement like "The definition is only relaxed enough when the analyzer package can take advantage."
If you have such a proposal that works in non-versioned vender code repositories (google3), this would be interesting.
I think initially we were talking about a language feature, and now its about an annotation in the meta package, which might be what you presented.
That's true. I'm interested in getting some performance optimizations data first. If there are performance benefits to be had, then we can re-visit the question of allowing package-level sub-classing. But to allow package-level sub-classing first prohibits the performance experiment.
This is very interesting. I can't argue with this. I think this was written with the idea that these discussions were about a language feature (which was also my intent), but since we've backed down to an experimental annotation, maybe we can leave this question for later.
We do still need the experiment. For the performance question, for how much authors will use this, how often authors want package-level sub-classes, and how often users fork libraries in order to sub-class. Since it costs developers to implement language features, the experiment can tell us how valuable the feature might be.
Package-level is an open question, but forbids the performance experiments. I hope I've answered a lot. It has been duly noted that there exist developers who would not use this feature on their own code. More feedback is definitely welcome. |
Sorry, but that's too vague to be an answer to my question. I understand from what you wrote that "meeting analyzer's needs" (or any packages with the same needs) is not the criteria you're using, but what is the criteria. Is the criteria "meeting angular's needs", or is it broader than that? My question is just a specific example of Bob's more general question of "What questions is the experiment trying to answer?", and I don't think that's been answered. Unless the following comment is intended to be the answer to that question:
If these are the questions we're trying to answer, then I think we need to clarify them a bit. For example:
Perhaps I'm wrong, but these appear to me to be inconsistent statements (unless "authors" is taken very narrowly). I think that we're conflating two questions: (1) can we get performance gains from supporting final/sealed classes, and (2) can we annotate code to discourage clients of packages from misusing the package's public API. I think we'll make progress a lot faster if we separate the two questions. And, I think that the performance question needs to be answered first because it potentially constrains the answer to the second question (assuming we want the same solution for both problems, which isn't at all clear to me). |
Are the questions under the Isn't "experiment" just another word for "I don't want to go through the trouble of actual approval?" heading not what you are looking for?
How about Angular, Analyzer, Sass, Flutter, and vocal people, e.g. on GitHub, mailing lists, chats, ...
I think @mraleph has expressed interest that the VM could run an experiment. Someone on the VM team would do this work. It doesn't need to be done anytime soon. If it's never done, I wouldn't be bummed out, and we could re-examine the package-level sub-classes question.
They might.
That's fair. I guess as long as we meet some low-water mark (at least one important, heavily depend on project would experiment with it), then I don't see other responses of "I won't use it" as interesting.
I disagree. I've outlined an experimental |
Sorry, I didn't notice those when I read through. There's still a question in my mind of how the experiment will answer some of those questions (some seem obvious).
In the sense of "do we have any users that want this feature as defined", I think the answer is clearly "yes".
I'm not sure what "abuse" means in this context.
Do we have access to enough user-written code to know whether these are happening?
We don't need an experiment to answer that question. The real question here is what percentage of users that want to control sub-classing are we willing to not support?
I agree. I probably misinterpreted, but if felt like you were including "I want to use it but can't use it as is" as also being uninteresting.
The performance goal is limiting the definition of the annotating classes goal. They aren't independent as currently defined. I don't want multiple variations of the "sealed" annotation either. Nor is the following a suggestion that we implement them. That said, if we did have them then we could measure the adoption of various flavors and possibly use that information to inform the decision of how relaxed the definition needs to be. |
Haha, one man's "abuse" is another man's "defensive programming." I guess this is coupled with the next two questions:
My answer is just, community feedback. We can poke around internally as well. For example, if we see a greenfield class being written with
Why not? While some people here like the general idea of being able to seal classes, some are of the opinion that even just the ability will be a net loss to the Dart language and community.
I see. No that is interesting. In answer to "what percentage of users that want to control sub-classing are we willing to not support?" I would guess that a good chunk (20% - 50%) of authors who would use this would need package-level sub-classing.
I think that's true, but doesn't block the proposal; just a known limitation. Without the performance experiment, I would still be against package-level sub-classes as a first experiment, because it has a worse path forward to language feature. It would be stuck as an unenforceable annotation. The three paths I see to a language feature:
|
Well, they're just wrong! :-)
Note that that would satisfy the concern above. If it's advisory-only, then there's no loss to the community because it can be ignored. It also happens to be sufficient to the annotating classes goal (or at least my version of the goal :-), but not to the performance goal. If we knew that such a feature was not useful for performance reasons, then I think it might well change the tenor of the conversation. So I still think that we should separate the two goals and test the performance hypothesis before worrying more about the definition of the annotation. (And I'm guessing that we want to use a |
So I think @srawlins has answered and done everything he can be expected to do to document the decisions being made here and we should go ahead with adding the annotation. (I don't recall any similar process for any other annotation in package:meta - where was the design review for If anyone has any other serious objections please talk to Sam or Leaf directly. |
@leafpetersen did raise a good point. At the moment, people often just don't do things that would break users (like add methods to a class). Or they mark it a breaking change, which slowly ripples through the community. With an ignorable I think its reasonable to use pub warnings to try to prevent it, but that is not free. It could alternatively be something the experiment aims to measure. |
I'm coming around to the idea of two annotation experiments. ducks One would be this one, as it is written up top. The other would be something like,
OK. What's the path forward?
Mulling this over at lunch, I think this satisfies a lot of requests, and lays out some possible, optimistic, paths for the future. I'd be very happy as well to write the analyzer Hints for both 1 meaning by Angular, Analyzer, Sass, Flutter, and vocal people, e.g. on GitHub, mailing lists, chats, ... 2 "when" is TBD. 3 "what is 'important'" is TBD. |
I'm certainly not objecting to additional experimental annotations (certainly most of the annotations in One issue though with adding more of these is that we're asking them to be hints by default (i.e. they will show up in everyone's IDE unless they turn them off), and I'm concerned internal to Google it will be very confusing to explain the difference between "library" and "package" to most users (I have another thread on this) - for prior art see @srawlins' work on |
That's a good point. I think we can try to mitigate it though. (For those not using bazel: the terminology gets bad quickly: a bazel package can have multiple We can try to be specific in the messaging:
Eh? |
@srawlins, I like your updates to the proposal and I think it's reasonable to move forward with doing an experiment with I think it would be useful if you could extend that scope to some sort of larger "package" or "compilation target" granularity. But that notion doesn't currently exist, and would likely take a lot of effort to define. I don't think we should block |
Thanks a lot for all of the useful input folks, I've picked up a lot from the discussions here and offline - I appreciate everyone staying civil and constructive on a topic that I know there are strong feelings about. And thanks very much to @srawlins for writing up a detailed proposal and spending time on feedback. I'm still digesting this a bit, but it seems like the interesting issues are:
On the question of @Sealed vs @packageSealed: for the experiment would it be reasonable instead to have @Sealed take some kind of optional argument defining the scope? This might give us data on what scope authors actually want? Just a thought. |
Some of us talked offline more, and we've changed gears to recommending a package-level defintion of "sealed classes." I've updated in the summary up top, the Definition, Alternative Definition, and Path towards a language feature sections above. This was largely the result of dropping the performance experiment from the story; although sealed classes could still be performance-experimented, by using sealed classes that are sealed within libraries, or by using package-level compilation units in something like dynamic code loading. In any case, performance considerations are just not part of this feature request. @eernstg, you strongly recommended using library-sealed classes over final classes in sealed's clothing. I don't see why package-sealed would change your argument, but please comment if it does. |
No problem! I just argued that it's more useful to have a notion of sealed classes which creates a closed set of types, rather than just sealing one class at a type (and being unable to close any type hierarchy as a hole, because no non-bottom types can be 'sealed' when that just means final). Any granularity which is a library or something bigger (a set of libraries that somehow form a group) would have that property. Any developer who is working on, say, a package which contains a sealed type hierarchy would still be able to work on the complete class hierarchy and rely on the fact that there are no other subtypes. So that developer would still know that no "outside" subtypes are going to break if something changes, it's enough to worry about breaking changes as seen from clients (and private classes would always be invisible and hence safe when the scope is at least a library). It would always be possible for an implementation to use the improved situation where something is sealed in a library to obtain some additional optimizations, and it shouldn't be hard to support library-sealed classes later on. |
I, personally, have no problem adding "package" as a concept to the language. It will be "Dart package", because that's all the source can represent. I can see (multiple) ways to do "package private" with that, and perhaps even allow overriding it for testing. So, making sealed package-scoped is not necessarily a blocker for making it a language feature. That said, I'd much rather be working on features that make it possible to modify a class without breaking sub-classes, rather than require you to seal the class up-front just to have the ability to maybe modify it later. Such a feature would help you even if you didn't seal the class. I can absolutely guarantee you that if we had had sealed in the language then, |
I know that this thread is really about the lack of Out of frustration not having Kotlin style I am hoping that this will help some until we have a real implementation. @brianegan has a sample MVi architectural example that uses this package. |
This has been released in Dart SDK release 2.1.1; the CHANGELOG notes the two new codes: |
Users have expressed a desire for the ability to mark a class as "final" or "sealed," so that it is unavailable as a super-class. That is, it is an error for a class to extend, implement, or mix in a "sealed class." Specifically, the Angular Dart team wishes to seal certain classes that have very undefined behavior when users subclass.
There is good discussion on an MVP, make-something-cheap-available-to-users request for a
@sealed
annotation.An experiment
All that is being suggested here, after some discussion 1, is an annotation in the meta package,
@sealed
, enforced with some analyzer Hint codes.Use cases
The primary use case is to remove burden from, and give assurance to, authors of public classes. Today, as all classes are "open," there is an expectation from users that they can extend, implement, and mix in classes from other packages safely. For this to be true, authors must actually have extensions, implementations, and mix ins in mind, unless they write "DO NOT EXTEND/IMPLEMENT" in their Documentation comments.
A "sealed classes" feature can remove burden from authors by allowing for a class that doesn't need to support the possibility that other classes use it as a super-class. Authors can write a class
without worrying about how to support sub-classes.
A "sealed classes" feature can give assurance to an author, when considering how a change to the class will affect users. An author can:
without worrying about how the change will affect potential sub-classes.
Definition
A sealed class, one which has been annotated with
@sealed
from the meta package, can only be extended by, implemented by, or mixed into a class declared in the same "package" (to be defined) as the sealed class. The consequences of "sealing" a class are primarily in static analysis, and would be implemented in the analyzer package:@sealed
annotation occurs on anything other than a class.@sealed
.Why a Hint?
All analyzer codes that do not arise from text found in the spec are HINT codes (well, except TODO and LINT, and STRONG_MODE_* codes). Since this is the most formal proposal for an annotation that I know of, perhaps it will pass enough muster to be listed as an ERROR or WARNING... if there is such a request, this can be specified at such time. (@MichaelRFairhurst requested below not to use error.)
Ignorable?
All analyzer codes are ignorable with an
// ignore
comment. The above Hints will also be ignorable:// ignore: USE_OF_SEALED_SUPER_CLASS
will allow a sealed class to be used as a super-class outside of the library in which it is declared.// ignore: OPEN_SEALED_SUB_CLASS
will allow a non-sealed class to sub-class a sealed class from within the library in which each is declared.(@MichaelRFairhurst requested below to allow these analyzer results to be ignorable.)
Alternative definitions
Library-level sub-classes
Library-level sub-classes would be very similar to package-level subclasses, but would be more restrictive. Library-level sub-classes make an earlier suggestion of performance experiments easier, but the performance experiments are now a non-goal of this annotation. Members of the Dart team feel that a package boundary is the more natural boundary for something that authors create for themselves; typically the authors of a package "own" the whole package, rather than distinct pieces.
Additionally, new visibility mechanisms were suggested; maybe Dart can support an "open part" (as @eernstg suggests below or "friend library" (as I suggest in a comment on testing private methods). The part/friend concept would help back-ends close the list of all sub-classes, but we don't have this concept yet, so cannot experiment yet.
Single concrete class
The "sealed classes" feature originally restricted sealed classes to be un-extendable, un-implementable, and unable to be mixed in, by any class anywhere ("final classes"). @eernstg argues below that the reasons for making a "final class" are different from those for making a "sealed class," and that it would not be very meaningful to switch the definition of
@sealed
from one to the other.Since Angular Dart can make use of library-sealed just as easily, and back-ends like the VM can optimize just as easily, then we use the library-sealed definition.
Isn't "experiment" just another word for "I don't want to go through the trouble of actual approval?"
We actually do want to experiment. Real world usage can help the language team steer towards correct choices later:
// ignore
the "sealed classes" Hints?Depending on the answers to the above, "sealed classes" may be shelved, or implemented with the same definition as the annotation, or may be implemented with changes. Other features may be implemented or experimented with, such as final classes, sealed-by-default, open, noImplicitInterface, etc.
Cost of rolling back the experiment
@munificent points out below that asserts-in-initializers and supermixins were both "experiments" that did not smoothly transition to full support; we should try to avoid a similarly bumpy road.
If the
@sealed
experiment "fails", i.e. the team decides it should be rolled back, it can be done so without much fanfare. Rolling back the feature means removing enforcement from the analyzer (and any back-ends with performance experiments based on the annotation). A Dart release will include a release note saying something to the effect of "Any sealed classes may now be sub-classed at will; don't rely on them not being sub-classed."Path towards a language feature
For package-level "sealed classes" to graduate from an experimental annotation to a language feature (like a class modifier), a definition for package will first need to enter the language. There is currently no effort towards defining such a thing, but there is motivation from several facets of Dart to make one.
Prior art, discussion
Java
A case of prior art / prior discussion, Joshua Bloch, once Google's chief Java architect and author of Effective Java,
wrote in Effective Java,
Joshua goes on to explain the work involved in properly designing, implementing,
and testing a class that is subclassable, which is substantial. When Joshua writes "or else prohibit it," he is referring to the use of either (a) marking a class
final
or (b) using only private constructors. The private constructor solution, (b), does not work for Dart today, as all classes have implicit interfaces which can be implemented. A "no implicit interface" modifier could be a sibling feature to this "sealed classes" feature, but I consider it far out of scope.Kotlin
Kotlin has a more advanced feature set regarding "sealing" or "finalizing" classes. Here's a quick rundown:
open
modifier must be applied.sealed
modifier. This means that the class can only be sub-classed within the file where it is declared. This has the effect that the author immediately knows all of the possible direct sub-classes, and can use this knowledge in switch statements; you can cover every possibility of sub-classes with a finite and immediately known set.open
, and they can be sub-classed when open.I really like these two similar but distinct features. For a sealed class, the ultimate set of concrete classes with a sealed class as a super-class cannot be known, unless all direct sub-classes are "closed." This property is under the author's control; if the author just wants to know all direct sub-classes, for use in, e.g. a switch statement, (and if they're willing to support the idea of sub-classing), then they can mark sub-classes as open. Otherwise, if they want to know every concrete class with a sealed class as a super-class, they can leave sub-classes closed, and not have to support the concept of sub-classing.
Other languages
Other languages, in addition to Java and Kotlin, have a similar / identical feature, either "sealed" or "final" classes, including C++, C#, and Swift. Neither JavaScript nor TypeScript have a similar feature.
Footnotes
1 Initially, I did not predict the level of discussion that this feature request would raise. Initially, I thought that the experimental
@sealed
annotation would land quickly and quietly. This feature request was initially for a language feature, "sealed classes." After seeing all of the issues being raised, and some thinking that this was just a proposal for the experimental annotation, I've decided to make it that.The text was updated successfully, but these errors were encountered: