SIP: Make classes `sealed` by default

@odersky can we make case classes (and case objects too) genuinely final by default? since we now have the open keyword, it (along with sealed) could be used as an escape hatch.

// Scala 3
case class A(x: Int)  // final
sealed case class B(x: Int)  // can be extended, but only in the same compilation unit
open case class C(x: Int)  // can be extended anywhere
final case class D(x: Int)  // `final` is redundant here
8 Likes

Most concerningly it forces library authors (if they choose to legitimately use class ) to be precisely perfect in predicting how all consumers will use their library, or they will just spam the “open” keyword everywhere which then means its the wrong default.

Most libraries I use (which, admittedly are on the FP-side of Scala – but I found the same e.g. in Akka) obsessively spam final class everywhere, which to me indicates that open is the wrong default, at least for the authors of these libraries!

9 Likes

No final will be generated in the bytecode. But I don’t think it matters for performance.

final affects inliner decisions, inliner skips all non-final methods and classes

1 Like

The JVM can inline after call site profiling data indicates it is likely monomorphic (inline with uncommon trap for unexpected class) or bimorphic (with branch and uncommon trap), regardless of final status, but it also depends on method bytecode size and some other heuristics.

It is true that it may inline small final or private methods earlier and more aggressively. The most common case is private accessors and small private helper methods.

For most method calls that are not private, avoiding megamorphism at the call site is a bigger performance boost than inlining.

4 Likes

I write a library whose API is pretty object-oriented, and even there I spam final class everywhere. With a few exceptions for rare classes that are carefully designed, documented and tested for extension.

For library authors, final is clearly the right default IMO.

5 Likes

Scala has its own compile-time inliner enabled with opt:l:inline - it can only inline marked final methods unfortunately, because it can’t tell statically if a class is “effectively-final”

Yes, which is very important for hot megamorphic call sites like many higher order functions where there is only one call site for the jvm to profile (e.g. the function application inside a map method). For common OOP style stuff, the calls are spread over many call sites and the ability of scalac/dotty to inline is much less important.

final in bytecode might impact the optimizations performed by Android’s dexopt optimizer and dex2oat compiler, GraalVM’s native-image compiler, IBM J9’s AOT compiler, JEP-295’s jaotc, and Excelsior JET (unfortunately discontinued this year).

2 Likes

I believe this to be incorrect. (1) and (3) are quite distinguishable. In the case of (1), an author would most likely use either a trait or an abstract class, where’s in the case of (3) an author would not.

The way I see it, when authors intend for users to extend a class (1), they doucment it – either in code comments (scaldoc) or in an online resource (README, site, etc). Annotation are also a way to imply intention (assuming they are documented).

This is the crux of the problem with this SIP. It will change the way developers interact with third party libraries. I believe that in most cases, (3) is the state of affairs, which is actually a good thing, as authors cannot accurately predict the way their library will be used, and they often can rely on users to explore new ways of interacting with their library.

This is also why I’m against having a warning when users extend “simple” classes. In my opinion, this is not actually a risk, but a common use-case. I mean, using a third-party library is always a risk in itself, should we warn on that?

If you are using a third-party library in any way then you are tightly coupled with it by definition, and you have no real control over its development, which may break in the future. This is the nature of development and out-sourcing your work someplace else.

Perhaps the only thing Scala should introduce is a new annotation which indicates that a class / trait is intended to be extended. open should be @Open.

1 Like

Status update: https://github.com/lampepfl/dotty/pull/7471 has been merged into the Dotty codebase. As is the case for all other new features it will go through the SIP process and undergo a public review.

This modifier, and more importantly, the associated warning that comes with it, just complicates the language and alienates it from new users, I believe. Imagine how developers are going to argue whether a class should be open or not, where in fact their project is not a library and not open-sourced. They will basically argue over nothing and waste time. Make it an annotation, not a compiler feature.

1 Like

I don’t understand your argument. Currently developers can make their classes final. So the argument (if exists) has not changed. All this does is make it easier for developers to open only what they explicitly wanted to open, instead of final-ing what they explicitly wanted to be final.

4 Likes

The more features and keywords a language has, the more choices one has to make, and the more discussion it may lead to. True, the argument over whether a class should be final or not in private repositories is very similar and may be redundant in many cases, but that doesn’t mean we should add more discussions like this.

I don’t see why an @Open annotation is not enough, and why do we need a compiler error or warning for extending non-explicit “open” classes. This is very confusing to see this type of warning for new developers, especially for Java newcomers.

1 Like

I consider this as a good warning. New developers have no business extending classes that are meant to be closed. It’s like a safety on a gun. It doesn’t prevent you from using the gun, but causes you to take one more explicit action before shooting yourself in the foot.

3 Likes

Classes that are meant to be closed are final or private[scope]. Users cannot extend them, period. The debate is whether a class which is not explicitly marked is intended to be extended, does not, or perhaps the author didn’t spend time thinking about it. I believe the latter is the case, and that this is a very legitimate state of affairs.

1 Like

I disagree. A good library tests its usable interfaces and that includes extending the open API classes. If something isn’t tested, it’s not known to work, and therefore shouldn’t be made available to users even if the author forgot to annotate it as final. I can easily forget to set a class as final, but not just as easily forget to make it open if I wrote a test for it.

3 Likes

There is already a long discussion over this point in the previous topic. The opinions go both ways, so there is no clear answer here. I’d rather the language core components refrain from being opinionated on such a controversial topic.

2 Likes

No I totally disagree with this. What constitutes a good library depends on its context. Yes ideally every library we use should be rigorously tested and extensively documented, but often one barely tested and minimally documented library is all that’s on offer. in that case users should know that there are risks attached to using such a library. Those risk will remain until a better competitor library appears, or the user finds the time and motivation to bring the existing library up to standard or write a library of their own.

In such situations, leaving classes open should definitely be the default. No one’s forcing the user to extend anything. I’d be happy with an open annotation, the compiler could then be directed to generate a warning or an error when extending classes from third party libraries that hadn’t been annotated as open. The user would then have the choice and could make their own judgement on the trade off that being strict imposed. Rather than some peoples preferences being forced on everyone for every project.

2 Likes

That’s exactly what this proposal’s PR does. It warns you and you can escape it using a language flag.

5 Likes

I can imagine that a SingleLinkedList class consisting of add and remove methods shouldn’t be extended to override the implementation in such a way that it isn’t a single linked list anymore.

Wouldn’t it be better in this case to provide a companion object containing the methods of the SingleLinkedList class.
In this way, a class consist only of signatures with(out) default methods which can be overriden by inheritance and which is intended to do so whereas method/function values which shouldn’t be overriden inhabitate the companion object as value which can’t be overriden.