Skip to content

Commit 1ecc201

Browse files
authored
Merge pull request #8354 from smoothdeveloper/fix-for-issue-8351
Fix for issue 8351
2 parents 4456e0a + 8bfcdd6 commit 1ecc201

10 files changed

Lines changed: 375 additions & 13 deletions

File tree

src/fsharp/AccessibilityLogic.fs

Lines changed: 52 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -249,12 +249,53 @@ let private IsILMethInfoAccessible g amap m adType ad ilminfo =
249249
let GetILAccessOfILPropInfo (ILPropInfo(tinfo, pdef)) =
250250
let tdef = tinfo.RawMetadata
251251
let ilAccess =
252-
match pdef.GetMethod with
253-
| Some mref -> (resolveILMethodRef tdef mref).Access
254-
| None ->
255-
match pdef.SetMethod with
256-
| None -> ILMemberAccess.Public
257-
| Some mref -> (resolveILMethodRef tdef mref).Access
252+
match pdef.GetMethod, pdef.SetMethod with
253+
| Some mref, None
254+
| None, Some mref -> (resolveILMethodRef tdef mref).Access
255+
256+
| Some mrefGet, Some mrefSet ->
257+
//
258+
// Dotnet properties have a getter and a setter method, each of which can have a separate visibility public, protected, private etc ...
259+
// This code computes the visibility for the property by choosing the most visible method. This approximation is usefull for cases
260+
// where the compiler needs to know the visibility of the property.
261+
// The specific ordering for choosing the most visible is:
262+
// ILMemberAccess.Public,
263+
// ILMemberAccess.FamilyOrAssembly
264+
// ILMemberAccess.Assembly
265+
// ILMemberAccess.Family
266+
// ILMemberAccess.FamilyAndAssembly
267+
// ILMemberAccess.Private
268+
// ILMemberAccess.CompilerControlled
269+
//
270+
let getA = (resolveILMethodRef tdef mrefGet).Access
271+
let setA = (resolveILMethodRef tdef mrefSet).Access
272+
273+
// Use the accessors to determine the visibility of the property.
274+
// N.B. It is critical to keep the ordering in decreasing visibility order in the following match expression
275+
match getA, setA with
276+
| ILMemberAccess.Public, _
277+
| _, ILMemberAccess.Public -> ILMemberAccess.Public
278+
279+
| ILMemberAccess.FamilyOrAssembly, _
280+
| _, ILMemberAccess.FamilyOrAssembly -> ILMemberAccess.FamilyOrAssembly
281+
282+
| ILMemberAccess.Assembly, _
283+
| _, ILMemberAccess.Assembly -> ILMemberAccess.Assembly
284+
285+
| ILMemberAccess.Family, _
286+
| _, ILMemberAccess.Family -> ILMemberAccess.Family
287+
288+
| ILMemberAccess.FamilyAndAssembly, _
289+
| _, ILMemberAccess.FamilyAndAssembly -> ILMemberAccess.FamilyAndAssembly
290+
291+
| ILMemberAccess.Private, _
292+
| _, ILMemberAccess.Private -> ILMemberAccess.Private
293+
294+
| ILMemberAccess.CompilerControlled, _
295+
| _, ILMemberAccess.CompilerControlled -> ILMemberAccess.CompilerControlled
296+
297+
| None, None -> ILMemberAccess.Public
298+
258299
ilAccess
259300

260301
let IsILPropInfoAccessible g amap m ad pinfo =
@@ -323,8 +364,11 @@ let IsMethInfoAccessible amap m ad minfo = IsTypeAndMethInfoAccessible amap m ad
323364

324365
let IsPropInfoAccessible g amap m ad = function
325366
| ILProp ilpinfo -> IsILPropInfoAccessible g amap m ad ilpinfo
326-
| FSProp (_, _, Some vref, _)
327-
| FSProp (_, _, _, Some vref) -> IsValAccessible ad vref
367+
| FSProp (_, _, Some vref, None)
368+
| FSProp (_, _, None, Some vref) -> IsValAccessible ad vref
369+
| FSProp (_, _, Some vrefGet, Some vrefSet) ->
370+
// pick most accessible
371+
IsValAccessible ad vrefGet || IsValAccessible ad vrefSet
328372
#if !NO_EXTENSIONTYPING
329373
| ProvidedProp (amap, tppi, m) as pp->
330374
let access =
@@ -343,4 +387,3 @@ let IsPropInfoAccessible g amap m ad = function
343387

344388
let IsFieldInfoAccessible ad (rfref:RecdFieldInfo) =
345389
IsAccessible ad rfref.RecdField.Accessibility
346-

tests/FSharp.Test.Utilities/TestFramework.fs

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,9 @@ module Commands =
9696
let csc exec cscExe flags srcFiles =
9797
exec cscExe (sprintf "%s %s" flags (srcFiles |> Seq.ofList |> String.concat " "))
9898

99+
let vbc exec vbcExe flags srcFiles =
100+
exec vbcExe (sprintf "%s %s" flags (srcFiles |> Seq.ofList |> String.concat " "))
101+
99102
let fsi exec fsiExe flags sources =
100103
exec fsiExe (sprintf "%s %s" flags (sources |> Seq.ofList |> String.concat " "))
101104

@@ -123,6 +126,8 @@ type TestConfig =
123126
{ EnvironmentVariables : Map<string, string>
124127
CSC : string
125128
csc_flags : string
129+
VBC : string
130+
vbc_flags : string
126131
BUILD_CONFIG : string
127132
FSC : string
128133
fsc_flags : string
@@ -213,7 +218,8 @@ let config configurationName envVars =
213218
let artifactsPath = repoRoot ++ "artifacts"
214219
let artifactsBinPath = artifactsPath ++ "bin"
215220
let coreClrRuntimePackageVersion = "3.0.0-preview-27318-01"
216-
let csc_flags = "/nologo"
221+
let csc_flags = "/nologo"
222+
let vbc_flags = "/nologo"
217223
let fsc_flags = "-r:System.Core.dll --nowarn:20 --define:COMPILED"
218224
let fsi_flags = "-r:System.Core.dll --nowarn:20 --define:INTERACTIVE --maxerrors:1 --abortonerror"
219225
let operatingSystem = getOperatingSystem ()
@@ -223,6 +229,7 @@ let config configurationName envVars =
223229
let requirePackage = requireFile packagesDir
224230
let requireArtifact = requireFile artifactsBinPath
225231
let CSC = requirePackage ("Microsoft.Net.Compilers" ++ "2.7.0" ++ "tools" ++ "csc.exe")
232+
let VBC = requirePackage ("Microsoft.Net.Compilers" ++ "2.7.0" ++ "tools" ++ "vbc.exe")
226233
let ILDASM_EXE = if operatingSystem = "win" then "ildasm.exe" else "ildasm"
227234
let ILDASM = requirePackage (("runtime." + operatingSystem + "-" + architectureMoniker + ".Microsoft.NETCore.ILDAsm") ++ coreClrRuntimePackageVersion ++ "runtimes" ++ (operatingSystem + "-" + architectureMoniker) ++ "native" ++ ILDASM_EXE)
228235
let ILASM_EXE = if operatingSystem = "win" then "ilasm.exe" else "ilasm"
@@ -231,6 +238,7 @@ let config configurationName envVars =
231238
let coreclrdll = requirePackage (("runtime." + operatingSystem + "-" + architectureMoniker + ".Microsoft.NETCore.Runtime.CoreCLR") ++ coreClrRuntimePackageVersion ++ "runtimes" ++ (operatingSystem + "-" + architectureMoniker) ++ "native" ++ CORECLR_DLL)
232239
let PEVERIFY_EXE = if operatingSystem = "win" then "PEVerify.exe" else "PEVerify"
233240
let PEVERIFY = requireArtifact ("PEVerify" ++ configurationName ++ peverifyArchitecture ++ PEVERIFY_EXE)
241+
// let FSI_FOR_SCRIPTS = artifactsBinPath ++ "fsi" ++ configurationName ++ fsiArchitecture ++ "fsi.exe"
234242
let FSharpBuild = requireArtifact ("FSharp.Build" ++ configurationName ++ fsharpBuildArchitecture ++ "FSharp.Build.dll")
235243
let FSharpCompilerInteractiveSettings = requireArtifact ("FSharp.Compiler.Interactive.Settings" ++ configurationName ++ fsharpCompilerInteractiveSettingsArchitecture ++ "FSharp.Compiler.Interactive.Settings.dll")
236244

@@ -266,7 +274,8 @@ let config configurationName envVars =
266274
ILDASM = ILDASM
267275
ILASM = ILASM
268276
PEVERIFY = PEVERIFY
269-
CSC = CSC
277+
VBC = VBC
278+
CSC = CSC
270279
BUILD_CONFIG = configurationName
271280
FSC = FSC
272281
FSI = FSI
@@ -277,9 +286,10 @@ let config configurationName envVars =
277286
FSharpBuild = FSharpBuild
278287
FSharpCompilerInteractiveSettings = FSharpCompilerInteractiveSettings
279288
csc_flags = csc_flags
280-
fsc_flags = fsc_flags
289+
fsc_flags = fsc_flags
281290
fsi_flags = fsi_flags
282-
Directory=""
291+
vbc_flags = vbc_flags
292+
Directory=""
283293
DotNetExe = dotNetExe
284294
DefaultPlatform = defaultPlatform }
285295

