Allow init accessors in readonly contexts #47652
Conversation
This allows `init` properties on readonly structs, readonly properties in structs, and allows `init` accessors to be declared readonly themselves. Fixes dotnet#47612.
|
|
||
| public struct S | ||
| { | ||
| public int I { get; readonly init; } |
There was a problem hiding this comment.
I think it is strange to allow this, and I also think it is strange to consider an 'init' accessor to be readonly--including when the containing type is 'readonly'. It's effectively saying: "The 'this' parameter to the accessor is a readonly ref, and the accessor is allowed to write to it". It can't cause any differences in behavior.
Also, due to the implementation of MethodSymbol.IsEffectivelyReadOnly, the public API will say that IMethodSymbol.IsReadOnly is false for the readonly init; accessor here. We should add a test for this accessor and a test for when the containing type is (you beat me to the punch 😉)readonly.
The more I look at this, the stranger I find it--but if you are sure that you do not want to introduce a restriction here, I won't belabor the point.
There was a problem hiding this comment.
I just think there's no real point to introduce a restriction here. This PR makes it so you can put an init in a readonly struct, and this is legal because the initter can only be called during the construction phase, when instance readonly is allowed to be violated. Doing this makes the language more regular. It might be worth clarifying with LDM as to whether we should allow this, but I just don't think we get any real benefits to restricting it.
There was a problem hiding this comment.
I agree it's a bit strange at first glance but I agree that it makes the language more regular to allow it. The mental model of readonly struct is that it's a struct where every single member has the readonly modifier. Hence if we allow init in a readonly struct then it should follow that we allow readonly init in a standard struct.
There was a problem hiding this comment.
In a readonly struct, most of the instance members are readonly. The constructors are not readonly. The static members are not readonly. "All the members have 'readonly'" is a reasonable mental model but it is a simplification of the reality of the language.
src/Compilers/CSharp/Test/Semantic/Semantics/InitOnlyMemberTests.cs
Outdated
Show resolved
Hide resolved
|
Will wait to merge until we get LDM clarification on the |
…prevent them from being considered readonly by the language.
* upstream/master: (114 commits) Remove langversion restriction for source generators. (dotnet#47714) Adjust disambiguation rules for type pattern in a switch expression. (dotnet#47756) Delete decommissioned benchview tool scripts (dotnet#47752) Emit conversions between native integers and pointers directly (dotnet#47708) Typeless expressions should contribute nullability to lambda return (dotnet#47581) Use a distinct diagnostic ID when an exhaustiveness report uses an unnamed enum value. (dotnet#47693) [master] Update dependencies from dotnet/arcade (dotnet#46586) Change `Location` of record's primary constructor to point to record's identifier. (dotnet#47715) Add public API test for extended partial methods (dotnet#47727) Rename in CheckValidNullableMethodOverride (dotnet#47718) Update docs Add more doc comments Add comments and doc comments for ExternalErrorDiagnosticUpdateSource Add documentation remarks for syntax kinds (dotnet#43646) Disable TestCancellation (dotnet#47725) Classify function pointer punctuation (dotnet#47668) Disable flaky optprof test Handle NotNullIfNotNull in delegate creation and overrides (dotnet#47572) Adjust QuickInfo on record BaseType syntax (dotnet#47656) Don't exclude events for NameOf context ...
ghost
added this to the 
This allows
initproperties on readonly structs, readonly properties in structs, and allowsinitaccessors to be declared readonly themselves. Fixes #47612.