No compiler error for instance members on static class; produces invalid IL & causes runtime error

[brianrourkeboll]

Repro steps

1. Define a static class using the combination of SealedAttribute and AbstractClassAttribute.
2. Add fields, instance members, even constructors:¹
   a. Fields (mutable or not), properties, methods, indexers.
   b. Abstract, concrete, override...

Examples

E.g., this

// This compiles and produces nonsense IL.
[<Sealed; AbstractClass>]
type T =
    abstract A : int
    abstract B : int with get, set
    abstract C : i:int -> int
    abstract D : i:int -> int
    default _.D i = i + 3
    member _.E = 3
    val F : int
    val mutable G : int
    member _.H (i, j) = i + j
    member _.Item with get i = 3 and set i value = ()
    override _.ToString () = "🙃"
    new () = { F = 3; G = 3 }
    new (x, y) = { F = x; G = y }

generates this (SharpLab):

[Serializable]
[Sealed]
[AbstractClass]
[CompilationMapping(SourceConstructFlags.ObjectType)]
public static class T
{
    internal int F@;

    public int G;

    [CompilationMapping(SourceConstructFlags.Field, 0)]
    public int F
    {
        get
        {
            return F@;
        }
    }

    public abstract override int A { get; }

    public abstract override int B { get; set; }

    public int E
    {
        get
        {
            return 3;
        }
    }

    public int this[object i]
    {
        get
        {
            return 3;
        }
        set
        {
        }
    }

    public abstract override int C(int i);

    public override int D(int i)
    {
        return i + 3;
    }

    public int H(int i, int j)
    {
        return i + j;
    }

    public override string ToString()
    {
        return "\ud83d\ude43";
    }

    public T()
    {
        F@ = 3;
        G = 3;
    }

    public T(int x, int y)
    {
        F@ = x;
        G = y;
    }
}

There is also no error when making a class with a primary constructor static (SharpLab):

// Just throw [<Sealed; AbstractClass>] on the default
// SharpLab class and it still compiles!
[<Sealed; AbstractClass>]
type C() =
    member _.M() = ()

There is no error for fieldless single-case unions, either (SharpLab):²³⁴⁵

[<Sealed; AbstractClass>]
type U = U
let u = U

There is likewise no warning or error when adding SealedAttribute to a struct fieldless single-case union (SharpLab):⁵

[<Sealed; Struct>]
type U = U

Expected behavior

The code should not compile.

Actual behavior

The code compiles, and calling it results in a System.BadImageFormatException with the message "Bad IL format." at runtime.⁶

> Unchecked.defaultof<T>.A;;
System.BadImageFormatException: Bad IL format.
   at <StartupCode$FSI_0007>.$FSI_0007.main@()
Stopped due to error

Known workarounds

Don't try weird things.

Related information

• .NET SDK 6.0.300.

Footnotes

1. Not auto-properties: using member val correctly gives FS3133, presumably because the check for a primary constructor is separate. member val is still let through if you do have a primary constructor (SharpLab). ↩

2. It looks like the compiler ignores the combination of Sealed and AbstractClass here and just adds the AbstractClass attribute without actually making the class static. ↩

3. The compiler correctly gives FS0942 and FS0939 when a field is added (SharpLab) or a second case is added (SharpLab). ↩

4. The compiler does disallow this for records with FS0942 and FS0939 (SharpLab). ↩

5. If you add a field or a bar | to either of these, the compiler does give you FS0942 and/or FS0939. ↩ ↩²

6. The compiler does disallow calling a constructor added to a static class with FS0759. ↩

[vzarytovskii]

This should be relatively easy to add check for and fix, I'll mark it as a good first issue.

[edgarfgp]

@T-Gro @psfinaki @0101 Would it make sense to split this in separate PRs?. Also, I'm a little lost regarding to what is allow or not allow at this point this code. Can we agree here. what or show raise a compiler warning so it will help to focus in one at the time . Thanks in advance :)

[vzarytovskii]

