Skip to content

Commit

Permalink
fixes #17541 - Equals visibility for DU's (#17548)
Browse files Browse the repository at this point in the history
  • Loading branch information
@KevinRansom
KevinRansom authored Aug 19, 2024
1 parent d3989f7 commit d0d6b4e
Show file tree
Hide file tree
Showing 15 changed files with 140 additions and 151 deletions.
1 change: 1 addition & 0 deletions docs/release-notes/.FSharp.Compiler.Service/9.0.100.md
View file
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
View file
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
4 changes: 1 addition & 3 deletions ...er.ComponentTests/Conformance/Types/TypeConstraints/IWSAMsAndSRTPs/IWSAMsAndSRTPsTests.fs
View file
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
3 changes: 1 addition & 2 deletions tests/FSharp.Compiler.ComponentTests/Conformance/Types/UnionTypes/UnionStructTypes.fs
View file
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
View file
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
56 changes: 56 additions & 0 deletions tests/FSharp.Compiler.ComponentTests/EmittedIL/CCtorDUWithMember/CCtorDUWithMember.fs
View file
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
71 changes: 34 additions & 37 deletions tests/FSharp.Compiler.ComponentTests/EmittedIL/GenericComparison/CrossAssembly.fs
View file
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
16 changes: 4 additions & 12 deletions tests/FSharp.Compiler.ComponentTests/EmittedIL/GenericComparison/Equals16.fsx.il.net472.bsl
View file
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

0 comments on commit d0d6b4e

Please sign in to comment.