@@ -506,6 +516,7 @@ let fscBothToOut cfg out arg = Printf.ksprintf (Commands.fsc cfg.Directory (exec
506516
let fscBothToOutExpectFail cfg out arg = Printf.ksprintf (Commands.fsc cfg.Directory (execBothToOutExpectFail cfg cfg.Directory out) cfg.DotNetExe cfg.FSC) arg
507517
let fscAppendErrExpectFail cfg errPath arg = Printf.ksprintf (Commands.fsc cfg.Directory (execAppendErrExpectFail cfg errPath) cfg.DotNetExe cfg.FSC) arg
508518
let csc cfg arg = Printf.ksprintf (Commands.csc (exec cfg) cfg.CSC) arg
519+
let vbc cfg arg = Printf.ksprintf (Commands.vbc (exec cfg) cfg.VBC) arg
509520
let ildasm cfg arg = Printf.ksprintf (Commands.ildasm (exec cfg) cfg.ILDASM) arg
510521
let ilasm cfg arg = Printf.ksprintf (Commands.ilasm (exec cfg) cfg.ILASM) arg
511522
let peverify cfg = Commands.peverify (exec cfg) cfg.PEVERIFY "/nologo"
Lines changed: 224 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,224 @@
1+
// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information.
2+
3+
namespace FSharp.Compiler.UnitTests
4+
5+
open FSharp.Compiler.SourceCodeServices
6+
open FSharp.Reflection
7+
open FSharp.Test
8+
open FSharp.Test.Utilities
9+
open FSharp.Test.Utilities.Utilities
10+
open NUnit.Framework
11+
12+
[<TestFixture>]
13+
module ILMemberAccessTests =
14+
15+
let csharpBaseClass = """
16+
namespace ExhaustiveCombinations
17+
{
18+
public class CSharpBaseClass
19+
{
20+
public string GetPublicSetInternal { get; internal set; }
21+
public string GetPublicSetProtected { get; protected set; }
22+
public string GetPublicSetPrivateProtected { get; private protected set; }
23+
public string GetPublicSetProtectedInternal { get; protected internal set; }
24+
public string GetPublicSetPrivate { get; private set; }
25+
26+
public string SetPublicGetInternal { internal get; set; }
27+
public string SetPublicGetProtected { protected get; set; }
28+
public string SetPublicGetPrivateProtected { private protected get; set; }
29+
public string SetPublicGetProtectedInternal { protected internal get; set; }
30+
public string SetPublicGetPrivate { private get; set; }
31+
}
32+
}
33+
"""
34+
35+
let fsharpBaseClass = """
36+
namespace ExhaustiveCombinations
37+
38+
open System
39+
40+
type FSharpBaseClass () =
41+
42+
member this.GetPublicSetInternal with public get() = "" and internal set (parameter:string) = ignore parameter
43+
member this.GetPublicSetPrivate with public get() = "" and private set (parameter:string) = ignore parameter
44+
member this.SetPublicGetInternal with internal get() = "" and public set (parameter:string) = ignore parameter
45+
member this.SetPublicGetPrivate with private get() = "" and public set (parameter:string) = ignore parameter
46+
47+
"""
48+
49+
50+
[<Test>]
51+
let ``VerifyVisibility of Properties C# class F# derived class -- AccessPublicStuff`` () =
52+
53+
let fsharpSource =
54+
fsharpBaseClass + """
55+
open System
56+
open ExhaustiveCombinations
57+
58+
type MyFSharpClass () =
59+
inherit CSharpBaseClass()
60+
61+
member this.AccessPublicStuff() =
62+
63+
this.GetPublicSetInternal <- "1" // Inaccessible
64+
let _ = this.GetPublicSetInternal // Accessible
65+
66+
this.GetPublicSetPrivateProtected <- "1" // Accessible
67+
let _ = this.GetPublicSetPrivateProtected // Accessible
68+
69+
this.GetPublicSetProtectedInternal <- "1" // Accessible
70+
let _ = this.GetPublicSetProtectedInternal // Accessible
71+
72+
this.GetPublicSetProtected <- "1" // Accessible
73+
let _ = this.GetPublicSetProtected // Accessible
74+
75+
this.GetPublicSetPrivate <- "1" // Inaccessible
76+
let _ = this.GetPublicSetPrivate // Accessible
77+
()
78+
"""
79+
80+
let csCmpl =
81+
CompilationUtil.CreateCSharpCompilation(csharpBaseClass, CSharpLanguageVersion.CSharp8, TargetFramework.NetCoreApp30)
82+
|> CompilationReference.Create
83+
84+
let fsCmpl =
85+
Compilation.Create(fsharpSource, Fsx, Exe, options = [|"--langversion:preview"|], cmplRefs = [csCmpl])
86+
87+
CompilerAssert.CompileWithErrors(fsCmpl, [|
88+
(FSharpErrorSeverity.Error, 491, (22, 9, 22, 41),
89+
"The member or object constructor 'GetPublicSetInternal' is not accessible. Private members may only be accessed from within the declaring type. Protected members may only be accessed from an extending type and cannot be accessed from inner lambda expressions.");
90+
(FSharpErrorSeverity.Error, 491, (25, 9, 25, 49),
91+
"The member or object constructor 'GetPublicSetPrivateProtected' is not accessible. Private members may only be accessed from within the declaring type. Protected members may only be accessed from an extending type and cannot be accessed from inner lambda expressions.");
92+
(FSharpErrorSeverity.Error, 491, (34, 9, 34, 40),
93+
"The member or object constructor 'GetPublicSetPrivate' is not accessible. Private members may only be accessed from within the declaring type. Protected members may only be accessed from an extending type and cannot be accessed from inner lambda expressions.")|])
94+
95+
96+
[<Test>]
97+
let ``VerifyVisibility of Properties C# class F# non-derived class -- AccessPublicStuff`` () =
98+
99+
let fsharpSource =
100+
fsharpBaseClass + """
101+
open System
102+
open ExhaustiveCombinations
103+
104+
type MyFSharpClass () =
105+
106+
member _.AccessPublicStuff() =
107+
let bc = new CSharpBaseClass()
108+
109+
bc.GetPublicSetInternal <- "1" // Inaccessible
110+
let _ = bc.GetPublicSetInternal // Accessible
111+
112+
bc.GetPublicSetPrivateProtected <- "1" // Inaccessible
113+
let _ = bc.GetPublicSetPrivateProtected // Accessible
114+
115+
bc.GetPublicSetProtectedInternal <- "1" // Accessible
116+
let _ = bc.GetPublicSetProtectedInternal // Inaccessible
117+
118+
bc.GetPublicSetProtected <- "1" // Inaccessible
119+
let _ = bc.SetPublicGetProtected // Accessible
120+
121+
bc.GetPublicSetPrivate <- "1" // Inaccessible
122+
let _ = bc.GetPublicSetPrivate // Accessible
123+
()
124+
"""
125+
126+
let csCmpl =
127+
CompilationUtil.CreateCSharpCompilation(csharpBaseClass, CSharpLanguageVersion.CSharp8, TargetFramework.NetCoreApp30)
128+
|> CompilationReference.Create
129+
130+
let fsCmpl =
131+
Compilation.Create(fsharpSource, Fsx, Exe, options = [|"--langversion:preview"|], cmplRefs = [csCmpl])
132+
133+
CompilerAssert.CompileWithErrors(fsCmpl, [|
134+
(FSharpErrorSeverity.Error, 491, (22, 9, 22, 39),
135+
"The member or object constructor 'GetPublicSetInternal' is not accessible. Private members may only be accessed from within the declaring type. Protected members may only be accessed from an extending type and cannot be accessed from inner lambda expressions.");
136+
(FSharpErrorSeverity.Error, 491, (25, 9, 25, 47),
137+
"The member or object constructor 'GetPublicSetPrivateProtected' is not accessible. Private members may only be accessed from within the declaring type. Protected members may only be accessed from an extending type and cannot be accessed from inner lambda expressions.");
138+
(FSharpErrorSeverity.Error, 491, (28, 9, 28, 48),
139+
"The member or object constructor 'GetPublicSetProtectedInternal' is not accessible. Private members may only be accessed from within the declaring type. Protected members may only be accessed from an extending type and cannot be accessed from inner lambda expressions.");
140+
(FSharpErrorSeverity.Error, 491, (31, 9, 31, 40),
141+
"The member or object constructor 'GetPublicSetProtected' is not accessible. Private members may only be accessed from within the declaring type. Protected members may only be accessed from an extending type and cannot be accessed from inner lambda expressions.");
142+
(FSharpErrorSeverity.Error, 491, (32, 17, 32, 41),
143+
"The member or object constructor 'SetPublicGetProtected' is not accessible. Private members may only be accessed from within the declaring type. Protected members may only be accessed from an extending type and cannot be accessed from inner lambda expressions.");
144+
(FSharpErrorSeverity.Error, 491, (34, 9, 34, 38),
145+
"The member or object constructor 'GetPublicSetPrivate' is not accessible. Private members may only be accessed from within the declaring type. Protected members may only be accessed from an extending type and cannot be accessed from inner lambda expressions.")|])
146+
147+
148+
[<Test>]
149+
let ``VerifyVisibility of Properties F# base F# derived class -- AccessPublicStuff`` () =
150+
151+
let fsharpSource =
152+
fsharpBaseClass + """
153+
open System
154+
open ExhaustiveCombinations
155+
156+
type MyFSharpClass () =
157+
inherit FSharpBaseClass()
158+
159+
member this.AccessPublicStuff() =
160+
161+
this.GetPublicSetInternal <- "1" // Inaccessible
162+
let _ = this.GetPublicSetInternal // Accessible
163+
164+
this.GetPublicSetPrivate <- "1" // Inaccessible
165+
let _ = this.GetPublicSetPrivate // Accessible
166+
167+
this.SetPublicGetInternal <- "1" // Accessible
168+
let _ = this.SetPublicGetInternal // Inaccessible
169+
170+
this.SetPublicGetPrivate <- "1" // Accessible
171+
let _ = this.SetPublicGetPrivate // accessible
172+
173+
()
174+
"""
175+
176+
let csCmpl =
177+
CompilationUtil.CreateCSharpCompilation(csharpBaseClass, CSharpLanguageVersion.CSharp8, TargetFramework.NetCoreApp30)
178+
|> CompilationReference.Create
179+
180+
let fsCmpl =
181+
Compilation.Create(fsharpSource, Fsx, Exe, options = [|"--langversion:preview"|], cmplRefs = [csCmpl])
182+
183+
CompilerAssert.CompileWithErrors(fsCmpl, [|
184+
(FSharpErrorSeverity.Error, 810, (25, 9, 25, 33),
185+
"Property 'GetPublicSetPrivate' cannot be set");
186+
(FSharpErrorSeverity.Error, 807, (32, 17, 32, 41),
187+
"Property 'SetPublicGetPrivate' is not readable")
188+
|])
189+
190+
191+
[<Test>]
192+
let ``VerifyVisibility of Properties F# class F# non-derived class -- AccessPublicStuff`` () =
193+
194+
let fsharpSource =
195+
fsharpBaseClass + """
196+
open System
197+
open ExhaustiveCombinations
198+
199+
type MyFSharpClass () =
200+
201+
member _.AccessPublicStuff() =
202+
let bc = new FSharpBaseClass()
203+
204+
bc.GetPublicSetInternal <- "1" // Inaccessible
205+
let _ = bc.GetPublicSetInternal // Accessible
206+
207+
bc.GetPublicSetPrivate <- "1" // Inaccessible
208+
let _ = bc.GetPublicSetPrivate // Accessible
209+
()
210+
"""
211+
212+
let csCmpl =
213+
CompilationUtil.CreateCSharpCompilation(csharpBaseClass, CSharpLanguageVersion.CSharp8, TargetFramework.NetCoreApp30)
214+
|> CompilationReference.Create
215+
216+
let fsCmpl =
217+
Compilation.Create(fsharpSource, Fsx, Exe, options = [|"--langversion:preview"|], cmplRefs = [csCmpl])
218+
219+
CompilerAssert.CompileWithErrors(fsCmpl, [|
220+
(FSharpErrorSeverity.Error, 810, (25, 9, 25, 31),
221+
"Property 'GetPublicSetPrivate' cannot be set")|])
222+
223+
224+

tests/fsharp/FSharpSuite.Tests.fsproj

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
<Compile Include="Compiler\CodeGen\EmittedIL\CeEdiThrow.fs" />
3434
<Compile Include="Compiler\Conformance\DataExpressions\ComputationExpressions.fs" />
3535
<Compile Include="Compiler\Conformance\BasicGrammarElements\BasicConstants.fs" />
36+
<Compile Include="Compiler\Conformance\Properties\ILMemberAccessTests.fs" />
3637
<Compile Include="Compiler\ConstraintSolver\PrimitiveConstraints.fs" />
3738
<Compile Include="Compiler\ConstraintSolver\MemberConstraints.fs" />
3839
<Compile Include="Compiler\Warnings\AssignmentWarningTests.fs" />

0 commit comments

Comments
 (0)