Build fails with Error FS2014, error: MethodDef not found when using static abstract stuff

[En3Tho]

Stumbled upon this recently when trying to make C#-like task builder which only requires the user to define awaiter, builder and task types

Repro steps
Extract the archive, try to build the solution
ClassLibrary1.zip

Tried it with net7 and net8 without any luck

[vzarytovskii]

To add: it affects only reference assembly generation, turning refassemblies off should be a feasible workaround until it's fixed.

[En3Tho]

@vzarytovskii Maybe I did something wrong, but turning off reference assembly generation did not help but the error message changed slightly

[En3Tho]

It seems like is set to false by default
With explicitly set false

FSC : error FS2014: Проблема при записи двоичного файла "G:\source\repos\En3Tho\FSharpExtensions\FSharpExtensions\StaticAbstractTest\obj\Debug\net7.0\StaticAbstractTest.dll"
: Error in pass3 for type StaticAbstractTest.MyValueTaskMethodBuilder`1, error: Error in GetMethodRefAsMethodDefIdx
for mref = ("En3Tho.FSharp.ComputationExpressions.Utilities.IAsyncMethodBuilderBase<StaticAbstractTest.MyValueTas kMethodBuilder<'a>>.Create", "StaticAbstractTest.MyValueTaskMethodBuilder`1"),
error: MethodDefNotFound [G:\source\repos\En3Tho\FSharpExtensions\FSharpExtensions\StaticAbstractTest\StaticAbstractTest.fsproj]

With explicitly set true

FSC : error FS2014: Проблема при записи двоичного файла "obj\Debug\net7.0\refint\En3Tho.FSharp.ComputationExpressions.dll"
: Error in pass3 for type En3Tho.FSharp.ComputationExpressions.Tasks.MyValueTaskMethodBuilder`1, error: Error in GetMethodRefAsMethodDefIdx
for mref = "En3Tho.FSharp.ComputationExpressions.Utilities.IAsyncMethodBuilderBase<En3Tho.FSharp.ComputationExpressions.Tasks.MyValueTaskMethodBuilder<'a>>.Create", "En3Tho.FSharp.ComputationExpressions.Tasks.MyValueTaskMethodBuilder`1"),
error: MethodDefNotFound [G:\source\repos\En3Tho\FSharpExtensions\FSharpExtensions\En3Tho.FSharp.ComputationExpressions\En3Tho.FSharp.ComputationExpressions.fsproj]

Note that when producing reference assembly, the only difference is "refinit" is in the path. Also, file name is not the full path but relative for some reason.

I'm not sure this is a regression because it's reproduceable both on .Net 7 and .Net 8.

P.S. It seems like I did post errors above from my main solution but repro project behaves the same. Sorry for confusion.

[En3Tho]

@vzarytovskii I can confirm reference assemblies have nothing to do with this again. I've managed to come up with a minimal repro. This is clearly a compiler bug.

a minimal repro:

type IHasStaticAbstractBase<'a> =
    static abstract Boom: unit -> 'a

type IHasStaticAbstractChild<'a> =
    inherit IHasStaticAbstractBase<'a>

type CompilerGoesBoom<'a>() =
    interface IHasStaticAbstractChild<'a> with // this goes boom because inheritance is involved
        static member Boom() = Unchecked.defaultof<'a>

type CompilerDoesNotGoBoom<'a>() =
    interface IHasStaticAbstractBase<'a> with // this does not go boom, direct base interface
        static member Boom() = Unchecked.defaultof<'a>

My best guess is that it is trying to find Boom member on IHasStaticAbstractChild<'a> and is unable to do so. It has to walk up the inheritance chain just like with non static members (or maybe IHasStaticAbstractChild<'a> should have it, I don't know).

[gusty]

Can we change the impact?
I don't think this is low impact at all, if you start using static abstracts chances are that at some point you'll find this bug.
It doesn't look like a corner case.

[vzarytovskii]

Can we change the impact?

I don't think this is low impact at all, if you start using static abstracts chances are that at some point you'll find this bug.

It doesn't look like a corner case.

Impact is internal for our own usage only.

It mostly means it doesn't impact any existing code.

[En3Tho]

@vzarytovskii Why do you think it's internal? As I said, reference assembly generation has nothing to do with this. Whole build simply fails if you use inheritance with static abstracts. I found this bug while writing my own code, not while tinkering with core lib or something like that

Or maybe I've misunderstood and "impact" itself is an internal marker for the team. This is sort of confusing.

Anyway, I don't think it's that "low". If it was about reference assemblies for example and there was a clear workaround (like just turn them off and be done with it, build is ok) then it might be "low". But this one has no workarounds except for "well, don't use that".

[vzarytovskii]

Why do you think it's internal?

Why do I think that "impact" label is for internal use only? Well, because it is.

Anyway, I don't think it's that "low".

Again, it does not reflect severity, or importance, or anything.

To reiterate: PLEASE, DON'T LOOK AT IMPACT TAGS AS IS THEY REFLECT SEVERITY OF BUGS

