From ed7905d266cc88147063fe220aac5bcf6470e675 Mon Sep 17 00:00:00 2001 From: Edgar Gonzalez Date: Tue, 30 Jul 2024 20:41:16 +0100 Subject: [PATCH 1/2] C# protected property can be assigned in a F# inherit constructor call (#17391) * C# protected property can be assigned in a F# inherit constructor call * Failing test * Keep the access rights even if this is no a base call * Keep the access rights even if this is no a base call * Update check * more tests * release notes * inline the check, so it's only performed if LHS is true --------- Co-authored-by: Petr --- .../.FSharp.Compiler.Service/9.0.100.md | 1 + src/Compiler/Checking/MethodCalls.fs | 2 +- src/Compiler/TypedTree/TypedTree.fs | 2 +- .../AccessibilityAnnotations/Basic/Basic.fs | 39 +++++++++++++++++++ 4 files changed, 42 insertions(+), 2 deletions(-) diff --git a/docs/release-notes/.FSharp.Compiler.Service/9.0.100.md b/docs/release-notes/.FSharp.Compiler.Service/9.0.100.md index 8d206a1bd1e..9b71a9c31b1 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/9.0.100.md +++ b/docs/release-notes/.FSharp.Compiler.Service/9.0.100.md @@ -3,6 +3,7 @@ * Compiler hangs when compiling inline recursive invocation ([Issue #17376](https://github.com/dotnet/fsharp/issues/17376), [PR #17394](https://github.com/dotnet/fsharp/pull/17394)) * 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)) ### Added diff --git a/src/Compiler/Checking/MethodCalls.fs b/src/Compiler/Checking/MethodCalls.fs index 25f9d420f2e..f75fd2fb6be 100644 --- a/src/Compiler/Checking/MethodCalls.fs +++ b/src/Compiler/Checking/MethodCalls.fs @@ -1237,7 +1237,7 @@ let MethInfoChecks g amap isInstance tyargsOpt objArgs ad m (minfo: MethInfo) = AccessibleFrom(paths, None) | _ -> ad - if not (IsTypeAndMethInfoAccessible amap m adOriginal ad minfo) then + if not (minfo.IsProtectedAccessibility && minfo.LogicalName.StartsWithOrdinal("set_")) && not(IsTypeAndMethInfoAccessible amap m adOriginal ad minfo) then error (Error (FSComp.SR.tcMethodNotAccessible(minfo.LogicalName), m)) if isAnyTupleTy g minfo.ApparentEnclosingType && not minfo.IsExtensionMember && diff --git a/src/Compiler/TypedTree/TypedTree.fs b/src/Compiler/TypedTree/TypedTree.fs index 6f897c7889a..37edb437867 100644 --- a/src/Compiler/TypedTree/TypedTree.fs +++ b/src/Compiler/TypedTree/TypedTree.fs @@ -90,7 +90,7 @@ type ValBaseOrThisInfo = /// Indicates a normal value | NormalVal - /// Indicates the 'this' value specified in a memberm e.g. 'x' in 'member x.M() = 1' + /// Indicates the 'this' value specified in a member e.g. 'x' in 'member x.M() = 1' | MemberThisVal /// Flags on values diff --git a/tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/AccessibilityAnnotations/Basic/Basic.fs b/tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/AccessibilityAnnotations/Basic/Basic.fs index 37add29d802..869e4ba9930 100644 --- a/tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/AccessibilityAnnotations/Basic/Basic.fs +++ b/tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/AccessibilityAnnotations/Basic/Basic.fs @@ -188,3 +188,42 @@ module AccessibilityAnnotations_Basic = compilation |> verifyCompileAndRun |> shouldSucceed + + [] + let ``C# protected property can be assigned in a F# inherit constructor call`` () = + + let csharp = + CSharp + """ +namespace Consumer +{ + public class Foo + { + protected string Value { get; set; } = ""; + } +} +""" + |> withName "CSLib" + + let fsharp = + FSharp + """ +module Hello +open Consumer + +type Bar() = + inherit Foo(Value = "Works") + +type Bar2() as this = + inherit Foo() + do this.Value <- "Works" + +{ new Foo(Value = "OK") with member x.ToString() = "OK" } |> ignore +""" + |> withLangVersion80 + |> withName "FSLib" + |> withReferences [ csharp ] + + fsharp + |> compile + |> shouldSucceed From 44e7bfa1208a768fb63fcf458b0e9820ba44ed53 Mon Sep 17 00:00:00 2001 From: "Kevin Ransom (msft)" Date: Wed, 31 Jul 2024 04:24:46 -0700 Subject: [PATCH 2/2] Fixes #17447 -MethodAccessException on equality comparison of a record with private fields (#17467) * Fix17447 * tests + readme --- .../.FSharp.Compiler.Service/9.0.100.md | 1 + .../Checking/AugmentWithHashCompare.fs | 4 +- .../GenericComparison/CrossAssembly.fs | 272 ++++++++++++++---- .../GenericComparison/GenericComparison.fs | 1 - tests/FSharp.Test.Utilities/Compiler.fs | 6 + 5 files changed, 221 insertions(+), 63 deletions(-) diff --git a/docs/release-notes/.FSharp.Compiler.Service/9.0.100.md b/docs/release-notes/.FSharp.Compiler.Service/9.0.100.md index 9b71a9c31b1..c1ba8b4a6fc 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/9.0.100.md +++ b/docs/release-notes/.FSharp.Compiler.Service/9.0.100.md @@ -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 diff --git a/src/Compiler/Checking/AugmentWithHashCompare.fs b/src/Compiler/Checking/AugmentWithHashCompare.fs index 8d82a32ac4e..c64b0001a29 100644 --- a/src/Compiler/Checking/AugmentWithHashCompare.fs +++ b/src/Compiler/Checking/AugmentWithHashCompare.fs @@ -1333,7 +1333,7 @@ let MakeValsForEqualsAugmentation g (tcref: TyconRef) = g tcref ty - vis + tcref.Accessibility (if tcref.Deref.IsFSharpException then None else @@ -1376,7 +1376,7 @@ let MakeValsForEqualityWithComparerAugmentation g (tcref: TyconRef) = g tcref ty - vis + tcref.Accessibility // This doesn't implement any interface. None "Equals" diff --git a/tests/FSharp.Compiler.ComponentTests/EmittedIL/GenericComparison/CrossAssembly.fs b/tests/FSharp.Compiler.ComponentTests/EmittedIL/GenericComparison/CrossAssembly.fs index 8e2cd0853c0..a93d48635fe 100644 --- a/tests/FSharp.Compiler.ComponentTests/EmittedIL/GenericComparison/CrossAssembly.fs +++ b/tests/FSharp.Compiler.ComponentTests/EmittedIL/GenericComparison/CrossAssembly.fs @@ -7,87 +7,239 @@ open FSharp.Test.Compiler module GenericComparisonCrossAssembly = - [] - let ``fslib``() = + [] // RealSig + [] // Regular + [] + 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::.ctor(!0, - !1) - IL_0007: call valuetype [FSharp.Core]Microsoft.FSharp.Core.FSharpValueOption`1 valuetype [FSharp.Core]Microsoft.FSharp.Core.FSharpValueOption`1>::NewValueSome(!0) - IL_000c: stsfld valuetype [FSharp.Core]Microsoft.FSharp.Core.FSharpValueOption`1> Program::x@1 - IL_0011: ldc.i4.2 - IL_0012: ldc.i4.3 - IL_0013: newobj instance void class [runtime]System.Tuple`2::.ctor(!0, - !1) - IL_0018: call valuetype [FSharp.Core]Microsoft.FSharp.Core.FSharpValueOption`1 valuetype [FSharp.Core]Microsoft.FSharp.Core.FSharpValueOption`1>::NewValueSome(!0) - IL_001d: stsfld valuetype [FSharp.Core]Microsoft.FSharp.Core.FSharpValueOption`1> Program::y@1 - IL_0022: ldsflda valuetype [FSharp.Core]Microsoft.FSharp.Core.FSharpValueOption`1> Program::x@1 - IL_0027: call valuetype [FSharp.Core]Microsoft.FSharp.Core.FSharpValueOption`1> 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>::Equals(valuetype [FSharp.Core]Microsoft.FSharp.Core.FSharpValueOption`1, - class [runtime]System.Collections.IEqualityComparer) - IL_0036: stsfld bool Program::arg@1 - IL_003b: ret - } """ ] - [] - let ``Another assembly``() = - let module1 = + [] // RealSig + [] // Regular + [] + let ``Another Assembly - record with private fields`` (realsig) = + let library = FSharpWithFileName "Module1.fs" """ module Module1 - [] - 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 + + [] // Legacy, private WrapType, private visibility in IL + [] // RealSig, internal WrapType, assembly visibility in IL + [] // Legacy, public WrapType, public visibility in IL + [] // RealSig, private WrapType, private visibility in IL + [] // RealSig, internal WrapType, assembly visibility in IL + [] // RealSig, public WrapType, public visibility in IL + [] + let ``Generated typed Equals`` (realsig, typeScope, targetVisibility) = + let library = + FSharpWithFileName "Program.fs" + $""" +module Module1 = + + [] + 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 + }} """ ] + + + [] // Legacy, private record fields, private visibility in IL + [] // RealSig, internal record fields, assembly visibility in IL + [] // Legacy, public record fields, public visibility in IL + [] // RealSig, private record fields, private visibility in IL + [] // RealSig, internal record fields, assembly visibility in IL + [] // RealSig, public record fields, public visibility in IL + [] + 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) + """ ] + + + [] // Legacy, private WrapType, private visibility in IL + [] // RealSig, internal WrapType, assembly visibility in IL + [] // Legacy, public WrapType, public visibility in IL + [] // RealSig, private WrapType, private visibility in IL + [] // RealSig, internal WrapType, assembly visibility in IL + [] // RealSig, public WrapType, public visibility in IL + [] + let ``scoped type arg`` (realsig, argScope, targetVisibility) = + let mainModule = + FSharpWithFileName "Program.fs" + $""" +module IPartialEqualityComparer = + open System.Collections.Generic + + [] + 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 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 V_0, + class Program/IPartialEqualityComparer/WrapType`1 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::item + IL_0012: stloc.2 + IL_0013: ldloc.1 + IL_0014: ldfld !0 class Program/IPartialEqualityComparer/WrapType`1::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(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 + }} """ ] diff --git a/tests/FSharp.Compiler.ComponentTests/EmittedIL/GenericComparison/GenericComparison.fs b/tests/FSharp.Compiler.ComponentTests/EmittedIL/GenericComparison/GenericComparison.fs index 2fd53e46b56..091817ee76d 100644 --- a/tests/FSharp.Compiler.ComponentTests/EmittedIL/GenericComparison/GenericComparison.fs +++ b/tests/FSharp.Compiler.ComponentTests/EmittedIL/GenericComparison/GenericComparison.fs @@ -85,7 +85,6 @@ module GenericComparison = |> verifyCompilation - // SOURCE=Hash01.fsx SCFLAGS="-a -g --optimize+" COMPILE_ONLY=1 POSTCMD="..\\CompareIL.cmd Hash01.dll" # Hash01.fs - [] let ``Hash01_fsx`` compilation = diff --git a/tests/FSharp.Test.Utilities/Compiler.fs b/tests/FSharp.Test.Utilities/Compiler.fs index 4a2d52ef4d4..9fb050b09df 100644 --- a/tests/FSharp.Test.Utilities/Compiler.fs +++ b/tests/FSharp.Test.Utilities/Compiler.fs @@ -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