Skip to content

Commit

Permalink
Fixes #17447 -MethodAccessException on equality comparison of a recor…
Browse files Browse the repository at this point in the history 
…d with private fields (#17467)

* Fix17447

* tests + readme
  • Loading branch information
@KevinRansom
KevinRansom authored Jul 31, 2024
1 parent ed7905d commit 44e7bfa
Show file tree
Hide file tree
Showing 5 changed files with 221 additions and 63 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 @@ -4,6 +4,7 @@
* Fix reporting IsFromComputationExpression only for CE builder type constructors and let bindings. ([PR #17375](https://github.com/dotnet/fsharp/pull/17375))
* Optimize simple mappings in comprehensions when the body of the mapping has `let`-bindings and/or sequential expressions before a single yield. ([PR #17419](https://github.com/dotnet/fsharp/pull/17419))
* C# protected property can be assigned in F# inherit constructor call. ([Issue #13299](https://github.com/dotnet/fsharp/issues/13299), [PR #17391](https://github.com/dotnet/fsharp/pull/17391))
* MethodAccessException on equality comparison of a record with private fields. ([Issue #17447](https://github.com/dotnet/fsharp/issues/17447), [PR #17391](https://github.com/dotnet/fsharp/pull/17467))

### Added

Expand Down
4 changes: 2 additions & 2 deletions src/Compiler/Checking/AugmentWithHashCompare.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1333,7 +1333,7 @@ let MakeValsForEqualsAugmentation g (tcref: TyconRef) =
g
tcref
ty
vis
tcref.Accessibility
(if tcref.Deref.IsFSharpException then
None
else
Expand Down Expand Up @@ -1376,7 +1376,7 @@ let MakeValsForEqualityWithComparerAugmentation g (tcref: TyconRef) =
g
tcref
ty
vis
tcref.Accessibility
// This doesn't implement any interface.
None
"Equals"
Expand Down
272 changes: 212 additions & 60 deletions tests/FSharp.Compiler.ComponentTests/EmittedIL/GenericComparison/CrossAssembly.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,87 +7,239 @@ open FSharp.Test.Compiler

module GenericComparisonCrossAssembly =

[<Fact>]
let ``fslib``() =
[<InlineData(true)>] // RealSig
[<InlineData(false)>] // Regular
[<Theory>]
let ``fslib``(realsig) =
FSharpWithFileName "Program.fs"
"""
ValueSome (1, 2) = ValueSome (2, 3) |> ignore"""
|> withRealInternalSignature realsig
|> withOptimize
|> compileExeAndRun
|> shouldSucceed
|> verifyIL [ """
.method assembly specialname static void staticInitialization@() cil managed
{
.maxstack 8
IL_0000: ldc.i4.1
IL_0001: ldc.i4.2
IL_0002: newobj instance void class [runtime]System.Tuple`2<int32,int32>::.ctor(!0,
!1)
IL_0007: call valuetype [FSharp.Core]Microsoft.FSharp.Core.FSharpValueOption`1<!0> valuetype [FSharp.Core]Microsoft.FSharp.Core.FSharpValueOption`1<class [runtime]System.Tuple`2<int32,int32>>::NewValueSome(!0)
IL_000c: stsfld valuetype [FSharp.Core]Microsoft.FSharp.Core.FSharpValueOption`1<class [runtime]System.Tuple`2<int32,int32>> Program::x@1
IL_0011: ldc.i4.2
IL_0012: ldc.i4.3
IL_0013: newobj instance void class [runtime]System.Tuple`2<int32,int32>::.ctor(!0,
!1)
IL_0018: call valuetype [FSharp.Core]Microsoft.FSharp.Core.FSharpValueOption`1<!0> valuetype [FSharp.Core]Microsoft.FSharp.Core.FSharpValueOption`1<class [runtime]System.Tuple`2<int32,int32>>::NewValueSome(!0)
IL_001d: stsfld valuetype [FSharp.Core]Microsoft.FSharp.Core.FSharpValueOption`1<class [runtime]System.Tuple`2<int32,int32>> Program::y@1
IL_0022: ldsflda valuetype [FSharp.Core]Microsoft.FSharp.Core.FSharpValueOption`1<class [runtime]System.Tuple`2<int32,int32>> Program::x@1
IL_0027: call valuetype [FSharp.Core]Microsoft.FSharp.Core.FSharpValueOption`1<class [runtime]System.Tuple`2<int32,int32>> Program::get_y@1()
IL_002c: call class [runtime]System.Collections.IEqualityComparer [FSharp.Core]Microsoft.FSharp.Core.LanguagePrimitives::get_GenericEqualityComparer()
IL_0031: call instance bool valuetype [FSharp.Core]Microsoft.FSharp.Core.FSharpValueOption`1<class [runtime]System.Tuple`2<int32,int32>>::Equals(valuetype [FSharp.Core]Microsoft.FSharp.Core.FSharpValueOption`1<!0>,
class [runtime]System.Collections.IEqualityComparer)
IL_0036: stsfld bool Program::arg@1
IL_003b: ret
}
""" ]

[<Fact>]
let ``Another assembly``() =
let module1 =
[<InlineData(true)>] // RealSig
[<InlineData(false)>] // Regular
[<Theory>]
let ``Another Assembly - record with private fields`` (realsig) =
let library =
FSharpWithFileName "Module1.fs"
"""
module Module1
[<Struct>]
type Struct(v: int, u: int) =
member _.V = v
member _.U = u """
type Value =
private { value: uint32 }
static member Zero = { value = 0u }
static member Create(value: int) = { value = uint value } """
|> withRealInternalSignature realsig
|> withOptimize
|> asLibrary
|> withName "module1"

let module2 =
let mainModule =
FSharpWithFileName "Program.fs"
"""
Module1.Struct(1, 2) = Module1.Struct(2, 3) |> ignore"""
$"""
open Module1
Value.Zero = Value.Create 0 |> ignore"""

module2
|> withReferences [module1]
mainModule
|> withRealInternalSignature realsig
|> withReferences [ library ]
|> compileExeAndRun
|> shouldSucceed

[<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
[<Theory>]
let ``Generated typed Equals`` (realsig, typeScope, targetVisibility) =
let library =
FSharpWithFileName "Program.fs"
$"""
module Module1 =
[<Struct>]
type {typeScope} Struct(v: int, u: int) =
member _.V = v
member _.U = u"""

library
|> asExe
|> withNoWarn 988
|> withRealInternalSignature realsig
|> withOptimize
|> compile
|> shouldSucceed
|> verifyIL [
$"""
.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 )
.maxstack 8
IL_0000: ldarg.0
IL_0001: ldfld int32 Program/Module1/Struct::v
IL_0006: ldarga.s obj
IL_0008: ldfld int32 Program/Module1/Struct::v
IL_000d: bne.un.s IL_001f
IL_000f: ldarg.0
IL_0010: ldfld int32 Program/Module1/Struct::u
IL_0015: ldarga.s obj
IL_0017: ldfld int32 Program/Module1/Struct::u
IL_001c: ceq
IL_001e: ret
IL_001f: ldc.i4.0
IL_0020: ret
}} """ ]


[<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
[<Theory>]
let ``Record with various fields`` (realsig, fieldScope) =

let mainModule =
FSharpWithFileName "Program.fs"
$"""
module Module1 =
type Value =
{fieldScope} {{ value: uint32 }}
static member Zero = {{ value = 0u }}
static member Create(value: int) = {{ value = uint value }}
Value.Zero = Value.Create 0 |> ignore
printfn "Hello, World" """

mainModule
|> withRealInternalSignature realsig
|> compileExeAndRun
|> shouldSucceed
|> verifyIL [ """
.method assembly specialname static void staticInitialization@() cil managed
{
.maxstack 8
IL_0000: ldc.i4.1
IL_0001: ldc.i4.2
IL_0002: newobj instance void [module1]Module1/Struct::.ctor(int32,
int32)
IL_0007: stsfld valuetype [module1]Module1/Struct Program::x@1
IL_000c: ldc.i4.2
IL_000d: ldc.i4.3
IL_000e: newobj instance void [module1]Module1/Struct::.ctor(int32,
int32)
IL_0013: stsfld valuetype [module1]Module1/Struct Program::y@1
IL_0018: ldsflda valuetype [module1]Module1/Struct Program::x@1
IL_001d: call valuetype [module1]Module1/Struct Program::get_y@1()
IL_0022: call class [runtime]System.Collections.IEqualityComparer [FSharp.Core]Microsoft.FSharp.Core.LanguagePrimitives::get_GenericEqualityComparer()
IL_0027: call instance bool [module1]Module1/Struct::Equals(valuetype [module1]Module1/Struct,
class [runtime]System.Collections.IEqualityComparer)
IL_002c: stsfld bool Program::arg@1
IL_0031: ret
}
""" ]
|> verifyIL [
"""
.method public hidebysig virtual final instance bool Equals(class Program/Module1/Value obj) cil managed
{
.custom instance void [runtime]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( 01 00 00 00 )
.maxstack 8
IL_0000: ldarg.0
IL_0001: brfalse.s IL_0017
IL_0003: ldarg.1
IL_0004: brfalse.s IL_0015
IL_0006: ldarg.0
IL_0007: ldfld uint32 Program/Module1/Value::value@
IL_000c: ldarg.1
IL_000d: ldfld uint32 Program/Module1/Value::value@
IL_0012: ceq
IL_0014: ret
IL_0015: ldc.i4.0
IL_0016: ret
IL_0017: ldarg.1
IL_0018: ldnull
IL_0019: cgt.un
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
[<Theory>]
let ``scoped type arg`` (realsig, argScope, targetVisibility) =
let mainModule =
FSharpWithFileName "Program.fs"
$"""
module IPartialEqualityComparer =
open System.Collections.Generic
[<StructuralEquality; NoComparison>]
type {argScope} WrapType<'T> = Wrap of 'T
"""
mainModule
|> asExe
|> withNoWarn 988
|> withRealInternalSignature realsig
|> withOptimize
|> compile
|> shouldSucceed
|> verifyIL [
$"""
.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 )
.maxstack 5
.locals init (class Program/IPartialEqualityComparer/WrapType`1<!T> V_0,
class Program/IPartialEqualityComparer/WrapType`1<!T> V_1,
!T V_2,
!T V_3)
IL_0000: ldarg.0
IL_0001: brfalse.s IL_0027
IL_0003: ldarg.1
IL_0004: brfalse.s IL_0025
IL_0006: ldarg.0
IL_0007: pop
IL_0008: ldarg.0
IL_0009: stloc.0
IL_000a: ldarg.1
IL_000b: stloc.1
IL_000c: ldloc.0
IL_000d: ldfld !0 class Program/IPartialEqualityComparer/WrapType`1<!T>::item
IL_0012: stloc.2
IL_0013: ldloc.1
IL_0014: ldfld !0 class Program/IPartialEqualityComparer/WrapType`1<!T>::item
IL_0019: stloc.3
IL_001a: ldarg.2
IL_001b: ldloc.2
IL_001c: ldloc.3
IL_001d: tail.
IL_001f: call bool [FSharp.Core]Microsoft.FSharp.Core.LanguagePrimitives/HashCompare::GenericEqualityWithComparerIntrinsic<!T>(class [runtime]System.Collections.IEqualityComparer,
!!0,
!!0)
IL_0024: ret
IL_0025: ldc.i4.0
IL_0026: ret
IL_0027: ldarg.1
IL_0028: ldnull
IL_0029: cgt.un
IL_002b: ldc.i4.0
IL_002c: ceq
IL_002e: ret
}} """ ]
1 change: 0 additions & 1 deletion tests/FSharp.Compiler.ComponentTests/EmittedIL/GenericComparison/GenericComparison.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@ module GenericComparison =
|> verifyCompilation



// SOURCE=Hash01.fsx SCFLAGS="-a -g --optimize+" COMPILE_ONLY=1 POSTCMD="..\\CompareIL.cmd Hash01.dll" # Hash01.fs -
[<Theory; Directory(__SOURCE_DIRECTORY__, Includes=[|"Hash01.fsx"|])>]
let ``Hash01_fsx`` compilation =
Expand Down
6 changes: 6 additions & 0 deletions tests/FSharp.Test.Utilities/Compiler.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -601,6 +601,12 @@ module rec Compiler =
| FS fs -> FS { fs with Options = fs.Options @ ["--realsig+"] }
| _ -> failwith "withRealInternalSignatureOn only supported by f#"

let withRealInternalSignature (realSig: bool) (cUnit: CompilationUnit) : CompilationUnit =
if realSig then
cUnit |> withRealInternalSignatureOn
else
cUnit |> withRealInternalSignatureOff

let asExe (cUnit: CompilationUnit) : CompilationUnit =
withOutputType CompileOutput.Exe cUnit

Expand Down

0 comments on commit 44e7bfa

Please sign in to comment.