[vzarytovskii]

Investigation

Non-static abstract

Info

mref.Name: Foo.IHasStaticAbstractBase<'a>.Boom
tref.Name: Foo.CompilerGoesBoom`1
tidx: 4
mdkey.Name: Foo.IHasStaticAbstractBase<'a>.Boom
mdkey.IsStatic: false
idx: 3

Mdefs

{Boom}
{.ctor}
{FSI_0008.Foo.IHasStaticAbstractBase<'a>.Boom}
{main@}

Writing mdefs

1:
    {type Foo.IHasStaticAbstractBase`1}
    {method Boom}
2:
    {type Foo.CompilerGoesBoom`1}
    {method .ctor}
3:
    {type Foo.CompilerGoesBoom`1}
    {method Foo.IHasStaticAbstractBase<'a>.Boom}
4:
    {type <StartupCode$>.$Repro}
    {method main@}

Static abstract

Info

mref.Name: Foo.IHasStaticAbstractBase<'a>.Boom
tref.Name: Foo.CompilerGoesBoom`1
tidx: 4
mdkey.Name: Foo.IHasStaticAbstractBase<'a>.Boom
mdkey.IsStatic: true
idx:

Mdefs

{Boom}
{.ctor}
{FSI_0007.Foo.IHasStaticAbstractChild<'a>.Boom}
{main@}

Writing mdefs

1:
    {type Foo.IHasStaticAbstractBase`1}
    {method Boom}
2:
    {type Foo.CompilerGoesBoom`1}
    {method .ctor}
3:
    {type Foo.CompilerGoesBoom`1}
    {method Foo.IHasStaticAbstractChild<'a>.Boom}
4:
    {type <StartupCode$>.$Repro}
    {method main@}

Differences

Method getting emitted in case of static abstract is

{type Foo.CompilerGoesBoom`1}
{method Foo.IHasStaticAbstractChild<'a>.Boom}

Just abstract:

{type Foo.CompilerGoesBoom`1}
{method Foo.IHasStaticAbstractBase<'a>.Boom}

Lookup is, however, trying to search for Foo.CompilerGoesBoom'1.Foo.IHasStaticAbstractBase<'a>.Boom.

[vzarytovskii]

The bug is that we don't generate fixups method rename fixup for the static abstracts here:

fsharp/src/Compiler/CodeGen/IlxGen.fs

Lines 9214 to 9221 in f88d5d4

	else // We want to get potential fixups and hidebysig for abstract statics:
	let flagFixups = [ fixupStaticAbstractSlotFlags ]
	let mdef =
	mkILStaticMethod (ilMethTypars, mspec.Name, access, ilParams, ilReturn, ilMethodBody)
	let mdef = List.fold (fun mdef f -> f mdef) mdef flagFixups
	mdef

like we do it for normal slots here:

fsharp/src/Compiler/CodeGen/IlxGen.fs

Lines 9227 to 9246 in f88d5d4

	let flagFixups = ComputeFlagFixupsForMemberBinding cenv v
	let cconv =
	if memberInfo.MemberFlags.IsInstance then
	ILCallingConv.Instance
	else
	ILCallingConv.Static
	let mdef =
	mkILGenericVirtualMethod (
	mspec.Name,
	cconv,
	ILMemberAccess.Public,
	ilMethTypars,
	ilParams,
	ilReturn,
	ilMethodBody
	)
	let mdef = List.fold (fun mdef f -> f mdef) mdef flagFixups

[vzarytovskii]

That would be the fix, I think. If someone wants to make the fix + few tests:

diff --git a/src/Compiler/CodeGen/IlxGen.fs b/src/Compiler/CodeGen/IlxGen.fs
index 33b39ce7f..a987b78ab 100644
--- a/src/Compiler/CodeGen/IlxGen.fs
+++ b/src/Compiler/CodeGen/IlxGen.fs
@@ -9212,7 +9212,13 @@ and GenMethodForBinding
                             if not memberInfo.MemberFlags.IsOverrideOrExplicitImpl then
                                 mkILStaticMethod (ilMethTypars, mspec.Name, access, ilParams, ilReturn, ilMethodBody)
                             else // We want to get potential fixups and hidebysig for abstract statics:
-                                let flagFixups = [ fixupStaticAbstractSlotFlags ]
+                                let flagFixups =
+                                    [
+                                        fixupStaticAbstractSlotFlags
+                                        match ComputeMethodImplNameFixupForMemberBinding cenv v with
+                                        | Some nm -> renameMethodDef nm
+                                        | None -> ()
+                                    ]

                                 let mdef =
                                     mkILStaticMethod (ilMethTypars, mspec.Name, access, ilParams, ilReturn, ilMethodBody)

Otherwise, I can make it next week once I'm back.

[gusty]

Maybe we should a severity tag.

@vzarytovskii good job finding the root cause. I was researching it as well, but you are miles ahead. I will create a PR with a fix.

[En3Tho]

@vzarytovskii Thank you