From 3320bb96016c09f85f2e92acef1977ffc3ff9b20 Mon Sep 17 00:00:00 2001 From: jnm2 Date: Sat, 12 Apr 2025 18:18:41 -0400 Subject: [PATCH 1/4] Fix the spec section --- proposals/readonly-setter-calls-on-non-variables.md | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/proposals/readonly-setter-calls-on-non-variables.md b/proposals/readonly-setter-calls-on-non-variables.md index 0c2699fd01..92a6ee476a 100644 --- a/proposals/readonly-setter-calls-on-non-variables.md +++ b/proposals/readonly-setter-calls-on-non-variables.md @@ -120,18 +120,11 @@ It's desirable for this error to remain, because the setter _does_ mutate the st ## Specification -### Waiting for v8 and corrected v7 specification +The current v8 specification draft does not yet specify readonly members (). The following updates intend to leverage the concept of a _readonly member_. A _readonly member_ is a member which either is directly marked with the `readonly` modifier or which is contained inside a _struct_type_ which is marked with the `readonly` modifier. -The published v7 spec and the draft specs are all missing the wording that removed the CS1612 error for invocation expressions. This is tracked by which suggests the following addition to describe the current behavior: +[§12.21.2](https://github.com/dotnet/csharpstandard/blob/standard-v7/standard/expressions.md#12212-simple-assignment) _Simple assignment_ is updated: -[§12.21.2](https://github.com/dotnet/csharpstandard/blob/standard-v7/standard/expressions.md#12212-simple-assignment) _Simple assignment_ -> When a property or indexer declared in a _struct_type_ is the target of an assignment, **unless the _struct_type_ has the `readonly` modifier ([§16.2.2](https://github.com/dotnet/csharpstandard/blob/standard-v7/standard/structs.md#1622-struct-modifiers)) and the instance expression is an invocation**, the instance expression associated with the property or indexer access shall be classified as a variable. If the instance expression is classified as a value, a binding-time error occurs. - -Then, the v8 spec is still in draft and the updates for readonly members have not yet merged. When it merges, it will need to redefine the condition as whether the setter itself is readonly, directly or indirectly. - -### Spec updates - -This proposal would remove the text "**and the instance expression is an invocation**" from the paragraph mentioned above in [Waiting for v8 and corrected v7 specification](#waiting-for-v8-and-corrected-v7-specification). +> When a property or indexer declared in a _struct_type_ is the target of an assignment, **either** the instance expression associated with the property or indexer access shall be classified as a variable, **or the set accessor of the property or indexer shall be a readonly member ([§16.2.2](https://github.com/dotnet/csharpstandard/blob/standard-v7/standard/structs.md#1622-struct-modifiers))**. If the instance expression is classified as a value **and the set accessor is not a readonly member**, a binding-time error occurs. ## Expansions From ac56a9f71419b97dd486765f197787c6b715d5ec Mon Sep 17 00:00:00 2001 From: jnm2 Date: Sat, 12 Apr 2025 18:32:50 -0400 Subject: [PATCH 2/4] Turn expansion into an answered question --- .../readonly-setter-calls-on-non-variables.md | 76 +++++++++++-------- 1 file changed, 44 insertions(+), 32 deletions(-) diff --git a/proposals/readonly-setter-calls-on-non-variables.md b/proposals/readonly-setter-calls-on-non-variables.md index 92a6ee476a..5ed90c733a 100644 --- a/proposals/readonly-setter-calls-on-non-variables.md +++ b/proposals/readonly-setter-calls-on-non-variables.md @@ -126,27 +126,48 @@ The current v8 specification draft does not yet specify readonly members ( When a property or indexer declared in a _struct_type_ is the target of an assignment, **either** the instance expression associated with the property or indexer access shall be classified as a variable, **or the set accessor of the property or indexer shall be a readonly member ([§16.2.2](https://github.com/dotnet/csharpstandard/blob/standard-v7/standard/structs.md#1622-struct-modifiers))**. If the instance expression is classified as a value **and the set accessor is not a readonly member**, a binding-time error occurs. -## Expansions +## Downsides + +If this proposal is taken, it becomes a source-breaking change to remove the `readonly` keyword from a struct or setter. Without the `readonly` keyword, the errors would then be relevant and would reappear. -There's another location where this kind of assignment is blocked, which is in object initializers: +Due to what looks like an unintentional change in the compiler, this source-breaking change is already in effect when the setter is called on an invocation expression: ```cs -// ❌ CS1918 Members of property 'C.ArraySegmentProp' of type 'ArraySegment' cannot be assigned with an object -// initializer because it is of a value type -_ = new C { ArraySegmentProp = { [42] = new object() } }; -// ~~~~~~~~~~~~~~~~ +// Removing 'readonly' from S1 causes a CS1612 error. +M().Prop = 1; -class C +S1 M() => default; + +public readonly struct S1 { - public ArraySegment ArraySegmentProp { get; set; } + public int Prop { get => 0; set { } } +} +``` + +```cs +// Removing 'readonly' from S2.Prop.set causes a CS1612 error. +M().Prop = 1; + +S2 M() => default; + +public struct S2 +{ + public int Prop { get => 0; readonly set { } } } ``` -For the same reasons as above, this error is unnecessary when the properties being initialized have readonly `set`/`init` accessors. The error could be made more granular, placed on each property initializer which calls a _non-readonly_ setter. +## Answered LDM questions + +### Should similar assignments be permitted in object initializers? -Even in the limited cases where a ref-returning indexer is applicable, it does not help with CS1918, where it still unnecessarily broad: +There's a separate error, CS1918 that blocks assignments through readonly setters when the assignments appear in object initializers. In addition, this error even blocks assignments to ref-returning properties and indexers, and those assignments are not blocked when they appear outside of object initializers. ```cs +// ❌ CS1918 Members of property 'C.ArraySegmentProp' of type 'ArraySegment' cannot be assigned with an object +// initializer because it is of a value type +_ = new C { ArraySegmentProp = { [42] = new object() } }; +// ~~~~~~~~~~~~~~~~ + // ❌ CS1918 Members of property 'C.StructWithRefReturningIndexer' of type 'Span' cannot be assigned with an object // initializer because it is of a value type _ = new C { StructWithRefReturningIndexer = { [42] = new object() } }; @@ -159,32 +180,23 @@ class C } ``` -## Downsides - -If this proposal is taken, it becomes a source-breaking change to remove the `readonly` keyword from a struct or setter. Without the `readonly` keyword, the errors would then be relevant and would reappear. - -Due to what looks like an unintentional change in the compiler, this source-breaking change is already in effect when the setter is called on an invocation expression: +Such assignments desugar to the following form, the same form in which the CS1612 warning is being removed: ```cs -// Removing 'readonly' from S1 causes a CS1612 error. -M().Prop = 1; - -S1 M() => default; - -public readonly struct S1 -{ - public int Prop { get => 0; set { } } -} +var temp = new C(); +// Warning being removed: +// CS1612 Cannot modify the return value of 'C.ArraySegmentProp' because it is not a variable +temp.ArraySegmentProp[42] = new object(); ``` ```cs -// Removing 'readonly' from S2.Prop.set causes a CS1612 error. -M().Prop = 1; +var temp = new C(); +// Permitted today +temp.StructWithRefReturningIndexer[42] = new object(); +``` -S2 M() => default; +Should this check be made more granular, so that members of struct types may be assigned when they would be allowed to be assigned in the desugared form? -public struct S2 -{ - public int Prop { get => 0; readonly set { } } -} -``` +#### Answer + +Yes. This expansion will be included. [(LDM 2025-04-02)](https://github.com/dotnet/csharplang/blob/main/meetings/2025/LDM-2025-04-02.md#expansions) From f13b7b07cab457c8df459873be09efca05670db8 Mon Sep 17 00:00:00 2001 From: jnm2 Date: Sat, 12 Apr 2025 18:45:28 -0400 Subject: [PATCH 3/4] Fix object initializers link in meeting notes --- meetings/2025/LDM-2025-04-02.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meetings/2025/LDM-2025-04-02.md b/meetings/2025/LDM-2025-04-02.md index 06f1a94270..475c03b64e 100644 --- a/meetings/2025/LDM-2025-04-02.md +++ b/meetings/2025/LDM-2025-04-02.md @@ -71,7 +71,7 @@ Question: https://github.com/dotnet/csharplang/blob/e34ecf622058b53f0fb37705baa8 There are cases where it is appropriate to error if a non-readonly setter is called, that may be reasonable when a `readonly` setter is called. We explored two of those cases that currently error with _CS1918 Member of property ... cannot be assigned with an object initializer because it is of a value type._ Should we include allowing readonly setters in these cases as part of the broader feature. -A change would be needed to the C# spec on object initializers ([§11.18.2](https://github.com/dotnet/csharpstandard/blob/standard-v6/standard/expressions.md#11182-simple-assignment)), which will be added to this proposal. +A change would be needed to the C# spec on object initializers ([§12.8.16.3](https://github.com/dotnet/csharpstandard/blob/standard-v7/standard/expressions.md#128163-object-initializers)), which will be added to this proposal. It was noted that expansions could be split off and done after the rest of the work in this proposal. From b2f545298a15ad5efce111de6d6ee1438760f1b8 Mon Sep 17 00:00:00 2001 From: jnm2 Date: Sat, 12 Apr 2025 18:59:43 -0400 Subject: [PATCH 4/4] Work the expansion into the proposal --- .../readonly-setter-calls-on-non-variables.md | 26 +++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/proposals/readonly-setter-calls-on-non-variables.md b/proposals/readonly-setter-calls-on-non-variables.md index 5ed90c733a..4c382f9945 100644 --- a/proposals/readonly-setter-calls-on-non-variables.md +++ b/proposals/readonly-setter-calls-on-non-variables.md @@ -4,7 +4,7 @@ Champion issue: ## Summary -Permits a readonly setter to be called on _all_ non-variable expressions: +Permits a readonly setter to be called on non-variable expressions, and permits object initializers to be used with value types: ```cs var c = new C(); @@ -17,6 +17,14 @@ c.ArraySegmentMethod()[10] = new object(); // In limited cases, ref-returning indexers can be used to work around this: c.RefReturningIndexerWorkaround[10] = new object(); +_ = new C +{ + // Remove the current CS1918 error: + ArraySegmentProp = { [10] = new object() }, + // Remove the current CS1918 error: + RefReturningIndexerWorkaround = { [10] = new object() }, +}; + class C { public ArraySegment ArraySegmentProp { get; set; } @@ -75,7 +83,9 @@ These use cases would no longer be blocked if CS1612 is fully updated with an un ## Detailed design -The CS1612 error is not produced for assignments where the setter is readonly. (If the whole struct is readonly, then the setter is also readonly.) The setter call is emitted the same way as any non-accessor readonly instance method call. +For part 1, the CS1612 error is not produced for assignments where the setter is readonly. (If the whole struct is readonly, then the setter is also readonly.) The setter call is emitted the same way as any non-accessor readonly instance method call. + +For part 2, object initializers are now permitted to be used on value types. This means that the CS1918 error will no longer be produced at all. This opens the door to assignments using readonly setters or using refs returned by getters. However, this does not open the door to assignments that would not be permitted in the desugared form. Errors such as CS1612 and CS0313 will be updated to appear within object initializers now that CS1918 is no longer blocking off the entire space. ### Null-conditional assignment @@ -120,12 +130,24 @@ It's desirable for this error to remain, because the setter _does_ mutate the st ## Specification +Insertions are in **bold**, deletions are in ~~strikethrough~~. + +### Updates permitting readonly setter calls on non-variables + The current v8 specification draft does not yet specify readonly members (). The following updates intend to leverage the concept of a _readonly member_. A _readonly member_ is a member which either is directly marked with the `readonly` modifier or which is contained inside a _struct_type_ which is marked with the `readonly` modifier. [§12.21.2](https://github.com/dotnet/csharpstandard/blob/standard-v7/standard/expressions.md#12212-simple-assignment) _Simple assignment_ is updated: > When a property or indexer declared in a _struct_type_ is the target of an assignment, **either** the instance expression associated with the property or indexer access shall be classified as a variable, **or the set accessor of the property or indexer shall be a readonly member ([§16.2.2](https://github.com/dotnet/csharpstandard/blob/standard-v7/standard/structs.md#1622-struct-modifiers))**. If the instance expression is classified as a value **and the set accessor is not a readonly member**, a binding-time error occurs. +### Updates permitting object initializers for value types + +The CS1918 error is completely removed. There is no need to block value types. "\[T]he assignments in the nested object initializer are treated as assignments to members of the field or property." Such assignments must already conform to rules such as the one enforced by CS1612, including when inside an object initializer. + +[§12.8.16.3](https://github.com/dotnet/csharpstandard/blob/standard-v7/standard/expressions.md#128163-object-initializers) _Object initializers_ is updated: + +> A member initializer that specifies an object initializer after the equals sign is a ***nested object initializer***, i.e., an initialization of an embedded object. Instead of assigning a new value to the field or property, the assignments in the nested object initializer are treated as assignments to members of the field or property. ~~Nested object initializers cannot be applied to properties with a value type, or to read-only fields with a value type.~~ + ## Downsides If this proposal is taken, it becomes a source-breaking change to remove the `readonly` keyword from a struct or setter. Without the `readonly` keyword, the errors would then be relevant and would reappear.