Skip to content

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

Closed
#16195
Closed
Build fails with Error FS2014, error: MethodDef not found when using static abstract stuff#15919
#16195
Assignees
vzarytovskii
Labels
Area-Compiler-CodeGenIlxGen, ilwrite and things at the backendBugImpact-Low(Internal MS Team use only) Describes an issue with limited impact on existing code.
Milestone
Backlog

Description

@En3Tho
En3Tho
opened on Sep 2, 2023

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

Activity

  1. En3Tho
    added
    Bug
    Needs-Triage
    on Sep 2, 2023
  2. github-project-automation
    added this to F# Compiler and Toolingon Sep 2, 2023
  3. github-project-automation
    moved this to Not Planned in F# Compiler and Toolingon Sep 2, 2023
  4. github-actions
    added this to the Backlog milestone on Sep 2, 2023

  5. vzarytovskii commented on Sep 2, 2023

    @vzarytovskii
    vzarytovskii
    on Sep 2, 2023
    Member

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

  6. En3Tho commented on Sep 2, 2023

    @En3Tho
    En3Tho
    on Sep 2, 2023
    ContributorAuthor

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

  7. En3Tho commented on Sep 4, 2023

    @En3Tho
    En3Tho
    on Sep 4, 2023
    ContributorAuthor

    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.

  8. 0101
    added
    Impact-Low(Internal MS Team use only) Describes an issue with limited impact on existing code.
    Area-Compiler-CodeGenIlxGen, ilwrite and things at the backend
    and removed
    Needs-Triage
    on Sep 4, 2023
  9. 0101
    assigned
    vzarytovskii
    on Sep 4, 2023

  10. En3Tho commented on Oct 15, 2023

    @En3Tho
    En3Tho
    on Oct 15, 2023
    ContributorAuthor

    @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).

  11. gusty commented on Oct 27, 2023

    @gusty
    gusty
    on Oct 27, 2023
    Contributor

    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.

  12. vzarytovskii commented on Oct 27, 2023

    @vzarytovskii
    vzarytovskii
    on Oct 27, 2023
    Member

    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.

  13. En3Tho commented on Oct 27, 2023

    @En3Tho
    En3Tho
    on Oct 27, 2023
    ContributorAuthor

    @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".

  14. vzarytovskii commented on Oct 27, 2023

    @vzarytovskii
    vzarytovskii
    on Oct 27, 2023
    Member

    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

  15. vzarytovskii commented on Oct 27, 2023

    @vzarytovskii
    vzarytovskii
    on Oct 27, 2023
    Member

    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.

  16. vzarytovskii commented on Oct 27, 2023

    @vzarytovskii
    vzarytovskii
    on Oct 27, 2023
    Member

    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

  17. vzarytovskii commented on Oct 27, 2023

    @vzarytovskii
    vzarytovskii
    on Oct 27, 2023
    Member

    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.

  18. gusty commented on Oct 27, 2023

    @gusty
    gusty
    on Oct 27, 2023
    Contributor

    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.

  19. En3Tho commented on Oct 27, 2023

    @En3Tho
    En3Tho
    on Oct 27, 2023
    ContributorAuthor

    @vzarytovskii Thank you

  20. T-Gro
    closed this as completedin #16195on Oct 30, 2023
  21. github-project-automation
    moved this from Future to Done in F# Compiler and Toolingon Oct 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Area-Compiler-CodeGenIlxGen, ilwrite and things at the backendBugImpact-Low(Internal MS Team use only) Describes an issue with limited impact on existing code.

Type

No type

Projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions