Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixes #17541 - Equals visibility for DU's #17548

Merged
merged 2 commits into from
Aug 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/release-notes/.FSharp.Compiler.Service/9.0.100.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
* Fix `function` implicit conversion. ([Issue #7401](https://github.com/dotnet/fsharp/issues/7401), [PR #17487](https://github.com/dotnet/fsharp/pull/17487))
* Compiler fails to recognise namespace in FQN with enabled GraphBasedChecking. ([Issue #17508](https://github.com/dotnet/fsharp/issues/17508), [PR #17510](https://github.com/dotnet/fsharp/pull/17510))
* Fix missing message for type error (FS0001). ([Issue #17373](https://github.com/dotnet/fsharp/issues/17373), [PR #17516](https://github.com/dotnet/fsharp/pull/17516))
* MethodAccessException on equality comparison of a type private to module. ([Issue #17541](https://github.com/dotnet/fsharp/issues/17541), [PR #17548](https://github.com/dotnet/fsharp/pull/17548))

### Added

Expand Down
25 changes: 11 additions & 14 deletions src/Compiler/Checking/AugmentWithHashCompare.fs
Original file line number Diff line number Diff line change
Expand Up @@ -1322,7 +1322,7 @@ let MakeValsForCompareWithComparerAugmentation g (tcref: TyconRef) =
let MakeValsForEqualsAugmentation g (tcref: TyconRef) =
let m = tcref.Range
let _, ty = mkMinimalTy g tcref
let vis = tcref.TypeReprAccessibility
let vis = tcref.Accessibility
let tps = tcref.Typars m

let objEqualsVal =
Expand All @@ -1333,7 +1333,7 @@ let MakeValsForEqualsAugmentation g (tcref: TyconRef) =
g
tcref
ty
tcref.Accessibility
vis
(if tcref.Deref.IsFSharpException then
None
else
Expand All @@ -1347,16 +1347,13 @@ let MakeValsForEqualsAugmentation g (tcref: TyconRef) =

let MakeValsForEqualityWithComparerAugmentation g (tcref: TyconRef) =
let _, ty = mkMinimalTy g tcref
let vis =
// Equality method for union types match the union type visibility rather than the TypeReprAccessibility
if tcref.IsUnionTycon then tcref.Accessibility
else tcref.TypeReprAccessibility
let vis = tcref.Accessibility
let tps = tcref.Typars tcref.Range

let objGetHashCodeVal =
mkValSpec g tcref ty vis (Some(mkGetHashCodeSlotSig g)) "GetHashCode" (tps +-> (mkHashTy g ty)) unitArg false

let withcGetHashCodeVal =
let withGetHashCodeVal =
mkValSpec
g
tcref
Expand All @@ -1368,27 +1365,27 @@ let MakeValsForEqualityWithComparerAugmentation g (tcref: TyconRef) =
unaryArg
false

let withcEqualsVal =
let withEqualsVal =
mkValSpec g tcref ty vis (Some(mkIStructuralEquatableEqualsSlotSig g)) "Equals" (tps +-> (mkEqualsWithComparerTy g ty)) tupArg false

let withcEqualsValExact =
let withEqualsExactWithComparer =
let vis = TAccess (updateSyntaxAccessForCompPath (vis.CompilationPaths) SyntaxAccess.Public)
mkValSpec
g
tcref
ty
tcref.Accessibility
vis
// This doesn't implement any interface.
None
"Equals"
(tps +-> (mkEqualsWithComparerTyExact g ty))
tupArg
false

{
GetHashCode = objGetHashCodeVal
GetHashCodeWithComparer = withcGetHashCodeVal
EqualsWithComparer = withcEqualsVal
EqualsExactWithComparer = withcEqualsValExact
GetHashCodeWithComparer = withGetHashCodeVal
EqualsWithComparer = withEqualsVal
EqualsExactWithComparer = withEqualsExactWithComparer
}

let MakeBindingsForCompareAugmentation g (tycon: Tycon) =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -769,9 +769,7 @@ let main _ =
|> shouldSucceed
|> verifyIL [
"""
.method public specialname static class [FSharp.Core]Microsoft.FSharp.Core.FSharpChoice`2<class [FSharp.Core]Microsoft.FSharp.Core.Unit,class [FSharp.Core]Microsoft.FSharp.Core.Unit>
'|IsEqual|IsNonEqual|'<(class [Potato]Potato.Lib/IPotato`1<!!T>) T>(!!T x,
!!T y) cil managed
.method public specialname static class [FSharp.Core]Microsoft.FSharp.Core.FSharpChoice`2<class [FSharp.Core]Microsoft.FSharp.Core.Unit,class [FSharp.Core]Microsoft.FSharp.Core.Unit> '|IsEqual|IsNonEqual|'<(class [Potato]Potato.Lib/IPotato`1<!!T>) T>(!!T x, !!T y) cil managed
{

.maxstack 8
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -871,8 +871,7 @@ let main args =
IL_0002: newobj instance void Foo/StructUnion::.ctor(int32)
IL_0007: ret
}""";(*This is a 'maker method' New{CaseName} used for cases which do have fields associated with them, + the _tag gets initialized*)"""
NewCase3(string _field1_3,
string _field2_3) cil managed
NewCase3(string _field1_3, string _field2_3) cil managed
{
.custom instance void [FSharp.Core]Microsoft.FSharp.Core.CompilationMappingAttribute::.ctor(valuetype [FSharp.Core]Microsoft.FSharp.Core.SourceConstructFlags,
int32) = ( 01 00 08 00 00 00 02 00 00 00 00 00 )
Expand Down
16 changes: 4 additions & 12 deletions tests/FSharp.Compiler.ComponentTests/EmittedIL/ByRefTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -283,9 +283,7 @@ type C() =
.get instance int32& modreq([runtime]System.Runtime.InteropServices.InAttribute) Test/C::get_X()
}"""

let verifyMethod = """.method public hidebysig specialname
instance int32& modreq([runtime]System.Runtime.InteropServices.InAttribute)
get_X() cil managed
let verifyMethod = """.method public hidebysig specialname instance int32& modreq([runtime]System.Runtime.InteropServices.InAttribute) get_X() cil managed
{
.param [0]
.custom instance void [runtime]System.Runtime.CompilerServices.IsReadOnlyAttribute::.ctor() = ( 01 00 00 00 )"""
Expand Down Expand Up @@ -313,9 +311,7 @@ type C() =
.get instance int32& modreq([netstandard]System.Runtime.InteropServices.InAttribute) Test/C::get_X()
}"""

let verifyMethod = """.method public hidebysig specialname
instance int32& modreq([netstandard]System.Runtime.InteropServices.InAttribute)
get_X() cil managed
let verifyMethod = """.method public hidebysig specialname instance int32& modreq([netstandard]System.Runtime.InteropServices.InAttribute) get_X() cil managed
{
.param [0]
.custom instance void System.Runtime.CompilerServices.IsReadOnlyAttribute::.ctor() = ( 01 00 00 00 )"""
Expand Down Expand Up @@ -452,9 +448,7 @@ type C<'T>() =
abstract X<'U> : unit -> inref<'U>
"""

let verifyMethod = """.method public hidebysig abstract virtual
instance !!U& modreq([runtime]System.Runtime.InteropServices.InAttribute)
X<U>() cil managed
let verifyMethod = """.method public hidebysig abstract virtual instance !!U& modreq([runtime]System.Runtime.InteropServices.InAttribute) X<U>() cil managed
{
.param [0]
.custom instance void [runtime]System.Runtime.CompilerServices.IsReadOnlyAttribute::.ctor() = ( 01 00 00 00 )"""
Expand All @@ -481,9 +475,7 @@ type C =
.get instance int32& modreq([runtime]System.Runtime.InteropServices.InAttribute) Test/C::get_X()
}"""

let verifyMethod = """.method public hidebysig specialname abstract virtual
instance int32& modreq([runtime]System.Runtime.InteropServices.InAttribute)
get_X() cil managed
let verifyMethod = """.method public hidebysig specialname abstract virtual instance int32& modreq([runtime]System.Runtime.InteropServices.InAttribute) get_X() cil managed
{
.param [0]
.custom instance void [runtime]System.Runtime.CompilerServices.IsReadOnlyAttribute::.ctor() = ( 01 00 00 00 )"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,3 +144,59 @@ type ILArrayShape =
]
|> shouldSucceed

[<InlineData(true, "public")>] // RealSig
[<InlineData(false, "assembly")>] // Regular
[<Theory>]
let ``private DU in module`` (realSig, expected) =
FSharp """
module RealInternalSignature
module Module =
type private DU = ABC | YYZ

let publicFunction () : bool =
ABC = YYZ

Module.publicFunction () |> printfn "%b"
"""
|> asExe
|> withRealInternalSignature realSig
|> compileAndRun
|> withILContains [
$$"""
.method {{expected}} hidebysig instance bool Equals(class RealInternalSignature/Module/DU obj, class [runtime]System.Collections.IEqualityComparer comp) cil managed
{
.custom instance void [runtime]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( 01 00 00 00 )

.maxstack 4
.locals init (int32 V_0,
int32 V_1)
IL_0000: ldarg.0
IL_0001: brfalse.s IL_001b

IL_0003: ldarg.1
IL_0004: brfalse.s IL_0019

IL_0006: ldarg.0
IL_0007: ldfld int32 RealInternalSignature/Module/DU::_tag
IL_000c: stloc.0
IL_000d: ldarg.1
IL_000e: ldfld int32 RealInternalSignature/Module/DU::_tag
IL_0013: stloc.1
IL_0014: ldloc.0
IL_0015: ldloc.1
IL_0016: ceq
IL_0018: ret

IL_0019: ldc.i4.0
IL_001a: ret

IL_001b: ldarg.1
IL_001c: ldnull
IL_001d: cgt.un
IL_001f: ldc.i4.0
IL_0020: ceq
IL_0022: ret
}
"""
]
|> shouldSucceed
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,11 @@ Value.Zero = Value.Create 0 |> ignore"""
|> compileExeAndRun
|> shouldSucceed

[<InlineData(false, "private", "assembly")>] // Legacy, private WrapType, private visibility in IL
[<InlineData(false, "private", "assembly")>] // Legacy, private WrapType, assembly visibility in IL
[<InlineData(false, "internal", "assembly")>] // RealSig, internal WrapType, assembly visibility in IL
[<InlineData(false, "public", "public")>] // Legacy, public WrapType, public visibility in IL
[<InlineData(true, "private", "private")>] // RealSig, private WrapType, private visibility in IL
[<InlineData(true, "internal", "assembly")>] // RealSig, internal WrapType, assembly visibility in IL
[<InlineData(true, "private", "public")>] // RealSig, private WrapType, public visibility in IL
[<InlineData(true, "internal", "public")>] // RealSig, internal WrapType, public visibility in IL
[<InlineData(true, "public", "public")>] // RealSig, public WrapType, public visibility in IL
[<Theory>]
let ``Generated typed Equals`` (realsig, typeScope, targetVisibility) =
Expand All @@ -81,9 +81,7 @@ module Module1 =
|> shouldSucceed
|> verifyIL [
$"""
.method {targetVisibility} hidebysig instance bool
Equals(valuetype Program/Module1/Struct obj,
class [runtime]System.Collections.IEqualityComparer comp) cil managed
.method {targetVisibility} hidebysig instance bool Equals(valuetype Program/Module1/Struct obj, class [runtime]System.Collections.IEqualityComparer comp) cil managed
{{
.custom instance void [runtime]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( 01 00 00 00 )

Expand All @@ -106,24 +104,24 @@ module Module1 =
}} """ ]


[<InlineData(false, "private")>] // Legacy, private record fields, private visibility in IL
[<InlineData(false, "internal")>] // RealSig, internal record fields, assembly visibility in IL
[<InlineData(false, "public")>] // Legacy, public record fields, public visibility in IL
[<InlineData(true, "private")>] // RealSig, private record fields, private visibility in IL
[<InlineData(true, "internal")>] // RealSig, internal record fields, assembly visibility in IL
[<InlineData(true, "public")>] // RealSig, public record fields, public visibility in IL
[<InlineData(false, "private")>] // Legacy, private WrapType
[<InlineData(false, "internal")>] // RealSig, internal WrapType
[<InlineData(false, "public")>] // Legacy, public WrapType
[<InlineData(true, "private")>] // RealSig, private WrapType
[<InlineData(true, "internal")>] // RealSig, internal WrapType
[<InlineData(true, "public")>] // RealSig, public WrapType
[<Theory>]
let ``Record with various fields`` (realsig, fieldScope) =
let ``Record with various scoped fields`` (realsig, fieldScope) =

let mainModule =
FSharpWithFileName "Program.fs"
$"""
$$"""
module Module1 =
type Value =
{fieldScope} {{ value: uint32 }}
{{fieldScope}} { value: uint32 }

static member Zero = {{ value = 0u }}
static member Create(value: int) = {{ value = uint value }}
static member Zero = { value = 0u }
static member Create(value: int) = { value = uint value }

Value.Zero = Value.Create 0 |> ignore
printfn "Hello, World" """
Expand All @@ -134,7 +132,7 @@ module Module1 =
|> shouldSucceed
|> verifyIL [
"""
.method public hidebysig virtual final instance bool Equals(class Program/Module1/Value obj) cil managed
.method public hidebysig instance bool Equals(class Program/Module1/Value obj, class [runtime]System.Collections.IEqualityComparer comp) cil managed
{
.custom instance void [runtime]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( 01 00 00 00 )

Expand All @@ -161,26 +159,27 @@ module Module1 =
IL_001b: ldc.i4.0
IL_001c: ceq
IL_001e: ret
} """
"""
IL_0020: call class [runtime]System.Collections.IEqualityComparer [FSharp.Core]Microsoft.FSharp.Core.LanguagePrimitives::get_GenericEqualityComparer()
IL_0025: callvirt instance bool Program/Module1/Value::Equals(class Program/Module1/Value,
class [runtime]System.Collections.IEqualityComparer)
""" ]


[<InlineData(false, "private", "assembly")>] // Legacy, private WrapType, private visibility in IL
[<InlineData(false, "internal", "assembly")>] // RealSig, internal WrapType, assembly visibility in IL
[<InlineData(false, "public", "public")>] // Legacy, public WrapType, public visibility in IL
[<InlineData(true, "private", "private")>] // RealSig, private WrapType, private visibility in IL
[<InlineData(true, "internal", "assembly")>] // RealSig, internal WrapType, assembly visibility in IL
[<InlineData(true, "public", "public")>] // RealSig, public WrapType, public visibility in IL
} """ ]


[<InlineData(false, "public", "private", "assembly")>] // public module - Legacy, private WrapType, private visibility in IL
[<InlineData(false, "public", "internal", "assembly")>] // public module - RealSig, internal WrapType, assembly visibility in IL
[<InlineData(false, "public", "public", "public")>] // public module - Legacy, public WrapType, public visibility in IL
[<InlineData(true, "public", "private", "public")>] // public module - RealSig, private WrapType, public visibility in IL
[<InlineData(true, "public", "internal", "public")>] // public module - RealSig, internal WrapType, public visibility in IL
[<InlineData(true, "public", "public", "public")>] // public module - RealSig, public WrapType, public visibility in IL
[<InlineData(false, "private", "private", "assembly")>] // private module - Legacy, private WrapType, private visibility in IL
[<InlineData(false, "private", "internal", "assembly")>] // private module - RealSig, internal WrapType, assembly visibility in IL
[<InlineData(false, "private", "public", "assembly")>] // private module - Legacy, public WrapType, assembly visibility in IL
[<InlineData(true, "private", "private", "public")>] // private module - RealSig, private WrapType, public visibility in IL
[<InlineData(true, "private", "internal", "public")>] // private module - RealSig, internal WrapType, public visibility in IL
[<InlineData(true, "private", "public", "public")>] // private module - RealSig, public WrapType, public visibility in IL
[<Theory>]
let ``scoped type arg`` (realsig, argScope, targetVisibility) =
let ``scoped main and scoped type Equals`` (realsig, moduleScope, argScope, targetVisibility) =
let mainModule =
FSharpWithFileName "Program.fs"
$"""
module IPartialEqualityComparer =
module {moduleScope} IPartialEqualityComparer =
open System.Collections.Generic

[<StructuralEquality; NoComparison>]
Expand All @@ -195,9 +194,7 @@ module IPartialEqualityComparer =
|> shouldSucceed
|> verifyIL [
$"""
.method {targetVisibility} hidebysig instance bool
Equals(class Program/IPartialEqualityComparer/WrapType`1<!T> obj,
class [runtime]System.Collections.IEqualityComparer comp) cil managed
.method {targetVisibility} hidebysig instance bool Equals(class Program/IPartialEqualityComparer/WrapType`1<!T> obj, class [runtime]System.Collections.IEqualityComparer comp) cil managed
{{
.custom instance void [runtime]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( 01 00 00 00 )

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,9 +128,7 @@
IL_000c: ret
}

.method public hidebysig virtual final
instance int32 CompareTo(object obj,
class [runtime]System.Collections.IComparer comp) cil managed
.method public hidebysig virtual final instance int32 CompareTo(object obj, class [runtime]System.Collections.IComparer comp) cil managed
{
.custom instance void [runtime]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( 01 00 00 00 )

Expand Down Expand Up @@ -235,9 +233,7 @@
IL_000b: ret
}

.method assembly hidebysig instance bool
Equals(valuetype assembly/EqualsMicroPerfAndCodeGenerationTests/SomeStruct obj,
class [runtime]System.Collections.IEqualityComparer comp) cil managed
.method public hidebysig instance bool Equals(valuetype assembly/EqualsMicroPerfAndCodeGenerationTests/SomeStruct obj, class [runtime]System.Collections.IEqualityComparer comp) cil managed
{
.custom instance void [runtime]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( 01 00 00 00 )

Expand All @@ -259,9 +255,7 @@
IL_0020: ret
}

.method public hidebysig virtual final
instance bool Equals(object obj,
class [runtime]System.Collections.IEqualityComparer comp) cil managed
.method public hidebysig virtual final instance bool Equals(object obj, class [runtime]System.Collections.IEqualityComparer comp) cil managed
{
.custom instance void [runtime]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( 01 00 00 00 )

Expand All @@ -285,9 +279,7 @@
IL_0019: ret
}

.method public specialname rtspecialname
instance void .ctor(int32 v,
int32 u) cil managed
.method public specialname rtspecialname instance void .ctor(int32 v, int32 u) cil managed
{

.maxstack 8
Expand Down
Loading
Loading