Fix virtual static call

[DragonSA]

Description

Fix calling an overridden virtual static method via the interface.

The current fsharp calls, ends up calling the default implementation of C# static virtual methods. This fixes this by ensuring that the static virtual metadata is honoured in method resolution.

Checklist

• Test cases added
• Release notes entry updated

Sponsorship

Counterpoint Dynamics (Pty) Ltd

[github-actions]

❗ Release notes required

---

✅ Found changes and release notes in following paths:

Change path	Release notes path	Description
src/Compiler	docs/release-notes/.FSharp.Compiler.Service/8.0.400.md

[vzarytovskii]

What was the behaviour before? This needs SRTP tests as well. Also need to make sure RFC for IWSAMs mentions virtual members as well.

[KevinRansom]

@vzarytovskii ...

It compiles the method invocation however it invokes the base class method implementation and not the overridden version.

static virtual members are confusingly necessary for the INumerics stuff because of generics.

It's a good change.

[vzarytovskii]

@vzarytovskii ...

It compiles the method invocation however it invokes the base class method implementation and not the overridden version.

static virtual members are confusingly necessary for the INumerics stuff because of generics.

It's a good change.

It's a breaking change then? What do we do in case of non-statics?

[KevinRansom]

The change is a good one, we need to consider the breaking change. Although my intuition is that it is rare enough and breaks an important dotnet feature so we would take the break.

Although ... the scenario to implement the INumerics, may not have been a goal, I wasn't involved in those design discussions.

[KevinRansom]

@vzarytovskii interfaces with virtual statics was implemented as part of the INumerics feature. We handle abstract/virtuals/override on instance methods compatibly with C#. I think we probably consume, the framework INumber implementations correctly. This particular bug is likely merely an oversight.

[KevinRansom]

Oh yeah, and there is no real performance implications.

[vzarytovskii]

@vzarytovskii interfaces with virtual statics was implemented as part of the INumerics feature. We handle abstract/virtuals/override on instance methods compatibly with C#. I think we probably consume, the framework INumber implementations correctly. This particular bug is likely merely an oversight.

I know, I implemented those in F#.

Question is which method do we call now
A. In case of static virtual
B. In case of non-static virtual

A should match B.

[KevinRansom]

For virtual inheritence, it should be the most derived. Static should emulate that I should think.

[DragonSA]

What was the behaviour before? This needs SRTP tests as well. Also need to make sure RFC for IWSAMs mentions virtual members as well.

The prior behaviour was the override of a virtual static method could not be called by F#.

Just a note that SRTP cannot be used to call the override of Echo, since that is defined on the interface and not the class itself. I've added a test (in 93e3828) to confirm the correct static methods get called.

Regarding the RFC, are you referring to RFC FS-1124, this does not mention virtual static methods.

@vzarytovskii ...
It compiles the method invocation however it invokes the base class method implementation and not the overridden version.
static virtual members are confusingly necessary for the INumerics stuff because of generics.
It's a good change.

It's a breaking change then? What do we do in case of non-statics?

The overridden method is called:

    [<FactForNETCOREAPP>]
    let ``F# can call overwritten virtual member from interface``() =
        let CSharpLib =
            CSharp """
namespace Test;

public interface I
{
    virtual string Echo(string x) => $"I.Echo: {x}";
}
            """
            |> withCSharpLanguageVersion CSharpLanguageVersion.CSharp11
            |> withName "CsLibAssembly"

        FSharp """
type Imp() =
    interface Test.I with
        member _.Echo (x: string) = $"Imp.I.Echo: {x}"

let echo<'T when 'T :> Test.I> (a: 'T) x = a.Echo(x)

match echo (Imp()) "a" with
| "Imp.I.Echo: a" -> printfn "success"
| _ -> failwith "incorrect value"
"""
        |> withReferences [CSharpLib]
        |> withLangVersion80
        |> asExe
        |> compileAndRun
        |> shouldSucceed

@vzarytovskii interfaces with virtual statics was implemented as part of the INumerics feature. We handle abstract/virtuals/override on instance methods compatibly with C#. I think we probably consume, the framework INumber implementations correctly. This particular bug is likely merely an oversight.

I know, I implemented those in F#.

Question is which method do we call now A. In case of static virtual B. In case of non-static virtual

A should match B.

I agree :-). Currently we:
A. In case of static virtual: F# calls the base implementation (C# calls the override) (this change calls the override).
B. In case of non-static: F# calls the override (C# does the same).

Thus, this change is required to bring A and B into alignment.

[DragonSA]

Out of interest, what is the protocol for resolving comments? Should I do it once I made the appropriate change/comment, or does the commenter resolve it once satisfied?

[vzarytovskii]

Regarding the RFC, are you referring to RFC FS-1124, this does not mention virtual static methods.

Yeah, this one. Probably then add a one-liner saying what will be preferred in case of virtual methods.

[DragonSA]

Regarding the RFC, are you referring to RFC FS-1124, this does not mention virtual static methods.

Yeah, this one. Probably then add a one-liner saying what will be preferred in case of virtual methods.

I've created fsharp/fslang-design#766

[vzarytovskii]

Out of interest, what is the protocol for resolving comments? Should I do it once I made the appropriate change/comment, or does the commenter resolve it once satisfied?

Feel free to resolve if you've addressed it

[edgarfgp]

Thanks @DragonSA and to Counterpoint Dynamics (Pty) for sponsoring this. Really cool 👍

[vzarytovskii]

Looks good, thanks

[dawedawe]

Thanks @DragonSA and to Counterpoint Dynamics (Pty) for sponsoring this. Really cool 👍

Absolutely, thanks for sponsoring ❤️
Would be awesome to see more actions like this.

[psfinaki]

Thanks!