Experiment - Class annotation for disallowing use as a super-type, a.k.a "sealed classes"

[srawlins]

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 ¹, 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

• whose methods call other public methods,
• whose methods refer to private members,

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:

• change a method to refer to a public method,
• add public methods,

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:

• It is a Hint if the @sealed annotation occurs on anything other than a class.
• Given a class C declared in a package P,
  • It is a Hint if a class which is declared in a package other than P extends, implements, or mixes in C.
  • It is a Hint if a class, declared in P, and either extending, implementing, or mixing in C, is not also annotated with @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 STRONGMODE* 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:

1. Are sealed classes useful?
2. Do public class authors "abuse" sealed classes? ("Abuse" might mean "very defensively program with." We want to see how sealed is used.)
3. Do users // ignore the "sealed classes" Hints?
4. Do users fork sealed classes in order to unseal them?
5. Is the "sealed" concept useful as the single great class modifier affecting sub-classing? Do users basically use it as "final?"

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,

Design and document for inheritance or else prohibit it.

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:

• By default, a class is "final," such that it cannot be subclassed. This is directly motivated by Joshua Bloch's writing. To make a class open for subclassing, the open modifier must be applied.
• A class can be "sealed" with the 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.
• One more note: the sub-classes of a sealed class have no restrictions different from that of other classes; they can be marked as 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

¹ 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.

[srawlins]

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.

[munificent]

I think this is great.

[bwilkerson]

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.

[Hixie]

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.

[srawlins]

@bwilkerson

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

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.

[srawlins]

@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.

[Hixie]

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.

[matanlurey]

Huge 👍 for this first version of this feature, and for experimenting. For folks that aren't aware, we are suggesting the @sealed annotation as an analysis-only experiment, and I'm more than happy to roll it back if it ends up not solving the problems we're intending to solve or if it isn't practical enough.

---

@bwilkerson:

I would love to see something like this, but I'd need a little more flexibility.

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 @sealed in this case, right?

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 ... I understand that "package" isn't a concept known to the language spec

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.

@Hixie:

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?

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 @sealed could help in those scenarios.

... Ideally there would be no performance implications

What do you mean by this? How is this related to @sealed?

[Hixie]

I'd hope @sealed could help in those scenarios.

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. @Sealed('Consider using foobar instead of subclassing Baz, because subclassing Baz is known to have major performance implications.'), which causes a warning to be shown in the analyzer when you extend it (similar to @Deprecated('...')), but which you can then silence if you want using // ignore.

[matanlurey]

@Hixie:

but which you can then silence if you want using // ignore.

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 int type signature is optional all over again). From talking to @mraleph and the AOT-compiler team, they really want more ways for the compiler to be guaranteed certain things so they can optimize based on it, and making it ignorable would not help here.

... though as far as it stays an analyzer-only hint you can // ignore: it anyway 🙌 - and like I said there is no guarantee this will even make it out of the analyzer, so the conversation is likely premature anyway - and I don't want to de-rail this issue thread. Flutter of course is not required to use this (optional) feature and we in google3 would gain a huge advantage with the definition as-is.,

[MichaelRFairhurst]

I personally totally disagree with this:

By default, a class is "final," such that it cannot be subclassed.

And agree with Ian:

if people want to override it, who are we to argue?

I do think that this shows something bad:

unless they write "DO NOT EXTEND/IMPLEMENT" in their Documentation comments.

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?

Joshua goes on to explain the work involved in properly designing, implementing, and testing a class that is subclassable, which is substantial.

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!"

[matanlurey]

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 // ignore: ...'d anyway), so I'd prefer we don't use this thread to do so.

Dart has a number of sealed types (int, double, num, String). If the language team found it useful to constrict being able to sub-class these types I find it unfair that library and framework authors cannot also determine value-like or private-ish 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)

[matanlurey]

@MichaelRFairhurst:

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?

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 _private visibility. Do you fork a library every-time some field or method is private that you want access to?

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.

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 Type PrivateFoo is not a PrivateFoo errors). But FWIW, if we need --strict-sealed-types (ugh!), fine, as long as I get the benefit.

(Unfortunately the compilers will not, in that case, though)

[MichaelRFairhurst]

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.

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 find it unfair that library and framework authors cannot also determine value-like or private-ish sealed types.

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?"

(Unfortunately the compilers will not, in that case, though)

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 --strict-sealed-types as a compilation flag as well as an analysis flag, they should be able to get that benefit too.

[matanlurey]

@MichaelRFairhurst:

