From d8d26b8ba4d13942f1447b8a697a9e0ddaed66e1 Mon Sep 17 00:00:00 2001 From: edgarfgp Date: Mon, 8 Jul 2024 21:17:47 +0100 Subject: [PATCH 1/8] C# protected property can be assigned in a F# inherit constructor call --- src/Compiler/Checking/TypeRelations.fs | 20 +++++------ .../AccessibilityAnnotations/Basic/Basic.fs | 33 +++++++++++++++++++ 2 files changed, 43 insertions(+), 10 deletions(-) diff --git a/src/Compiler/Checking/TypeRelations.fs b/src/Compiler/Checking/TypeRelations.fs index 49afd4825b6..d4faa21d944 100644 --- a/src/Compiler/Checking/TypeRelations.fs +++ b/src/Compiler/Checking/TypeRelations.fs @@ -117,16 +117,16 @@ let rec TypeFeasiblySubsumesType ndeep g amap m ty1 canCoerce ty2 = | _ -> // F# reference types are subtypes of type 'obj' - (isObjTy g ty1 && (canCoerce = CanCoerce || isRefTy g ty2)) - || - (isAppTy g ty2 && - (canCoerce = CanCoerce || isRefTy g ty2) && - begin match GetSuperTypeOfType g amap m ty2 with - | None -> false - | Some ty -> TypeFeasiblySubsumesType (ndeep+1) g amap m ty1 NoCoerce ty - end || - ty2 |> GetImmediateInterfacesOfType SkipUnrefInterfaces.Yes g amap m - |> List.exists (TypeFeasiblySubsumesType (ndeep+1) g amap m ty1 NoCoerce)) + let isObjAndRefType = (isObjTy g ty1 && (canCoerce = CanCoerce || isRefTy g ty2)) + let isAppTy = isAppTy g ty2 + let isAppAndRefType = isAppTy && (canCoerce = CanCoerce || isRefTy g ty2) + let superType = + match GetSuperTypeOfType g amap m ty2 with + | None -> false + | Some ty -> TypeFeasiblySubsumesType (ndeep+1) g amap m ty1 NoCoerce ty + let immediateInterfacesOfType = ty2 |> GetImmediateInterfacesOfType SkipUnrefInterfaces.Yes g amap m + let immediateInterfaces = immediateInterfacesOfType |> List.exists (TypeFeasiblySubsumesType (ndeep+1) g amap m ty1 NoCoerce) + isObjAndRefType || isAppAndRefType || (isAppAndRefType && superType || immediateInterfaces) /// Choose solutions for Expr.TyChoose type "hidden" variables introduced /// by letrec nodes. Also used by the pattern match compiler to choose type 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..fd0843327d1 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,36 @@ 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 = "Fails") +""" + |> withLangVersion80 + |> withName "FSLib" + |> withReferences [ csharp ] + + fsharp + |> compile + |> shouldSucceed From b1db75be19a2dc43f67b73c6594fb33bfb63d433 Mon Sep 17 00:00:00 2001 From: edgarfgp Date: Mon, 8 Jul 2024 21:32:34 +0100 Subject: [PATCH 2/8] Failing test --- src/Compiler/Checking/TypeRelations.fs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Compiler/Checking/TypeRelations.fs b/src/Compiler/Checking/TypeRelations.fs index d4faa21d944..289fb5085c8 100644 --- a/src/Compiler/Checking/TypeRelations.fs +++ b/src/Compiler/Checking/TypeRelations.fs @@ -126,7 +126,7 @@ let rec TypeFeasiblySubsumesType ndeep g amap m ty1 canCoerce ty2 = | Some ty -> TypeFeasiblySubsumesType (ndeep+1) g amap m ty1 NoCoerce ty let immediateInterfacesOfType = ty2 |> GetImmediateInterfacesOfType SkipUnrefInterfaces.Yes g amap m let immediateInterfaces = immediateInterfacesOfType |> List.exists (TypeFeasiblySubsumesType (ndeep+1) g amap m ty1 NoCoerce) - isObjAndRefType || isAppAndRefType || (isAppAndRefType && superType || immediateInterfaces) + isObjAndRefType || (isAppAndRefType && superType || immediateInterfaces) /// Choose solutions for Expr.TyChoose type "hidden" variables introduced /// by letrec nodes. Also used by the pattern match compiler to choose type From 1f2f663c341542cf50b4d6dc6b348691b5b47e2c Mon Sep 17 00:00:00 2001 From: edgarfgp Date: Tue, 9 Jul 2024 15:54:38 +0100 Subject: [PATCH 3/8] Keep the access rights even if this is no a base call --- src/Compiler/Checking/MethodCalls.fs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Compiler/Checking/MethodCalls.fs b/src/Compiler/Checking/MethodCalls.fs index 46f32c53431..f36dcf7366e 100644 --- a/src/Compiler/Checking/MethodCalls.fs +++ b/src/Compiler/Checking/MethodCalls.fs @@ -1225,7 +1225,7 @@ let MethInfoChecks g amap isInstance tyargsOpt objArgs ad m (minfo: MethInfo) = elif IsBaseCall objArgs then ad else - AccessibleFrom(paths, None) + AccessibleFrom(paths, Some tcref) | _ -> ad if not (IsTypeAndMethInfoAccessible amap m adOriginal ad minfo) then From c8462be0ad441e9369e66cfa0027e8533b6cd9e1 Mon Sep 17 00:00:00 2001 From: edgarfgp Date: Tue, 9 Jul 2024 15:54:38 +0100 Subject: [PATCH 4/8] Keep the access rights even if this is no a base call --- src/Compiler/Checking/TypeRelations.fs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/Compiler/Checking/TypeRelations.fs b/src/Compiler/Checking/TypeRelations.fs index 289fb5085c8..49afd4825b6 100644 --- a/src/Compiler/Checking/TypeRelations.fs +++ b/src/Compiler/Checking/TypeRelations.fs @@ -117,16 +117,16 @@ let rec TypeFeasiblySubsumesType ndeep g amap m ty1 canCoerce ty2 = | _ -> // F# reference types are subtypes of type 'obj' - let isObjAndRefType = (isObjTy g ty1 && (canCoerce = CanCoerce || isRefTy g ty2)) - let isAppTy = isAppTy g ty2 - let isAppAndRefType = isAppTy && (canCoerce = CanCoerce || isRefTy g ty2) - let superType = - match GetSuperTypeOfType g amap m ty2 with - | None -> false - | Some ty -> TypeFeasiblySubsumesType (ndeep+1) g amap m ty1 NoCoerce ty - let immediateInterfacesOfType = ty2 |> GetImmediateInterfacesOfType SkipUnrefInterfaces.Yes g amap m - let immediateInterfaces = immediateInterfacesOfType |> List.exists (TypeFeasiblySubsumesType (ndeep+1) g amap m ty1 NoCoerce) - isObjAndRefType || (isAppAndRefType && superType || immediateInterfaces) + (isObjTy g ty1 && (canCoerce = CanCoerce || isRefTy g ty2)) + || + (isAppTy g ty2 && + (canCoerce = CanCoerce || isRefTy g ty2) && + begin match GetSuperTypeOfType g amap m ty2 with + | None -> false + | Some ty -> TypeFeasiblySubsumesType (ndeep+1) g amap m ty1 NoCoerce ty + end || + ty2 |> GetImmediateInterfacesOfType SkipUnrefInterfaces.Yes g amap m + |> List.exists (TypeFeasiblySubsumesType (ndeep+1) g amap m ty1 NoCoerce)) /// Choose solutions for Expr.TyChoose type "hidden" variables introduced /// by letrec nodes. Also used by the pattern match compiler to choose type From 5134a92567b8b2a70df325f12f81eeba734fb1bf Mon Sep 17 00:00:00 2001 From: Edgar Gonzalez Date: Sun, 28 Jul 2024 19:45:45 +0100 Subject: [PATCH 5/8] Update check --- src/Compiler/Checking/MethodCalls.fs | 5 +++-- src/Compiler/TypedTree/TypedTree.fs | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Compiler/Checking/MethodCalls.fs b/src/Compiler/Checking/MethodCalls.fs index 6354d770bb0..0a54f151c88 100644 --- a/src/Compiler/Checking/MethodCalls.fs +++ b/src/Compiler/Checking/MethodCalls.fs @@ -1234,10 +1234,11 @@ let MethInfoChecks g amap isInstance tyargsOpt objArgs ad m (minfo: MethInfo) = elif IsBaseCall objArgs then ad else - AccessibleFrom(paths, Some tcref) + AccessibleFrom(paths, None) | _ -> ad - if not (IsTypeAndMethInfoAccessible amap m adOriginal ad minfo) then + let isProtectedSetter = minfo.IsProtectedAccessibility && minfo.LogicalName.StartsWithOrdinal("set_") + if not (IsTypeAndMethInfoAccessible amap m adOriginal ad minfo) && not isProtectedSetter 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 From 6857513d5f5eba73e69bdf5af1886f65d7a944da Mon Sep 17 00:00:00 2001 From: Edgar Gonzalez Date: Sun, 28 Jul 2024 21:05:55 +0100 Subject: [PATCH 6/8] more tests --- .../AccessibilityAnnotations/Basic/Basic.fs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) 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 fd0843327d1..869e4ba9930 100644 --- a/tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/AccessibilityAnnotations/Basic/Basic.fs +++ b/tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/AccessibilityAnnotations/Basic/Basic.fs @@ -212,7 +212,13 @@ module Hello open Consumer type Bar() = - inherit Foo(Value = "Fails") + 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" From b6a383e64e15d6b1b72f6fe1ee6cf39d7cf4e784 Mon Sep 17 00:00:00 2001 From: Edgar Gonzalez Date: Sun, 28 Jul 2024 21:09:43 +0100 Subject: [PATCH 7/8] release notes --- docs/release-notes/.FSharp.Compiler.Service/9.0.100.md | 1 + 1 file changed, 1 insertion(+) 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 1166f8b413e..e888497ea65 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 From be81dc1ecefa494605a4dbd8f7e8c239e5185208 Mon Sep 17 00:00:00 2001 From: Edgar Gonzalez Date: Mon, 29 Jul 2024 21:11:32 +0100 Subject: [PATCH 8/8] inline the check, so it's only performed if LHS is true --- src/Compiler/Checking/MethodCalls.fs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Compiler/Checking/MethodCalls.fs b/src/Compiler/Checking/MethodCalls.fs index 0a54f151c88..f75fd2fb6be 100644 --- a/src/Compiler/Checking/MethodCalls.fs +++ b/src/Compiler/Checking/MethodCalls.fs @@ -1237,8 +1237,7 @@ let MethInfoChecks g amap isInstance tyargsOpt objArgs ad m (minfo: MethInfo) = AccessibleFrom(paths, None) | _ -> ad - let isProtectedSetter = minfo.IsProtectedAccessibility && minfo.LogicalName.StartsWithOrdinal("set_") - if not (IsTypeAndMethInfoAccessible amap m adOriginal ad minfo) && not isProtectedSetter 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 &&