From f6f617c6de759f718a91c48e8a2fdaf4fdad5af2 Mon Sep 17 00:00:00 2001 From: Vlad Zarytovskii Date: Mon, 27 Nov 2023 16:19:55 +0100 Subject: [PATCH 1/2] Always write static hidebysig to ref assembly --- src/Compiler/AbstractIL/ilwrite.fs | 7 +- .../EmittedIL/ReferenceAssemblyTests.fs | 133 ++++++++++++++++++ 2 files changed, 138 insertions(+), 2 deletions(-) diff --git a/src/Compiler/AbstractIL/ilwrite.fs b/src/Compiler/AbstractIL/ilwrite.fs index 9c6759ff5fb..cd4e1bfe721 100644 --- a/src/Compiler/AbstractIL/ilwrite.fs +++ b/src/Compiler/AbstractIL/ilwrite.fs @@ -1151,8 +1151,11 @@ let canGenMethodDef (tdef: ILTypeDef) cenv (mdef: ILMethodDef) = match mdef.Access with | ILMemberAccess.Public -> true // When emitting a reference assembly, do not emit methods that are private/protected/internal unless they are virtual/abstract or provide an explicit interface implementation. + // REVIEW: Addded(vlza, fixes #14937): + // We also emit methods that are marked as HideBySig and static, + // since they're not virtual or abstract, but we want (?) the same behaviour as normal instance implementations. | ILMemberAccess.Private | ILMemberAccess.Family | ILMemberAccess.Assembly | ILMemberAccess.FamilyOrAssembly - when mdef.IsVirtual || mdef.IsAbstract || mdef.IsNewSlot || mdef.IsFinal || mdef.IsEntryPoint -> true + when (mdef.IsHideBySig && mdef.IsStatic) || mdef.IsVirtual || mdef.IsAbstract || mdef.IsNewSlot || mdef.IsFinal || mdef.IsEntryPoint -> true // When emitting a reference assembly, only generate internal methods if the assembly contains a System.Runtime.CompilerServices.InternalsVisibleToAttribute. | ILMemberAccess.FamilyOrAssembly | ILMemberAccess.Assembly when cenv.hasInternalsVisibleToAttrib -> true @@ -2678,7 +2681,7 @@ let GenMethodImplPass3 cenv env _tgparams tidx mimpl = let midx2Tag, midx2Row = GetOverridesSpecAsMethodDefOrRef cenv env mimpl.Overrides AddUnsharedRow cenv TableNames.MethodImpl (UnsharedRow - [| SimpleIndex (TableNames.TypeDef, tidx) + [| SimpleIndex (TableNames.TypeDef, tidx) MethodDefOrRef (midxTag, midxRow) MethodDefOrRef (midx2Tag, midx2Row) |]) |> ignore diff --git a/tests/fsharp/Compiler/CodeGen/EmittedIL/ReferenceAssemblyTests.fs b/tests/fsharp/Compiler/CodeGen/EmittedIL/ReferenceAssemblyTests.fs index a50daae27fc..5c48adff0db 100644 --- a/tests/fsharp/Compiler/CodeGen/EmittedIL/ReferenceAssemblyTests.fs +++ b/tests/fsharp/Compiler/CodeGen/EmittedIL/ReferenceAssemblyTests.fs @@ -1405,4 +1405,137 @@ Console.WriteLine("Hello World!")""" ] |> ignore + [] + let ``Refassembly_emits_static_abstracts_implementations_the_same_way_it_does_for_instance_with_empty_signature`` () = + + let signature = """namespace Foobar +type IHasStaticAbstractBase<'a> = + static abstract BoomStatic: unit -> 'a + abstract BoomInstance: unit -> 'a + +type CompilerGoesBoom<'a> = + interface IHasStaticAbstractBase<'a> + new: unit -> CompilerGoesBoom<'a>""" + + let implementation = """namespace Foobar +type IHasStaticAbstractBase<'a> = + abstract BoomInstance: unit -> 'a + static abstract BoomStatic: unit -> 'a + +type CompilerGoesBoom<'a>() = + interface IHasStaticAbstractBase<'a> with + member (*virtual*) this.BoomInstance() = Unchecked.defaultof<'a> + static member (*non-virtual*) BoomStatic() = Unchecked.defaultof<'a> + """ + + Fsi signature + |> withAdditionalSourceFile (FsSource implementation) + |> withOptions [ "--refonly" ] + |> ignoreWarnings + |> compile + |> shouldSucceed + |> verifyIL + [ + referenceAssemblyAttributeExpectedIL + """.class interface public abstract auto ansi serializable beforefieldinit Foobar.IHasStaticAbstractBase`1 +{ + .custom instance void [FSharp.Core]Microsoft.FSharp.Core.CompilationMappingAttribute::.ctor(valuetype [FSharp.Core]Microsoft.FSharp.Core.SourceConstructFlags) = ( 01 00 03 00 00 00 00 00 ) + .method public hidebysig abstract virtual + instance !a BoomInstance() cil managed + { + } + + .method public hidebysig static abstract virtual + !a BoomStatic() cil managed + { + } + +}""" + """.method private hidebysig newslot virtual + instance !a 'Foobar.IHasStaticAbstractBase<\'a>.BoomInstance'() cil managed + { + .override method instance !0 class Foobar.IHasStaticAbstractBase`1::BoomInstance() + + .maxstack 8 + IL_0000: ldnull + IL_0001: throw + } + + .method assembly hidebysig static !a 'Foobar.IHasStaticAbstractBase<\'a>.BoomStatic'() cil managed + { + .override method !0 class Foobar.IHasStaticAbstractBase`1::BoomStatic() + + .maxstack 8 + IL_0000: ldnull + IL_0001: throw + }""" + ] + + + [] + let ``Refassembly_emits_static_abstracts_implementations_the_same_way_it_does_for_instance_with_signature`` () = + let signature = """namespace Foobar +type IHasStaticAbstractBase<'a> = + static abstract BoomStatic: unit -> 'a + abstract BoomInstance: unit -> 'a + +type CompilerGoesBoom<'a> = + interface IHasStaticAbstractBase<'a> + new: unit -> CompilerGoesBoom<'a>""" + + let implementation = """namespace Foobar +type IHasStaticAbstractBase<'a> = + abstract BoomInstance: unit -> 'a + static abstract BoomStatic: unit -> 'a + +type CompilerGoesBoom<'a>() = + interface IHasStaticAbstractBase<'a> with + member (*virtual*) this.BoomInstance() = Unchecked.defaultof<'a> + static member (*non-virtual*) BoomStatic() = Unchecked.defaultof<'a> + """ + + Fsi signature + |> withAdditionalSourceFile (FsSource implementation) + |> withOptions ["--refonly"] + |> ignoreWarnings + |> compile + |> shouldSucceed + |> verifyIL + [ + referenceAssemblyAttributeExpectedIL + """.class interface public abstract auto ansi serializable beforefieldinit Foobar.IHasStaticAbstractBase`1 +{ + .custom instance void [FSharp.Core]Microsoft.FSharp.Core.CompilationMappingAttribute::.ctor(valuetype [FSharp.Core]Microsoft.FSharp.Core.SourceConstructFlags) = ( 01 00 03 00 00 00 00 00 ) + .method public hidebysig abstract virtual + instance !a BoomInstance() cil managed + { + } + + .method public hidebysig static abstract virtual + !a BoomStatic() cil managed + { + } + +}""" + """.method private hidebysig newslot virtual + instance !a 'Foobar.IHasStaticAbstractBase<\'a>.BoomInstance'() cil managed + { + .override method instance !0 class Foobar.IHasStaticAbstractBase`1::BoomInstance() + + .maxstack 8 + IL_0000: ldnull + IL_0001: throw + } + + .method assembly hidebysig static !a 'Foobar.IHasStaticAbstractBase<\'a>.BoomStatic'() cil managed + { + .override method !0 class Foobar.IHasStaticAbstractBase`1::BoomStatic() + + .maxstack 8 + IL_0000: ldnull + IL_0001: throw + }""" + ] + + // TODO: Add tests for internal functions, types, interfaces, abstract types (with and without IVTs), (private, internal, public) fields, properties (+ different visibility for getters and setters), events. From bf23dd7f9935322db29d150a470fa9133da2cb97 Mon Sep 17 00:00:00 2001 From: Vlad Zarytovskii Date: Tue, 28 Nov 2023 11:51:43 +0100 Subject: [PATCH 2/2] Fix: run tests only in netcore clr --- .../Compiler/CodeGen/EmittedIL/ReferenceAssemblyTests.fs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/fsharp/Compiler/CodeGen/EmittedIL/ReferenceAssemblyTests.fs b/tests/fsharp/Compiler/CodeGen/EmittedIL/ReferenceAssemblyTests.fs index 5c48adff0db..b20315ffce3 100644 --- a/tests/fsharp/Compiler/CodeGen/EmittedIL/ReferenceAssemblyTests.fs +++ b/tests/fsharp/Compiler/CodeGen/EmittedIL/ReferenceAssemblyTests.fs @@ -1404,8 +1404,9 @@ Console.WriteLine("Hello World!")""" """ ] |> ignore - +#if NETCOREAPP [] +#endif let ``Refassembly_emits_static_abstracts_implementations_the_same_way_it_does_for_instance_with_empty_signature`` () = let signature = """namespace Foobar @@ -1472,7 +1473,9 @@ type CompilerGoesBoom<'a>() = ] +#if NETCOREAPP [] +#endif let ``Refassembly_emits_static_abstracts_implementations_the_same_way_it_does_for_instance_with_signature`` () = let signature = """namespace Foobar type IHasStaticAbstractBase<'a> =