@T-Gro @psfinaki @0101 Would it make sense to split this in separate PRs?.

Up to you, I'd say - the more granular - the better. Easier to reason about particular feature.

Also, I'm a little lost regarding to what is allow or not allow at this point this code. Can we agree here. what or show raise a compiler warning so it will help to focus in one at the time . Thanks in advance :)

What does the specification say for these?

[edgarfgp]

@vzarytovskii Are we talking about this specification ? 4.1. or there is more updated one ?

[vzarytovskii]

@vzarytovskii Are we talking about this specification ? 4.1. or there is more updated one ?

Yeah, in the this spec or any rfcs. But apparently there's nothing which mentions this.

[edgarfgp]

Quick question : Is there any specific rule to why Sealed and Abstract attributes can be used at the same time in the same type ? This is not allowed in C#

[<Sealed;AbstractClass>]
type CompilerAssert private () = .. 

[<Sealed; AbstractClass>]
type C() =
    member _.M() = ()

[<Sealed; AbstractClass>]
type U = U
let u = U

Or should raise a compiler error if both are used together in any type ?

@vzarytovskii @dsyme

https://sharplab.io/#v2:C4LgTgrgdgPgsAKAAIGYAEBnApgQwDZYAmaSATCQIwDsaA3oomkyejgEYbB[…]dFaubqZAX3W7pmg01SUAbCQAsaALI4AllHlIKABjQBtALpoA+orVGaPraQA===

[vzarytovskii]

Quick question : Is there any specific rule to why Sealed and Abstract attributes can be used at the same time in the same type ? This is not allowed in C#

[<Sealed;AbstractClass>]
type CompilerAssert private () = .. 

[<Sealed; AbstractClass>]
type C() =
    member _.M() = ()

[<Sealed; AbstractClass>]
type U = U
let u = U

Or should raise a compiler error if both are used together in any type ?

@vzarytovskii @dsyme

https://sharplab.io/#v2:C4LgTgrgdgPgsAKAAIGYAEBnApgQwDZYAmaSATCQIwDsaA3oomkyejgEYbB[…]dFaubqZAX3W7pmg01SUAbCQAsaALI4AllHlIKABjQBtALpoA+orVGaPraQA===

Sealed abstract types are pretty much just a way of having "static classes".

[edgarfgp]

Hmm I see thanks . There is a lot going on this issue . it difficult to identify what if any error/ warning are missing in this repro . I would suggest to split this in small ones with some extra description to make it a "real" good-first-issue.

Im leaving this for now . Happy to help in the future :)

[vzarytovskii]

I guess we can produce an error if type is both sealed and abstract and has instance members defined?
cc @dsyme @T-Gro

[T-Gro]

Personally I would see it a lot cleaner to raise a 'dragons,beware'-style warning (not error because of backwards compat?) whenever [<Sealed;AbstractClass>] is used.

For any kind of newcomer, a module with functions should be always preferred over this.

Warning would mean it is only for when people know what and why they are doing (which I personally do not see at the moment).
From this issue itself I get the feeling that there are too many corner cases around the concept of [<Sealed;AbstractClass>].
Weighting together the risks of getting something wrong (too defensive error, or missing something out) and the size of covering all cases on one side, and the amount of code really needing this on the other side => it does not add up.

(google search for fsharp [<Sealed;AbstractClass>] finds me this GH issue only)

[vzarytovskii]

not error because of backwards compat?

Well if it now produces invalid IL, can be error by default guarded by language version (if someone, for some reason has this code).

[T-Gro]

I meant an info/warning for [<Sealed;AbstractClass>] any time it is used (instance properties or not).
It does carry the feeling of an undocumented and unsupported scenario/hack.

[vzarytovskii]

I meant an info/warning for [<Sealed;AbstractClass>] any time it is used (instance properties or not).
It does carry the feeling of an undocumented and unsupported scenario/hack.

Oh, no, we probably shouldn't by default. It's a quite known way of having a "static class", when people don't want to use modules. I've seen it many times in different codebases.

Would be a good analyzer though, when we'll progress with the SDK.

[edgarfgp]

not error because of backwards compat?

Well if it now produces invalid IL, can be error by default guarded by language version (if someone, for some reason has this code).

Yeah this is what was advised in similar cases .

[brianrourkeboll]

Hmm I see thanks . There is a lot going on this issue . it difficult to identify what if any error/ warning are missing in this repro . I would suggest to split this in small ones with some extra description to make it a "real" good-first-issue.

Yes, there are a few different things here. When I discovered one, I figured I might as well poke around and see what other related oddities I could find 🙃. Philosophically, though, they pretty much all have to do with one of the following:

1. Allowing instance-level constructs on types that cannot be instantiated (this is the more noteworthy one, since you get no compile-time error and get a runtime failure instead).
2. Allowing SealedAttribute and/or AbstractClassAttribute on fieldless, |-less discriminated unions (including struct unions).

Quick question : Is there any specific rule to why Sealed and Abstract attributes can be used at the same time in the same type ? This is not allowed in C#

Yes, putting [<Sealed; AbstractClass>] (or [<AbstractClass; Sealed>]) on a regular F# class type compiles to a static class (Sealed → it can't be inherited from, AbstractClass → it can't be instantiated).

This can be very useful when defining a set of overloaded static methods,¹ for example, especially if such a type will be used from C#, since even though a type without a constructor like

type T =
    static member M () = ()

cannot be instantiated from F#, it can be instantiated from C# (see #8093, fsharp/fslang-suggestions#906), which is usually not intended.

Footnotes

1. If method-specific features like overloads were not needed, you could of course just use a module instead, since modules already compile to static classes. ↩

[abelbraaksma]

Related:

• BadImageFormatException through base: BadImageFormatException : Bad IL format when using base #13926
• BadImageFormatException with op_Addition: Bad image when calling op_Addition on interface #14012
• BadImageFormatException : Abstract properties are allowing additional generic arguments #12198 (solved, but since it's related, looking up that fix may help you here)

I'd like to emphasize that the static class case above (in F# that's the combination of Sealed and AbstractClass is considered "static class" in C#), produces correct IL. The IL itself has no concept of static class, that's a C# term and such C# code will add those attributes. However, by marking it both Abstract and Sealed, it's impossible to create an instance of the class (not sure about reflection, but it should be prohibited), which is where the term "static class" came from, as only its static members are accessible.

So, if you add instance members to a sealed class, that's totally fine, but can never be executed. Just like it is totally fine to write code after a failwith, which will also never be executed.

A warning may be nice, though. Here's another example showing that you can freely add extra constructors, or even let-bindings (which capture private-only fields). It may be non-trivial to detect all this:

[<Sealed; AbstractClass>]
type MyStaticClass() =
    let foo = 42  // huh?
    new(a, b) = MyStaticClass()
    member _.DoSomething() = ()

PS: there's literally nothing in the MSDN docs, and also nothing in the F# spec afaik. This is just a feature by accident: this combination of attributes is allowed by design, but doesn't necessarily limit what you can write (which is a bit unfortunate, really).

PPS: the BadImageFormatException in the example by the OP only happens after you do Unchecked.defaultof<T>.A. I.e., by attempting to create a null-reference. You'd expect an NRE though, not a bad image exception.

However, I cannot reproduce this. Here's what I tried (sharplab also shows correct, somewhat surprising IL):

> [<Sealed; AbstractClass>]
    type C() =
        member _.M() = 42;;
type C =
  new: unit -> C
  member M: unit -> int

> let x = Unchecked.defaultof<C>.M();;
val x: int = 42

Though the result is surprising. The instance member is treated as a static member. Now, in terms of method-calling, this is correct, because that is how you usually say in IL that you want to call a static member. However, it is not a static member, sooo.... weird stuff.

[brianrourkeboll]

@abelbraaksma yes, that particular example doesn't yield a BadImageFormatException because, I think, the method call is optimized away altogether by the F# compiler (x is directly assigned the value 42).