Allow types to be Sealed by default

[charlesroddie]

Allow types to be sealed by default

F# programmers often Love creating classes but Mostly Avoid class inheritance. From F# code I love :

There should be setting which allows types to be sealed by default.

This type for example would be Sealed with the setting on:

type Vector2D (dx:double, dy:double) =
    let d2 = dx*dx+dy*dy
    member v.DX = dx
    member v.DY = dy
    member v.Length = sqrt d2
    member v.Scale(k) = Vector2D (dx*k, dy*k)

To make a type not sealed, a user should write [<Sealed(false)>].

The existing way of approaching this problem in F# is

1. Write [<Sealed>] on every non-struct class. Or more likely
2. Don't write this and a) allow consumers to inherit when this is not intended, and b) give less help to compilers and JIT resulting in lower performance at some point (e.g. more analysis of what is not inherited by AOT compilers, or lower performance from JIT https://www.meziantou.net/performance-benefits-of-sealed-class.htm ).

Pros and Cons

The advantages of making this adjustment to F# are ...

Like immutability by default, this makes it easier to create code which is easier to reason about by humans and compilers, guiding F# developers towards writing code this sort of code.

The disadvantages of making this adjustment to F# are ...

To avoid breaking changes, an extra setting is needed. Eventually we are going to have to have a setting like -nice which bundles all of the good settings together.

Extra information

Estimated cost (XS, S, M, L, XL, XXL):

M

Related suggestions: (put links to related suggestions here)

https://github.com/fsharp/fslang-suggestions/issues/397

Affidavit (please submit!)

Please tick these items by placing a cross in the box:

• [ x ] This is not a question (e.g. like one you might ask on StackOverflow) and I have searched StackOverflow for discussions of this issue
• [ x ] This is a language change and not purely a tooling change (e.g. compiler bug, editor support, warning/error messages, new warning, non-breaking optimisation) belonging to the compiler and tooling repository
• [ x ] This is not something which has obviously "already been decided" in previous versions of F#. If you're questioning a fundamental design decision that has obviously already been taken (e.g. "Make F# untyped") then please don't submit it
• [ x ] I have searched both open and closed suggestions on this site and believe this is not a duplicate

Please tick all that apply:

• [ x ] This is not a breaking change to the F# language design
• [ x ] I or my company would be willing to help implement and/or test this

For Readers

If you would like to see this issue implemented, please click the :+1: emoji on this issue. These counts are used to generally order the suggestions by engagement.

[cartermp]

Ahhhh, if only this were the default in F# 1.x (and this attribute, or maybe named Inheritable or something).

I think I'd probably not be in favor because it's an opaque change. I now need to know if the setting in a project file exists to read and understand F# class code.

[xperiandri]

This can be at least done for non-public types

[dsyme]

The intention is reasonable - though this is pretty much in the "already decided in F# 1.x" category - it was something we considered. Also, if all types were sealed by default, we'd need ways to mark them non-sealed. So I'd say best not to go there.

[charlesroddie]

@cartermp I now need to know if the setting in a project file exists to read and understand F# class code.

Let's say you are reading F# code and make a mistake about whether a type is Sealed or not. There are then two ways you could do this:

• You expect a type to be unsealed and it is sealed. This is actually the normal situation right now, where almost all types are not intended to be inherited from and are not inherited from but can be. Since no inheritance takes place, there is neither surprise nor failure to understand any code.
• You expect a type to be sealed and it is unsealed. This would happen if this feature is implemented and sees majority adoption so you assume things are sealed, but you are looking at a project with the setting off. A rare situation. Then you will be surprised when a type inherits from this one. Beyond this surprise, there is no failure to understand what the second type is doing.

So if you do not read project settings you will not face difficulties in understanding any F# code, beyond the rare surprise described above.

On the other hand if you do read project settings and see sealed by default you will be better able to understand code since you will see [<Sealed(false)>] indicating that a type is supposed to be inherited from, which does convey something interesting about how it is intended to be used.

@dsyme Also, if all types were sealed by default, we'd need ways to mark them non-sealed.

That is [<Sealed(false)>] isn't it?

So I'd say best not to go there.

