PRE-SIP: Make classes `sealed` by default

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.

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

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

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.

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

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.

1 Like

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

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

My preference would be for the language flag to give the warning or error, but no warning or error with no language flag. So there would only be disruption for those that wanted it.

The whole point is to prevent something that could happen just in case either the developer forgets to mark the class as final/sealed or the user extends something that is not meant to be extended by mistake. This can only be done by requiring the flag. IMO, a language flag is no disruption.

4 Likes

Let’s not forget that you can add -language:adhocExtensions to compiler options and thus silence all warnings about extending non-open non-final classes. Adding -language:xxx to compiler options is equivalent to adding import scala.language.xxx to all .scala files in your project.

3 Likes

@soronpo Once again, this is an inclusion of a core feature to the language which dictates a controversial opinion (among the “contributors”, mind you). This is not just the flag, it’s also the documentation that has changed in the PR, which now contains a part about the motivation behind the flag and instructions on best-practices, which are being argued against in this very discussion and the previous (quite long) discussion.

Hi Martin & the Scala community,

I have to confess that I’m not that close the details of the Scala language design, but I do have a fear of whether Scala 3 is exhibiting second system syndrome. I think that Dotty/Scala 3 has some excellent new features that will help developers. But it also seems like some very significant language changes are being made right before the Scala 3 cutoff deadline. In my experience, rushing stuff in just before the deadline is usually a mistake …

In particular, the changes to implicit syntax, significant whitespace, and class scope changes, may end up not being the right choice in the long term.

I think that if Scala 3 ends up being too different from Scala 2 then you will end up with exactly the same issue as Python 2/3, which is still being worked out today.

If a particular feature is controversial then I would suggest that it is may be safer to leave it out of Scala 3, continue working on it until it is shown to be definitely better before adding it to a Scala 4. I don’t think that there is any reason why Scala 4 couldn’t be released in 2-3 years time, if that was the right thing to do.

Kind regards,
Rob

5 Likes

Most java’s best practices suggest to use final classes (Effective java, official concurrency tutorial). FP also favors this approach.

OTOH, many popular java libs/frameworks like JPA, Jackson, Spring require nonfinal, mutable POJOs. In Kotlin land they have the “allopen” plugin to open certain classes by default…
But there are not many Scala users that use those libs.

So, all in all, it is natural/preferrable to have classes final as a default. But, it’s a dangerous decision to make IMHO. I couldn’t imagine e.g. Java to switch to final classes by default… Maybe for case classes only, but regular ones…

2 Likes