One of my favorite things about Dart is that Dart does not do this.

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) sealed might be one of the few things library authors can do - otherwise it becomes harder (and eventually impossible) to evolve a library or API.

Why can't library authors trust users to subclass correctly?

I mean, if you want to try and convince the language and core team to allow sub-classing String, I'm all ears. But "rules for thee, and not for me" don't work out well in my experience, and are very frustrating to work around from a library author perspective.

Don't Dart2js and flutter already get all the performance benefit they need

My understanding talking to folks is that because both want to move to a more modular compile, knowing earlier that something is sealed could actually still help (as well as help users via the analyzer).

If we have --strict-sealed-types as a compilation flag as well as an analysis flag, they should be able to get that benefit too.

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 ADVANCED_OPTIMIZATIONS, you'd see that Dart really can't afford to fall into that trap.

[bwilkerson]

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 @sealed in this case, right?

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 want to be able to mark the public interface as being one that can only be extended/implemented/mixed-in within the same package ... I understand that "package" isn't a concept known to the language spec

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.

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.

[yjbanov]

I think we'll need to gather better evidence that sealed alone has enough performance benefits for performance to be listed as one of the issues that will be solved by this proposal.

Classes end up on the heap, and sealed does not change that. There will still be allocation and GC cost. Nor does it eliminate nullability, so there's still cost from null checks preceding dereferencing. Where sealed helps is with monomorphising access to the said objects after allocation and the null check. IOW, there would have to be enough non-null access to the object to offset the cost of all of the other stuff that class instances bring with them. ThemeData might be one of those cases where monomorphism matters because we access a lot of properties after dereferencing the object (this class has 40 properties). Other classes may not benefit so much just from sealing them.

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 sealed, they also need to be non-null, and have known size. For example, last time I checked Flutter apps ended up with a lot of _Double classes on the heap. This is despite the fact that doubles are already sealed. We should look at other small objects with value semantics that we might be allocating a lot: Size, Offset, Duration, Color, int64, small strings, small arrays.

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 sealed is going to participate in it, then we would either have evidence that it actually improves performance, or we would have a larger performance improvement proposal where sealed is shown to play a role (even if it's just a stepping stone and not a full solution). Good performance is rarely achieved by accident.

@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 Size class for one little widget can slow down unrelated parts of the framework. If the end user experience has higher priority than developer convenience, it is reasonable to restrict what the developer can do to make the app faster (if we have good reasons to believe that it actually does improve performance).

[MichaelRFairhurst]

"rules for thee, and not for me" don't work out well in my experience, and are very frustrating to work around

This is exactly what

the class can only be sub-classed within the file where it is declared

constitutes, though.

I want to empower developers:

• empower them to label their APIs as "not intended for extension/implementation"
• empower them to find, and correct, usages of these APIs when they come up
• empower them to create proper version constraints when they must use a package in a way the package author didn't want it to be used
• empower package authors to make changes freely even if some users didn't follow the restrictions
• empower compiler authors to get the performance benefit when users write ideal code

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.

[matanlurey]

@bwilkerson:

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.

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.

@yjbanov:

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...

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 sealed, not "sealed except for X") - i.e. a conflicting requirement with what @bwilkerson and @MichaelRFairhurst are suggesting so far.

@MichaelRFairhurst:

the class can only be sub-classed within the file where it is declared

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:

• Tests (test/) are in a different library than the lib/ code.
• Mocks (i.e. mockito) will never be in the same library as lib/ code.
• Conditional imports will never be in the same library as lib/ code:

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:

• @sealed
• @sealedExceptForTesting
• @sealedExceptForMocking
• @sealedOutsideOfSameLibrary
• @sealedOutsideOfSamePackage

... so starting with the most restrictive sub-set (i.e. "pure" @sealed) makes the most sense today. It can safely be (a) not used or (b) ignored via the analyzer until we have more data, usage statistics, and hopefully an answer to what unit(s) of code Dart:Next wants to consider significant for modularity and visibility restrictions (file, library, package, target, maybe even some new module concept).

[Hixie]

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.

[MichaelRFairhurst]

the class can only be sub-classed within the file where it is declared

I don't think its a good idea to block the restricted form of this analysis feature on those requirements, though

I should be clear that both suffer from this.

I can mark my code @sealed, and at any moment I feel like it I can unmark it @sealed without forking the library.

This is way more power that an author holds, than being able to override something in a the file its defined in.

