Repository navigation
[Version 9.0] Feature support for init accessors - #1452
BillWagner wants to merge 7 commits into
Conversation
| > <td rowspan="6">Property access</td> | ||
| > <td><code>P</code></td> | ||
| > <td>The get accessor of the property <code>P</code> in the containing class or struct is invoked. A compile-time error occurs if <code>P</code> is write-only. If <code>P</code> is not <code>static</code>, the instance expression is <code>this</code>.</td> | ||
| > <td>The get accessor of the property <code>P</code> in the containing class or struct is invoked. A compile-time error occurs if <code>P</code> is write-only or init-only. If <code>P</code> is not <code>static</code>, the instance expression is <code>this</code>.</td> |
There was a problem hiding this comment.
Should "containing class or struct" be extended to include interface? Here and a few other table rows below?
init accessorsinit accessors
06286d1 to
1dd0bbe
Compare
1dd0bbe to
0a30078
Compare
d16c29a to
87260cd
Compare
|
An earlier version of this feature is already present on |
|
Applied "meeting: discuss" so that we can all perform a first round of review before the next meeting. |
Co-authored-by: Copilot <[email protected]>
87260cd to
25b0a75
Compare
Nigel-Ecma
left a comment
There was a problem hiding this comment.
This is a partial review, stopped after reviewing classes.md.
I think changes are required.
|
|
||
| A set accessor and init accessor have the same signature; however, that for the init accessor also has an implementation-defined form of annotation to distinguish it from a set accessor. |
There was a problem hiding this comment.
I think this is description of a certain implementation, there appears to be no language level reason to require this. (Also there seems to be no CLR reason to do this; the CLR allows extra methods [accessors in C#] in a property. It is a particular C# implementation’s choice.)
It is also counter to the C# description of signature, which includes the name.
The language restriction is that there cannot be both a set and an init accessor.
Recommendation: remove
| A set accessor and init accessor have the same signature; however, that for the init accessor also has an implementation-defined form of annotation to distinguish it from a set accessor. |
There was a problem hiding this comment.
We could possibly add a note that init_P is not reserved. (But it doesn't need to be normative text.)
| ``` | ||
|
|
||
| Both signatures are reserved, even if the property is read-only or write-only. | ||
| Both signatures are reserved, even if the property has only one accessor. |
There was a problem hiding this comment.
We think this is okay - it's about the method signatures rather than the property accessor names being reserved.
| ``` | ||
|
|
||
| Both signatures are reserved, even if the indexer is read-only or write-only. | ||
| Both signatures are reserved, even if the indexer has only one accessor. |
There was a problem hiding this comment.
See comment for properties
| Both signatures are reserved, even if the indexer is read-only or write-only. | ||
| Both signatures are reserved, even if the indexer has only one accessor. | ||
|
|
||
| A set accessor and init accessor have the same signature; however, that for the init accessor also has an implementation-defined form of annotation to distinguish it from a set accessor. |
There was a problem hiding this comment.
See comment for properties
| The *accessor_declarations* consist of a *get_accessor_declaration*, a *set_accessor_declaration*, or both. Each accessor declaration consists of optional attributes, an optional *accessor_modifier*, the token `get` or `set`, followed by an *accessor_body*. | ||
| The *accessor_declarations* consist of either a *get_accessor_declaration*, optionally with a *set_accessor_declaration* or an *init_accessor_declaration*, or a *set_accessor_declaration* or *init_accessor_declaration* optionally with a *get_accessor_declaration*. Each accessor declaration consists of optional *attributes*, an optional *accessor_modifier*, the token `get`, `init`, or `set`, followed by an *accessor_body*. | ||
|
|
||
| For a ref-valued property the *ref_get_accessor_declaration* consists optional attributes, an optional *accessor_modifier*, the token `get`, followed by an *ref_accessor_body*. |
There was a problem hiding this comment.
Missing word? Also an -> a
| For a ref-valued property the *ref_get_accessor_declaration* consists optional attributes, an optional *accessor_modifier*, the token `get`, followed by an *ref_accessor_body*. | |
| For a ref-valued property the *ref_get_accessor_declaration* consists of optional attributes, an optional *accessor_modifier*, the token `get`, followed by a *ref_accessor_body*. |
| At the point an init accessor is invoked, the instance is known to be in the construction phase. Hence an init accessor may take the following actions in addition to what a set accessor can do: | ||
|
|
||
| 1. Call other init accessors available through `this` or `base` | ||
| 1. Assign `readonly` fields declared on the same type through `this` |
There was a problem hiding this comment.
What is this “instance”, it seems to pop up out of nowhere. Just sketching, hence not a suggestion block…
At the point a property’s init accessor is invoked, the property’s containing/owning type instance is known to be in the construction phase. Hence an init accessor may take the following actions in addition to what a set accessor can do:
- Set the value of read-init/init-only properties accessible through
thisorbase- Assign
readonlyfields declared on the same type throughthis
This also changes the wording “Call other init accessors” as code doesn’t directly call an accessor, unlike methods are directly called – the accessor is invoked as the consequence of some other action (assignment, ++, etc.). This might be a wider issue though!
There was a problem hiding this comment.
Line 3781 isn't too much earlier, although it refers to "object" instead of "instance":
An instance property containing an init_accessor_declaration is considered settable during the construction phase of the object
But I don't object to us making it clearer, of course.
| { | ||
| Name = "Jared" | ||
| }; | ||
| local.Name = "Jraed"; // Error |
There was a problem hiding this comment.
Intentionally 2 errors (assignment & spelling) or not? If intentional it is fine.
There was a problem hiding this comment.
My guess is that it's intentionally different but we might want to have a better example.
| - A set accessor corresponds to a method with a single value parameter of the property type, a void return type, and the same modifiers as the containing property. | ||
| - An init accessor corresponds to a method with a single value parameter of the property type, a `void` return type, and the same modifiers as the containing property. |
There was a problem hiding this comment.
Following the spirit of the suggestion on 3494, why not:
| - A set accessor corresponds to a method with a single value parameter of the property type, a void return type, and the same modifiers as the containing property. | |
| - An init accessor corresponds to a method with a single value parameter of the property type, a `void` return type, and the same modifiers as the containing property. | |
| - A set or init accessor corresponds to a method with a single value parameter of the property type, a `void` return type, and the same modifiers as the containing property. |
I tending to think that init & set accessors should be described as differing only in when they can be invoked, and not be listing out the (identical) other features.
| > | ||
| > *end example* | ||
|
|
||
| When an init accessor appears in a virtual property, all overrides for it shall also be marked `init`. Likewise, it is not possible to override a set accessor with an init accessor. |
There was a problem hiding this comment.
Is it stated elsewhere that all overrides for set & get also need to be “marked” set & get respectively?
I’m unsure whether the second sentence is saying something useful, or is maybe coming from a particular implementation translating an init into a set with flag. If it is the latter then an earlier suggestion has been made to remove this implementation specific detail, and if that is accepted then the text here might need to be reworded as well. (I.e. if the Standard never surfaces the implementation specific detail that an init is a set why would anyone think you can override one with the other?)
| ``` | ||
|
|
||
| Both signatures are reserved, even if the property is read-only or write-only. | ||
| Both signatures are reserved, even if the property has only one accessor. |
There was a problem hiding this comment.
We think this is okay - it's about the method signatures rather than the property accessor names being reserved.
25b0a75 to
fb19a35
Compare
| Both signatures are reserved, even if the property is read-only or write-only. | ||
| Both signatures are reserved, even if the property has only one accessor. | ||
|
|
||
| A set accessor and init accessor have the same signature; however, that for the init accessor also has an implementation-defined form of annotation to distinguish it from a set accessor. |
There was a problem hiding this comment.
Reading this change again, "however, that for the init accessor" reads very oddly to me.
| - A property that includes both a get accessor and an init accessor is said to be a ***read-init property***. It is a compile-time error for a read-init property to be the target of an assignment except during the construction phase of an object (§init-accessors). | ||
| - A property that has only a get accessor is said to be a ***read-only property***. It is a compile-time error for a read-only property to be the target of an assignment. | ||
| - A property that has only a set accessor is said to be a ***write-only property***. Except as the target of an assignment, it is a compile-time error to reference a write-only property in an expression. | ||
| - A property that has only an init accessor is said to be an ***init-only property***. Except as the target of an assignment during the construction phase of an object (§init-accessors), it is a compile-time error to reference an init-only property in an expression. |
There was a problem hiding this comment.
I hadn't spotted before that we have these definitions. I wonder whether we should keep them and refer to them, but introduce a new term of "it's only got a get access but it's wriable-ref-returning, so you can still use it as the target of an assignment". (This has come up in a few places recently.)
| > <!-- Example: {template:"code-in-main-without-using", name:"AccessorsInit2", additionalFiles:["PointStructWithInit.cs"]} --> | ||
| > | ||
| > ```csharp | ||
| > var p = new Point() { X = 42, Y = 13 }; |
There was a problem hiding this comment.
| > var p = new Point() { X = 42, Y = 13 }; | |
| > var p = new Point { X = 42, Y = 13 }; |
(Just to be more idiomatic.)
|
|
||
| #### §init-accessors Init accessors | ||
|
|
||
| An instance property containing an *init_accessor_declaration* is considered settable during the construction phase of the object, except when in a local function or lambda. The ***construction phase of an object*** includes the following: |
There was a problem hiding this comment.
I don't understand the "except when in a local function or lambda" part. When what is in a local function or lambda? Presumably the code that is invoking the init-accessor? An example might be beneficial.
There was a problem hiding this comment.
@jskeet – The restriction is (at least) that you cannot define a lambda in a constructor the body of which attempts to init the property. E.g.:
internal class Program
{
static void Main(string[] args) { new Program(); }
public Program()
{
P = 36; // OK
(Func<int, int>)(x => Q = x) = 42; // Err: Init-only Q can only be assigned in an initializer
}
private int p;
int P
{
get { return p; }
init { p = value; }
}
private int q;
int Q
{
get { return q; }
init { q = value; }
}
}Agree a (better than the above) sample would be helpful.
There was a problem hiding this comment.
Right. I think an example and a rewording would be helpful. Something about "can't be set via an implicit or explicit this reference captured by an anonymous function".
| > | ||
| > *end example* | ||
|
|
||
| The rules around when init accessors are settable extend across type hierarchies. If the member is accessible and the object is known to be in the construction phase, then the member is settable. |
There was a problem hiding this comment.
Does that include casting this to a derived type within a base type constructor?
internal class Base
{
public Base()
{
// This isn't valid...
if (this is Derived d)
{
d.X = 10;
}
// And neither is this...
(Derived) this.X = 10;
}
}
internal class Derived
{
public int X { get; init; }
public Derived()
{
X = 10;
}
}Maybe only refer to base types?
Add support for init accessors Add support for init accessors Add support for init accessors Add support for init accessors Add support for init accessors Add support for init accessors Add support for init accessors fix md formatting fix formatting, add xref links add xref links Co-authored-by: Copilot <[email protected]> Copilot-Session: 21570b74-c6f5-4fc5-ab6d-7546918d6ddb
Use different reasonable placeholders. Co-authored-by: Copilot <[email protected]> Copilot-Session: 21570b74-c6f5-4fc5-ab6d-7546918d6ddb
Co-authored-by: Copilot <[email protected]> Copilot-Session: 21570b74-c6f5-4fc5-ab6d-7546918d6ddb
I made these tweaks after comparing the edits I researched 2+ years ago and what I found in this new feature branch. Co-authored-by: Copilot <[email protected]> Copilot-Session: 21570b74-c6f5-4fc5-ab6d-7546918d6ddb
Co-authored-by: Copilot <[email protected]> Copilot-Session: 21570b74-c6f5-4fc5-ab6d-7546918d6ddb
Co-authored-by: Nigel-Ecma <[email protected]> Co-authored-by: Copilot <[email protected]> Copilot-Session: 21570b74-c6f5-4fc5-ab6d-7546918d6ddb
Co-authored-by: Copilot <[email protected]> Copilot-Session: 21570b74-c6f5-4fc5-ab6d-7546918d6ddb
01295fd to
f1e6670
Compare
This PR contains the work for
initaccessors in C# 9.The commits from #978 were squashed to one commit in this branch.
There are a number of comments that haven't been addressed on #978:
Notes:
initin it.