If this feature is not implemented, do you have any suggestion how a codebase can ensure that all F#-defined types are marked as Sealed? Is this something for an Analyzer and would it be an external tool or an internal one or a user-defined setting?

[dsyme]

@charlesroddie I do agree with all of the above. Users of all levels below "expert" probably would make inaccurate assessments of whether a type is sealed. They would loosely learn quickly that record, unions, structs, basic primitives are sealed, and interfaces are "obviously" unsealed, but would be foggy about class types.

But I don't think users below "expert" are really helped by a project file setting, in balance.

If this feature is not implemented, do you have any suggestion how a codebase can ensure that all F#-defined types are marked as Sealed? Is this something for an Analyzer ...

It would seem a good use-case for an analyzer, forcing every type to be annotated.

That is [<Sealed(false)>] isn't it?

Yes, you're correct.

[OwnageIsMagic]

I now need to know if the setting in a project file exists to read and understand F# class code.

@cartermp @dsyme Can you elaborate a bit more on this in context of functional heavy type-inferred language with name shadowing, operator and function overloading, implicit conversions and AutoOpen. I often see this argument, so it really bothers me. You always needed to know contents of each file before this one (defined types, shadowing, overloads) and contents of project file (target framework, language version and references).

forcing every type to be annotated.

This doesn't aligns well with word succinct in every F# description.

Inheritance is already discouraged (i.e. no support for protected), so we can expect there is little to none types without any virtual member expecting to be inherited. This change will actually increase readability since presence of attribute highlights deviant behavior and raises awareness.

The only real pain from breaking change here is clash with dynamic proxies (i.e. Castle.Proxy, Moq, Foq, Entity Framework).

Regardless the performance opportunity: I actually encountered with that problem some years ago. There was hot message dispatch loop with a lot of casting and it showed up in perf traces with non minuscule % of exclusive time. Marking some classes sealed and eliding couple of interfaces (nice story about IEnumerbale) really helped to cut some ms. But it was in C#. F# is rarely about performance (I feel regret every time when I open F# code in ILSpy).

But I don't think users below "expert" are really helped by a project file setting, in balance.

C# enabled NRT with off-by-default, on-in-default-template flag. I think this a good approach, but this feature alone seems too minor for a dedicated flag

[cartermp]

You always needed to know contents of each file before this one (defined types, shadowing, overloads) and contents of project file (target framework, language version and references).

You absolutely do not need to do that for the vast majority of F# code out there. The problem is that by making this a default behavior driven by a switch in a project file, it's now implicit. Most other new things that require a switch are explicit, and so if I see it, I know it's turned on (or in this case, I'm working with a newer language version / target framework). Same with a new API call, if it came from a newer TFM, then great.

[vzarytovskii]

You always needed to know contents of each file before this one (defined types, shadowing, overloads) and contents of project file (target framework, language version and references).

You absolutely do not need to do that for the vast majority of F# code out there. The problem is that by making this a default behavior driven by a switch in a project file, it's now implicit. Most other new things that require a switch are explicit, and so if I see it, I know it's turned on (or in this case, I'm working with a newer language version / target framework). Same with a new API call, if it came from a newer TFM, then great.

To add to that, if we decided to change the behaviour and make it rely on switch, it would become impossible to tell what's the behaviour by just looking at the code.

[charlesroddie]

I don't think the above two comments are valid.

It is very common to adjust settings around what to warn on, and whether to treat warnings as errors, and then behaviour of code will change: whether it compiles or doesn't compile. Behaviour, conditional on compiling, is usually constant wrt. project settings (but not always e.g. invariantglobalization or reflectionfree). Hence the observation that code can be understood without reading project files:

if I see it, I know it's turned on

Similarly with this feature, by adjusting the project file you can change behaviour in that code may compile or not compile, but given that it compiles its behaviour is constant.

If I see inheritance take place then I know a class is unsealed without reading a project file.

[OwnageIsMagic]

impossible to tell what's the behaviour by just looking at the code.

That's what I'm talking about. Let's you have this file:

open X
open System
let d = DateTime.Now
printfn $"{ {Date = d} }"

What will this code output? Does it even compiles?

I can't really see how it differs from this:

// f1.fs
(* T1 defined here *)

