fsharp
fsharp copied to clipboard
Allow access modifiers to auto properties getters and setters
Description
Implements this suggestion
Checklist
-
[x] RFC added
-
[x] Put it under preview flag
-
[ ] Test cases added
-
[ ] Performance benchmarks added in case of performance changes
-
[x] Release notes entry updated:
Please make sure to add an entry with short succinct description of the change as well as link to this pull request to the respective release notes file, if applicable.
Release notes files:
- If anything under
src/Compiler
has been changed, please make sure to make an entry indocs/release-notes/.FSharp.Compiler.Service/<version>.md
, where<version>
is usually "highest" one, e.g.42.8.200
- If language feature was added (i.e.
LanguageFeatures.fsi
was changed), please add it todocs/releae-notes/.Language/preview.md
- If a change to
FSharp.Core
was made, please make sure to editdocs/release-notes/.FSharp.Core/<version>.md
where version is "highest" one, e.g.8.0.200
.
Information about the release notes entries format can be found in the documentation. Example:
- More inlines for Result module. (PR #16106)
- Correctly handle assembly imports with public key token of 0 length. (Issue #16359, PR #16363)
*
while!
(Language suggestion #1038, PR #14238)
If you believe that release notes are not necessary for this PR, please add
NO_RELEASE_NOTES
label to the pull request. - If anything under
:heavy_exclamation_mark: Release notes required
:white_check_mark: Found changes and release notes in following paths:
Change path Release notes path Description src/Compiler
docs/release-notes/.FSharp.Compiler.Service/8.0.300.md LanguageFeatures.fsi
docs/release-notes/.Language/preview.md
Where should I add tests to?
Where should I add tests to?
They should go to ComponentTests probably.
Where should I add tests to?
I think this would be a good place https://github.com/dotnet/fsharp/tree/main/tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/MemberDefinitions
Should this also be allowed for abstract properties? Such as:
type IA =
abstract B: int with get, internal set
Should this also be allowed for abstract properties? Such as:
type IA = abstract B: int with get, internal set
It seems that abstract members doesn't allow access modifiers
By the way, access modifiers before getter and setter will be ignored and produce a warning in signature files
will anyone make reviews on this pr?
will anyone make reviews on this pr?
Yes, we will shortly, we're a bit short-handed now, a bunch of folks are off.
By the way, access modifiers before getter and setter will be ignored and produce a warning in signature files
Would this mean that a private val member with private getter and public setter
would be usable from the outside ?
type A() =
member val private X = 0 with public get, private set
let a = A()
a.X // ?
If that is the case we should add a compiler error saying that there are conflicting access modifiers.
type A() = member val private X = 0 with public get, private set
type A() =
// Error: When the visibility for a property is specified, setting the visibility of the set or get method is not allowed.
member val private X = 0 with public get, private set
// Ok
member val X = 0 with public get, private set
// Ok
member val private X = 0 with get, set
member val X = 0 with public get, private set
in signature files should be written as following, and can be usable from outside of A
type A =
new: unit -> A
member public D: int with get
member private D: int with set
I found that in ILSpy, member val private A = 0 with get, set
and member val A = 0 with internal get, private set
both produce a internal property. Is it correct behavior?
internal int A
{
[CompilerGenerated]
[DebuggerNonUserCode]
get
{
return A@;
}
[CompilerGenerated]
[DebuggerNonUserCode]
set
{
A@ = value;
}
}
I found that in ILSpy,
member val private A = 0 with get, set
andmember val A = 0 with internal get, private set
both produce a internal property. Is it correct behavior?internal int A { [CompilerGenerated] [DebuggerNonUserCode] get { return A@; } [CompilerGenerated] [DebuggerNonUserCode] set { A@ = value; } }
Yes, current compiler always emits everything (private and internal) as internal.
@Tangent-90 ugh, it seems you're facing some issues with tests post merge, sorry about that. Let me know if I can help resolving those.
@Tangent-90 ugh, it seems you're facing some issues with tests post merge, sorry about that. Let me know if I can help resolving those.
Just some boring error messages position mismatched in tests...
@vzarytovskii Still has a test failed.
Failed FSharpChecker.TransparentCompiler.File is not checked twice [291 ms]
Error Message:
Assert.Equal() Failure
Expected: FSharpList<JobEvent> [Weakened, Requested, Started, Finished]
Actual: FSharpList<JobEvent> [Weakened, Requested, Started]
Stack Trace:
at FSharpChecker.TransparentCompiler.File is not checked twice() in /home/vsts/work/1/s/tests/FSharp.Compiler.ComponentTests/FSharpChecker/TransparentCompiler.fs:line 242
at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
/azp run
Azure Pipelines successfully started running 2 pipeline(s).
/azp run
Azure Pipelines successfully started running 2 pipeline(s).
/azp run
[!CAUTION] Repository is on lockdown for maintenance, all merges are on hold.
By the way, access modifiers before getter and setter will be ignored and produce a warning in signature files
Why? Would it be possible to support signatures too?
member val X = 0 with public get, private set in signature files should be written as following, and can be usable from outside of A
Can we fix this and allow the same syntax, please?
@vzarytovskii I think we should not release the feature without a proper signature support.
By the way, access modifiers before getter and setter will be ignored and produce a warning in signature files
Why? Would it be possible to support signatures too?
That might so much thing to change... It also relates to the generation of signature files.