so starting with the most restrictive sub-set (i.e. "pure" @sealed) makes the most sense today. It can safely be (a) not used or (b) ignored via the analyzer until we have more data, usage statistics, and hopefully an answer to what unit(s) of code Dart:Next wants to consider significant for modularity and visibility restrictions

I agree here -- if its something you can // ignore, and if you can run code that does so, then it will be a great way of accumulating data about how this feature is used.

Ultimately, all sides of this are trying to mitigate risk -- risk of people @sealing things that would be better not @sealed, and risk of overriding @sealed where it makes no sense, meaning we lose other benefits to enable nonsensical patterns.

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 // ignore will be people who are not publishing their code. It's a workflow that only makes sense for apps, not libraries, and apps get published less often, are more likely to be proprietary. So maybe we should see if we can collect anonymous analytics on how often its // ignored, or reach out to people with closed source dart codebases before pulling the trigger.

[leafpetersen]

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.

[matanlurey]

I have opened https://github.com/dart-lang/sdk/issues/34135 to try to avoid talking too much about visibility/libraries/packages here. Hopefully interested folks can add their feedback to that thread instead.

[matanlurey]

@leafpetersen:

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.

Right, my understanding from talking to Dart2JS and the VM team is that @Hixie's suggestion of:

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.

... 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.

[bwilkerson]

so starting with the most restrictive sub-set (i.e. "pure" @Sealed) makes the most sense today. It can safely be (a) not used or (b) ignored via the analyzer until we have more data, usage statistics, and hopefully an answer to what unit(s) of code Dart:Next wants to consider significant for modularity and visibility restrictions

I agree here -- if its something you can // ignore, and if you can run code that does so, then it will be a great way of accumulating data about how this feature is used.

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.

[yjbanov]

@Hixie

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.

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.

[MichaelRFairhurst]

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.

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 @sealed for performance benefit.

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.

[munificent]

Long thread is long! I want to reply to one point:

we are suggesting the @sealed annotation as an analysis-only experiment,

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?

[leafpetersen]

I'd really like to avoid getting too derailed on the performance issue.

• We have non-closed world scenarios we need to support
• JITing, whole program compilation, modular AOT, link time optimization, interpreting, etc are all on the table, and all have their tradeoffs
• We have top people Top People! working on making things fast
• We believe that our Top People! could make use of this feature which is independently requested by a number of our customers for non-performance related reasons, for performance, and would like to experimentally validate this.
• Some kind of feature in this space is a request from several important platform customers for non-performance related reasons

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.

[MichaelRFairhurst]

We have top people Top People! working on making things fast

Thanks @leafpetersen, I am skeptical of almost every reason to do this, but I like experiments and appreciate the reminder that we have experts on all sides of this.

I also want to point out how we have experts advising against this. But usually I'm on the camp of supporting experimental work, so I should stop expressing dissentment I've already expressed, and let the coldness of experiment take it from here. :)

[matanlurey]

@leafpetersen Can some of said Top People! perhaps leave input on what conditions they would need for the optimizations they seek? Otherwise I'm tempted to just say "allow library-based inheritance" in order to get over to skip spending the rest of the quarter debating semantics :)

[mraleph]

When I was asked about @sealed I said the following:

I don't think that @sealed would help either pure-JIT peak performance or pure-AOT performance (assuming that the program itself is written in a reasonable way) because it does not communicate additional information to the compilers in these modes: JIT can optimistically assume that classes are @sealed, and AOT has global view of the program so it knows precise class hierarchy information.

However @sealed might be helpful in the modular compilation environments where we combine AOT compilation and dynamic code loading (e.g. enable our AOT to produce shared libraries or have AOT+interpreter combination) and can neither assume closed world, nor have any static linking phase, nor have access to dynamic optimizations. In this sort of environments AOT has to assume the worst - and have classes explicitly marked as @sealed would help it to not assume the worst about sealed class hierarchies.