// f2.fs
type T2() =
  inherit T1()
  do ()

Is T1 class sealed? Maybe, but the real question is 'Do I really care about it?'. Yes there some concerns when type is a part of public API and not used inside project as base but, as mentioned before there is already a problem: Users of all levels below "expert" probably would make inaccurate assessments of whether a type is sealed.

[vzarytovskii]

Similarly with this feature, by adjusting the project file you can change behaviour in that code may compile or not compile, but given that it compiles its behaviour is constant.

It's not only about that. It's about if we change the defaults, when you are making a nuget library, with old sdk it will export extensible types, with new one - not.

[OwnageIsMagic]

@vzarytovskii That's a 'breaking change' reason.

[vzarytovskii]

@vzarytovskii That's a 'breaking change' reason.

Sure, and we don't usually do this big breaking changes, hence all the concerns

[OwnageIsMagic]

As I mentioned before there is far more worse consequences like breaking Entity Framework, but what I am trying to say is that I now need to know if the setting in a project file exists to read and understand F# class code doesn't make much sense as a reject reason.

[cartermp]

I think it makes plenty of sense, for the reasons stated. Opaque changes are not generally good to make.

[vzarytovskii]

As I mentioned before there is far more worse consequences like breaking Entity Framework, but what I am trying to say is that I now need to know if the setting in a project file exists to read and understand F# class code doesn't make much sense as a reject reason.

Well, it kinda does, to a degree, when it comes to such a big behavioural change for both normal users, and more advanced ones and library authors.

The key thing here is "change", since all articles, docs and examples will be showing code which will not work in new sdk. Unless there's a flag passed somewhere which tells the compiler to fallback to the old behaviour.

[smoothdeveloper]

@charlesroddie, I think there is a distinction about F# types that aren't class (and are already sealed), and leveraging of the slide here is a bit biased.

If we'd follow that thinking, we could remove ability to define overloads if not adding [<AllowOverloads>] on the class, etc. after all, the slide puts "method overloading" in red too.

What we could have, is an optional warning, if a class has no virtual or abstract members, it could recommend setting it [<Sealed>].

warning FSxxxx: This class doesn't have any virtual or abstract member, consider setting it with [<Sealed>] if you intend to prevent it from being inherited.

I don't think I'd use such warning, for me, if using a class, anything goes, even the red parts of the slide.

We aren't preventing errors, but being very dogmatic about OO constructs if we'd consider making the suggestion a default for new SDK release.

[OwnageIsMagic]

@charlesroddie I agree with lang team, that breaking change is not a good solution. Alternatives are off-by-default noisy warning and opt-in mechanisms. For opt-in, this change seems to have too little impact to justify the creation of lang dialect.

[jwosty]

How about some way in-source to mark groups of things sealed-by-default, instead of a global setting as a compromise? For example, perhaps an attribute:

[<AutoSeal>]
module MyStuff =
    type MySealedType1 = class end
    type MySealedType2 = class end

An advantage of this approach is that some people may like the granularity of control:

[<AutoSeal>]
module MyModule1 =
    type MySealedType1 = class end
    type MySealedType2 = class end

module MyModule2 =
    type MyNonSealedType1 = class end
    type MyNonSealedType2 = class end

However it is a problem that there's currently no way to scope an attribute to a source file. This would have to be overcome, otherwise you couldn't use for classes in namespaces without a module. Perhaps with some kind of new "file" scoping (the rules of which would have to be designed carefully, of course)?

[<file: AutoSeal>]
namespace MyNamespace
open System
// ...

type SealedType1 = class end
type SealedType2 = class end

Alternatively, it could be a compiler directive:

#autoSeal enable

type MySealedType1 = class end
type MySealedType2 = class end

Seems kind of not-F#-like, but maybe it's not so bad since there's precedent over in C# for #nullable enable.

[jwosty]

If the attribute approach were taken, I'm not sure whether it would be good or bad to allow attribute scoping:

// Technically in source code, but at this point it may as well be a compiler flag. Feels icky but also powerful
[<assembly: AutoSeal>]

[vzarytovskii]

I'm going to close it, since it's quite stale and there seems to be a consensus from stakeholders that we don't want to introduce it in discussed forms.