[Language like Swift exists in such environment - I don't think it is completely accidental that it also seals the classes by default (though I don't necessarily agree with ergonomics of this - but that's completely separate question from the performance)]

Additional considerations:

1. For JIT mode there is always a question of warm up, performance stability and memory usage by the metadata/feedback colleciton associated with JIT compilation. @sealed might help here: if you know ahead of time which classes are sealed you can potentially do some devirtualization very early (basically in front-end) making unoptimized code faster and saving on other things (e.g. you no longer need to track type feedback in devirtualized calls and no longer need to register dependencies on your assumptions - because you have static guarantees). That said even with @sealed annotation JIT most likely would continue to track and use "dynamic sealedness" (e.g. which subclasses of a class are were actually instantiated).
2. For pure interpreted mode similar considerations apply - @sealed enables preoptimization of bytecode, leading to faster initial execution.

Note that assessment above is given under hypothetical best case scenario - performance gains might be very modest in the real world scenarios. However, we should take into account that performance is often a game of small wins that later all combine together into a large win.

[lrhn]

I'm not going to argue about performance, other people do that better. Or about the annotation, this is a language discussion, annotations like this are not part of the language, and you can program without them entirely. I do.

I am going to talk about usability, though, because that's at least as important for language features as it is for library design.

If we provide a feature to make a class sealed, then it's important to give users guidance on when they should do so, and when they shouldn't. Such guide-lines should be usable for all users who are creating classes. Every time any Dart programmer creates a class, they will think "should this be sealed?", and the answer should be simple to find with the information they have available.

The full answer is, summarizing for comments above, that you should seal a class

1. If you haven't planned for, or thought of, someone else extending or implementing it,
2. If you want to be prepared for making breaking changing the class in the future, or
3. If you want to ensure that the class performs well.

Making a class sealed later is a breaking change, so if you think you might want to make a class sealed in the future, you should make it sealed now (item 2).

I doubt anyone can say no to all three of these, so the guideline will be "always seal", unless you have already actively decided that your class is intended for subclassing. You can always open up the class later if you need to.

It's a bad feature design if the default behavior requires you to write more than the non-default behavior, or where you automatically do the wrong thing if you don't know about the feature (that is, if you write nothing). Swift and Kotlin's "sealed-by-default" is really the correct design if these are the reasons to seal the class.

On the other hand, that choice has also shown some disadvantages (like Kotlin's interaction with Java code that assumes otherwise: Open your classes and method in Kotlin, Never say final - mocking Kotlin classes in unit tests, How to mock final classes in Kotlin with Mockito 2).

This exemplifies one of the consequences of sealing - you cannot mock the class, or wrap it to add functionality. You have to jump through some hoops to do it in Java, which uses custom class loaders and/or reflection to remove the finality. Will we also need a way to side-step sealing, perhaps for testing? (If so, we have a problem, because I very much don't want to provide one - sealed means sealed!)

If we assume that most new classes will be sealed, how will that change the Dart programming environment? Nobody knows for sure, so we should look at other languages who are in that situation. People weren't universally positive about Swift making classes closed by default. It's good for the person sealing it, not so good for the person needing to re-implement or extend the class to get their job done. (Again, the incentive for the author is to seal as much as possible because they don't know, and cannot know, all the uses that will be made of their class).

tl;dr: A sealed class feature supporting the provided use-cases should make classes sealed by default, and give an option to open them. Making sealed opt-in while also designing it so that the incentive is to seal as much as possible, is just not good design.

[eernstg]

tl;dr   'Most restrictive' and 'no restrictions in same library' are incomparable: Let's do the latter.   rd;lt

Terms

For brevity and clarity, let's call the most restrictive model final classes (where a sealed class cannot have subtypes anywhere, of any kind: extends, with, implements) and the relaxation library-sealed classes (where all kinds of subtypes are allowed in the same library, but prohibited in other libraries).

Change the meaning of sealed?

It's been said several times in this discussion that we can let sealed mean 'final class' at first, and later we can switch over and make it mean 'library-sealed class'.

I think this is a dangerous path to take, because those two mechanisms are useful for different reasons. They will be applied in different locations, and hence we will not be able to make that switch after all: It will be a seriously breaking change.

Reasons for not changing the meaning

Where would final classes be used?: Only a leaf in the subtype graph can be final, so developers would use sealed on the bottommost classes in their hierarchies, probably in order to get better performance (this will work, and you basically can't use it for any other purpose, because realistic, reusable code will use supertypes of those leaf types, and they are not protected against being subtyped in other libraries so they would still have to be written to support arbitrary subtyping, or rely on "Don't subclass!" comments).

Where would library-sealed classes be used?: A very typical choice would be to seal the top of a subtype graph, e.g., we could seal AstNode in a compiler. With that, it would be guaranteed that nothing of type AstNode or any of its subtypes could be declared outside this library, so developers have the guarantees that were used to motivate this feature in the first place ("no need to live up to the standards required for arbitrary subtyping"); also, we can optimize and check exhaustiveness in switch-like constructs (pattern matching), etc.

In other words, the entire mindset about sealed would have to change, and the choice of which classes to seal would probably have to be re-done from scratch, in every piece of code out there which uses it.

On top of that, it's a safe bet to rely on 'library-sealed classes' in the sense that it is able to emulate 'final classes': If we make a leaf class sealed, it's just like a final class (except that it doesn't prevent subtypes in the same library, but that's much easier to prevent or fix than having arbitrary subtypes "out there").

Recommendation

Based on that, I'd recommend that we go directly for the most useful variant, namely library-sealed classes.

[matanlurey]

@eernstg: Fine with me.

As noted in https://github.com/dart-lang/language/issues/11#issuecomment-412658558, I imagine we will hit a wall very fast if we stick with the concept of "libraries" for visibility, but I'm willing to start there. We should consider though that if that is our intention, users are likely to complain:

... I think even your suggestion of sealed except for same library will not be enough...

Consider:

• Tests (test/) are in a different library than the lib/ code.
• Mocks (i.e. mockito) will never be in the same library as lib/ code.
• Conditional imports will never be in the same library as lib/ code:

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;
}

[munificent]

I fear we've thrown Sam in the deep end by asking him to write a proposal for contentious feature. :(

I'd like to drive this discussion towards something actionable so I'm going to try to see if there are topics we can take off the table that don't need to be discussed (yet). Unless you believe any of the following should be discussed before Sam's proposal, let silence be your assent:

• Whether or not sealed should be added to the language. As I understand it, Sam's proposal is for doing an experiment, not shipping a language feature. As such, whether or not it's a good feature is beside the point. We'll decide that after the experiment, taking the data from the experiment into account. All we need to decide now is if it's a good experiment.

• Whether sealed will help with performance. We have enough people saying it might that it's reasonable to go ahead with the experiment. We'll have more data on whether it will or not based on the results of that experiment.

• Whether classes should be sealed by default or not. This is a good discussion to have if the experiment shows that sealing turns out to be useful enough to add at all. For the experiment, I don't think we have any practical way of making the default be sealed, so I think we can defer this until after the experiment.

There are a couple of topics I do think we should discuss to unblock Sam:

• What questions is the experiment trying to answer? The proposal should be clear about that. In other words, the proposal should sell the experiment, not the feature.

• What is the scope of a sealed class? If the experiment is trying to get usability data on how useful the feature is and how often it will/can/should be used, then it should probably have the scope we intend to ship.

  If this is one of the questions we're trying to answer by doing the experiment, that's fine. In that case, pick a reasonable first choice and make it clear in the proposal that it's tentative. Again, we should be clear about how we plan to evaluate the results of the experiment to answer this question.

Does that sound reasonable?

[eernstg]

... I think even your suggestion of sealed except for same library will not be enough...

I would expect libraries as the unit of encapsulation to match any reasonable notion of modular compilation pretty well. It's of course tempting to ask for all kinds of other units (packages etc), but that's not currently well-supported by the language as such.

But the reference to tests and mocking brings up other topics that we've touched upon before, e.g., access to private features in unit tests.

I think it might be more useful to allow a Dart source file 'my_test.dart' to declare that it is a part of a given library L without having the reverse part 'my_test.dart' link in L. Such an "undeclared part" would have access to private names in L, it could contain main, and it would only be included when specified as the entry point to the analyzer/vm/etc or when imported from another undeclared part.

We might then be quite OK using libraries as the unit of encapsulation, both with respect to mocking and private access and sealing.

[yjbanov]

@munificent

Does that sound reasonable?

I think this is the most reasonable comment on this thread so far :smile:

What questions is the experiment trying to answer?

If the experiment plans to make a performance argument, then the question I would ask is:

What kind of code would benefit from sealed's performance characteristics and by how much? The tricky thing with sealed is that just introducing the keyword in the language alone will not have any performance effect on any existing code. Worse, applying the keyword on existing classes may not demonstrate anything either. Many existing classes may already be de facto sealed and recognized as such by the existing compilers. I think here we want to show examples in important applications (e.g. Flutter) where introducing polymorphism that would have been prevented by sealed degrades performance. A few concrete benchmarks could strengthen the argument further.

[MichaelRFairhurst]

I think here we want to show examples in important applications (e.g. Flutter) where introducing polymorphism that would have been prevented by sealed degrades performance

I think this is reasonable.

I think its also reasonable to get some experiment, so that if we add dynamic linking (in, most likely, another experiment):

@sealed might be helpful in the modular compilation environments where we combine AOT compilation and dynamic code loading (e.g. enable our AOT to produce shared libraries or have AOT+interpreter combination) and can neither assume closed world, nor have any static linking phase, nor have access to dynamic optimizations.

So that we aren't caught with our pants down by severe performance regressions, but instead have a plan, if this scenario does indeed unfold later.

This is not the same as having a reason to land it pre-emptively (preoptimization and all that), but, it seems smart to stay ahead of this curve by getting an experiment preemptively.

What questions are we trying to answer

I'm personally not opinionated on package level restrictions vs final classes. I won't use either. :)

However, regarding other usability issues, we have a number of cases, that can be put into a table:

Did the author design for subclassing? Is the class suitable for subclassing in a particular scenario? (these are different: the design may be poor, may just work out of the box) Does the client have a reason to subclass?

We get 8 scenarios. How likely are these scenarios? Do we expect the likelihood of the scenarios to change based on the availability of sealed classes? How are these scenarios helped/hindered by the existence of sealed classes?

No, No, No: Perhaps improved performance if sealed is completely unsealable Yes, No, No: No effect Yes, No, Yes: No effect -- bad subclassing still occurs and is still bad Yes, Yes, No: No effect Yes, Yes, Yes: No effect No, No, Yes: Good, catch users before they bang their head on a wall trying to get something to work that won't. Encourage discussion with library author, or forking the library in a worst case. No, Yes, Yes: I'm going to break this one down further:

• if sealed is completely unsealable, this is pretty much just bad. The lib does what you want but you can't do it because the author didn't trust you. However, in g3, sealed is good here, because extending the class anyway impedes the ability of the author to keep rolling changes. And you can easily send a CL to unseal it.
• if sealed is unsealable, then its good. Get warned you need to be careful and test thoroughly. Introduce version lock to be safe. Such version lock may make a lib more or less unpublishable, but careful testing could make for libs with a wider range than a single version.

It's notable that

• the "no, no, no" case is probably the most common.
• The more difficult it is to make properly subclassable classes, the more common "Yes, No, Yes" occurs. But the availability of sealing has no effect here.
• Mocking is arguably "No, Yes, Yes", and if so, is extremely common. Though it could be treated specially.
• I don't see any worsened cases for google3 users
• I don't see any worsened cases for external users if unsealing is possible.

This can inform some experiments:

• If unsealing is impossible, this can present performance gains. So this is a choice that can be experimentally guided. How big of performance gains? In what contexts?
• If unsealing is not possible, then usability is a contention is between "No, Yes, Yes" and "No, No, Yes." Can we discover (with or without writing an experimental feature) how common these cases are?

I think so long as we

• make classes unsealable
• have --strict runtimes which don't let you unseal classes. We would use this for google3. And potentially get performance boosts later.
• have no other places that force --strict (this means that in hypothetical future environments that have shared dynamically linked libraries, you could statically link in a pinch)

then, as far as these cases go, we no longer have any downsides for anyone. Do we need to experiment? Can we simply design the feature?

But as I said earlier, package-level restrictions vs final classes is not something I weighed into. Other more opinionated people on that topic, any questions we need answered there?

[srawlins]

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 @sealed, an experimental annotation (it was originally a language issue, but I've retreated). I've updated the main description to respond to many issues raised. In particular:

• I changed the feature to include library-level sub-classes, after @eernstg argued that this version makes the most sense to implement first. @matanlurey does this conflict with Angular Dart's request?
• Since this is just a request for an experimental feature, primarily from the Angular Dart team, I don't think that comments about whether your project would use sealed classes adds to the discussion.
• In "Why a Hint?" I address @MichaelRFairhurst's request that this not be an error, and his request that this be ignorable.
• In "Alternative definitions -> Package-level sub-classes" I address the request by @bwilkerson that we allow package-level sub-classes.
• In "Cost of rolling back the experiment," I address @munificent's concern that "experimental" features haven't had a great track record for paths to success.

(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:

@MichaelRFairhurst wrote

[If it's an error,] what inevitably happens is people fork libraries and change the class to be unsealed. What benefit does that serve anyone?

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, ...?

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).

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.

I will test that my own integration works, and if other cases aren't tested, that doesn't matter to me.

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.

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.

This only works in versioned vendor environments.

(maybe even a pre-publish warning, "^x.x.x" is an unsafe constraint because you extend a sealed class).

This language feature (or experimental annotation) does not warrant such an expensive feature for pub.

Sometimes you gotta let you kids fall down, and that's ok!

This is not a feature for the "kids;" it's a feature for the "parents."

@bwilkerson wrote:

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?

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."

@MichaelRFairhurst wrote

I want to empower developers:

• empower them to create proper version constraints when they must use a package in a way the package author didn't want it to be used
• empower package authors to make changes freely even if some users didn't follow the restrictions

If you have such a proposal that works in non-versioned vender code repositories (google3), this would be interesting.

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 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.

@bwilkerson wrote:

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.

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.

@lrhn wrote:

I doubt anyone can say no to all three of these, so the guideline will be "always seal", unless you have already actively decided that your class is intended for subclassing. You can always open up the class later if you need to.

It's a bad feature design if the default behavior requires you to write more than the non-default behavior, or where you automatically do the wrong thing if you don't know about the feature (that is, if you write nothing). Swift and Kotlin's "sealed-by-default" is really the correct design if these are the reasons to seal the class.

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.

@MichaelRFairhurst wrote:

I think so long as we ... make classes unsealable ... then, as far as these cases go, we no longer have any downsides for anyone. Do we need to experiment?

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.

But as I said earlier, package-level restrictions vs final classes is not something I weighed into. Other more opinionated people on that topic, any questions we need answered there?

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.

[bwilkerson]

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?

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."

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:

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.

If these are the questions we're trying to answer, then I think we need to clarify them a bit. For example:

• Who are the "authors" you refer to? Is it just the angular framework authors, or is there a broader audience?

• How do we answer the question "how much authors will use this"? Is this just a count of the number of times the annotation appears in code? Do we also want to know how often authors would have used this but couldn't because it's too restricted?

• How do we answer the question of "how often authors want package-level sub-classes"?

• How do we answer the question of "how often users fork libraries in order to sub-class"? (I'm skeptical that we can answer that, because I don't think we'll have visibility of the code in which the forks were created.)

• For the performance question I assume that some backend team needs to implement the optimizations based on this. Which team is going to do that work? And if it proves to be valuable from a performance perspective, will more than one backend team eventually implement this feature?

The primary use case is to remove burden from, and give assurance to, authors of public classes.

I don't think that comments about whether your project would use sealed classes adds to the discussion.

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).

[srawlins]

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.

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?

Who are the "authors" you refer to? Is it just the angular framework authors, or is there a broader audience?

How about Angular, Analyzer, Sass, Flutter, and vocal people, e.g. on GitHub, mailing lists, chats, ...

How do we answer the question "how much authors will use this"? Is this just a count of the number of times the annotation appears in code? Do we also want to know how often authors would have used this but couldn't because it's too restricted? How do we answer the question of "how often authors want package-level sub-classes"? How do we answer the question of "how often users fork libraries in order to sub-class"? (I'm skeptical that we can answer that, because I don't think we'll have visibility of the code in which the forks were created.)

If there are previous examples of metrics around a language feature, I'd love to use those to construct a metric. If not, the Dart team has created a beautiful language with beautiful features by perhaps previous experience, "feel," community outreach, and I'm happy to continue that tradition. Edit: Sorry, this was snarky. Basically community feedback is my answer.

For the performance question I assume that some backend team needs to implement the optimizations based on this. Which team is going to do that work?

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.

And if it proves to be valuable from a performance perspective, will more than one backend team eventually implement this feature?

They might.

Perhaps I'm wrong, but these appear to me to be inconsistent statements (unless "authors" is taken very narrowly).

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 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.

I disagree. I've outlined an experimental @sealed annotation above that allows some measuring of both questions. It doesn't allow for package-level sub-classes experimenting, so it is imperfect. @matanlurey wrote that we want to avoid @sealed, @sealedExceptForTesting, @sealedExceptForMocking, @sealedOutsideOfSameLibrary, @sealedOutsideOfSamePackage, but I suppose we could. We could have an additional @sealedOutsideOfSamePackage experimental annotation and observe how that is used.

[bwilkerson]

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?

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).

1. Are sealed classes useful when restricted to same-library sub-classes (and not allowing same-package sub-classes)?

In the sense of "do we have any users that want this feature as defined", I think the answer is clearly "yes".

3. Do public class authors abuse sealed classes?

I'm not sure what "abuse" means in this context.

4. Do users // ignore the "sealed classes" Hints?
5. Do users fork sealed classes in order to unseal them?

Do we have access to enough user-written code to know whether these are happening?

6. Is the "sealed" concept useful as the single great class modifier affecting sub-classing?

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 don't see other responses of "I won't use it" as interesting.

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.

I disagree. I've outlined an experimental @sealed annotation above that allows some measuring of both questions.

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.

[srawlins]

3. Do public class authors abuse sealed classes?

I'm not sure what "abuse" means in this context.

Haha, one man's "abuse" is another man's "defensive programming." I guess this is coupled with the next two questions:

4. Do users // ignore the "sealed classes" Hints?
5. Do users fork sealed classes in order to unseal them?

Do we have access to enough user-written code to know whether these are happening?

My answer is just, community feedback. We can poke around internally as well. For example, if we see a greenfield class being written with @sealed, just to be protective, I think @MichaelRFairhurst would see that as abuse. @lrhn brings up points to the opposite view though. Maybe opinions will be changed as well during the experiment.

Is the "sealed" concept useful as the single great class modifier affecting sub-classing?

We don't need an experiment to answer that question.

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 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.

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.

The performance goal is limiting the definition of the annotating classes goal.

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:

1. package-level sub-classing is beloved by all => we want to make it a language feature => we must reduce to library-level sub-classing => some authors cannot replace @sealed to sealed.
2. package-level sub-classing is beloved by all => we want to make it a language feature, as is => we must add package concepts to the language => performance optimizations are prohibited, unless we also get package-level kernel or package-level compilation units
3. package-level sub-classing is beloved by all => we want to make it a language feature, as is => we must add undeclared-parts or friend-libraries to the language => performance optimizations might still be prohibited.

[bwilkerson]

... some are of the opinion that even just the ability will be a net loss to the Dart language and community.

Well, they're just wrong! :-)

It would be stuck as an unenforceable annotation.

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 pragma rather than define an annotation in package:meta in order to run the performance experiment.)

[matanlurey]

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 @protected or @required?)

If anyone has any other serious objections please talk to Sam or Leaf directly.

[MichaelRFairhurst]

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.

@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 @sealed annotation, users could suddenly add members much more freely, and release it as a minor version update. If users unsealed a class, but didn't pin, now when they release it trickles through the community as a bug.

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.

[srawlins]

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.

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, @packageSealed. There isn't a lot of precedent for this, but I can imagine a @packagePrivate as well. Others?

@packageSealed would address some of the points @matanlurey has raised about library-level sub-classing: tests within the package could sub-class, and configurable imports would be allowed to sub-class (within a package of course). @packageSealed classes would not be open to mocking/faking/stubbing in user-land tests, but they could bulldoze through with an // ignore: invalid_use_of_package_sealed_class comment.

OK. What's the path forward?

1. Authors use @sealed and @packageSealed as they see fit. They are both only enforced by analyzer Hints. We can enforce them internally on day 1. An experiment can proceed at leisure for performance benefits of trusting @sealed.
2. If @sealed is not used ¹, we eventually drop it ². Below, assume it's used.
3. If @packageSealed is not used ¹, we eventually drop it ². Below, assume it's used.
4. If the performance experiment yields "meh" or no optimizations, we drop "performance" as a benefit. Below, assume a benefit can be made in some "important" environment ³.
5. OK now we're in a weird spot. @sealed is beloved by all, @packageSealed is beloved by all. @sealed is beloved by some back-end. What if at this point, assuming "package" concepts have not entered into the spec, we proceed with a proposal to make "sealed classes" a language feature, using the library-level sub-classes definition (following a hearty discussion of course), and leave @packageSealed an annotation, enforced with an analyzer Hint, and enforced internally? If "package" concepts have entered the spec, and we take the performance idea into account, we could expand the "sealed classes" language feature to allow for package-level sub-classes?

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 @sealed and @packageSealed.

¹ meaning by Angular, Analyzer, Sass, Flutter, and vocal people, e.g. on GitHub, mailing lists, chats, ...

² "when" is TBD.

³ "what is 'important'" is TBD.

[matanlurey]

I'm certainly not objecting to additional experimental annotations (certainly most of the annotations in package:meta have had no formal design review, or least nothing close to the "open comment" of this thread).

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 @visibleForTesting.

[srawlins]

and I'm concerned internal to Google it will be very confusing to explain the difference between "library" and "package" to most users

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 dart_library build targets, each of which can contain multiple Dart libraries, and which have no correlation to Dart packages. :frowning_face: )

We can try to be specific in the messaging:

• "The Foo class is sealed. It cannot be extended by, implemented by, or mixed into MyMockFoo, a class declared outside of the "package:something/src/foo.dart" library."
• "The Foo class is package-sealed. It cannot be extended by, implemented by, or mixed into MyMockFoo, a class declared outside of the "something" Dart package."

Eh?