-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Init only proposal document #3367
Conversation
Finished up the section detailing using attributes vs. modreq. Decided to make it an open question for now vs. a consideration. I felt less strongly about it after writing it. I still do feel quite passionate though about this with validators.
Got the basic summary and motivation added. Feeling good about the premise here.
This ended up persuading me that `init` should be on the `set` method, not the property itself. There is just too much in common with the `readonly` modifier here. Plus if we ever decide to include the concept of `init` members as a general feature then `init` would be required to be on the `set`.
After discussion in LDM we've decide against this as a feature. Reasons captured in the document.
This updates to the following two LDM decisions: 1. Disallow `init` on fields 1. Use `init` instead of `init set`
proposals/init.md
Outdated
These mechanisms are effective at allowing the construction of immutable data | ||
but they do so by adding cost to the boiler plate code of types and opting | ||
such types out of features like object and collection initializers. This means | ||
developers must choose between easy of use and immutability. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider being more concrete about "Ease of use". You haven't mentioned adding a set
yet.
Co-Authored-By: Fred Silberberg <[email protected]>
Thanks for the feedback @333fred. Got through most of it but still have a bit left. Pushed updates for all the parts I got through. |
Finish up the PR feedback on the proposal
@333fred responded to all feedback now. One comment is still open for feedback from you but everything else I've cleaned up. Think I took pretty much all your suggestions. 😄 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome. Looking forward to this feature eventually landing in the future
proposals/init.md
Outdated
int Prop2 | ||
{ | ||
get => 42; | ||
set |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be init
right? Otherwise I don't get why those assignments would be valid.
**Resolution** | ||
This scenario is not seen as compelling by LDM. | ||
|
||
### Modreqs vs. attributes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have a link that explains what a modreq is? This is a new term for me personally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah it's a term very few understand. Probably only people who've had to deal with metadata encoding / decoding.
https://twitter.com/jaredpar/status/1247014490069020672
I don't have a good link to a document describing what a modreq or modopt is other than some obscure passages in the ECMA CLI document (here). Should probably write something up about it at some point.
Probably the easiest way to think about them is they're like attributes on method signatures but they're actually a part of the signature. That its an attribute that must be repeated in the same position whenever a method is overridden or when a method implements an interface members. They have the further requirement than if a compiler doesn't understand the type inside a modreq then it should effectively ignore the containing member altogether.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, thanks for the explanation! So basically custom markers for the compiler and the runtime to handle stuff in a special way. Works for me :D
One final question: What does the “mod” in “modreq” and “modopt” stand for? Req is likely requirement and opt is option, but mod? “Modification” as in to make a modification on the generated IL?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Mod" stands for modifier.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More specifically:
- modreq = modifier required: that is it is required that the compiler reading the entry understand the type in the modreq. That is because the type in the modreq implies special behavior that compilers are required to understand in order to handle the function correctly. Hence if the compiler does not understand it should discard that member.
- modopt = modifier optional: that is the compiler is not required to understand the type in the modopt. The type indicates behavior that can be beneficial to compilers who understand the behavior but knowledge is not required in order to correctly invoke the member.
Hi; can I request clarification for what features, if any, will be discoverable / usable via reflection? In particular, I'm thinking of meta-programming code that pre-dates generators (serializers, ORMs, etc), and which may do any of the following today:
In particular I want to guard against scenarios where a consumer moves from |
Hi, will init properties also be taken into consideration when initializing NRTs somehow? It would be great if the caller was forced to specify a value for a non-nullable reference type, otherwise I'm guessing you would be required to still initialize it from the constructor or make init properties always nullable? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some typos :)
I agree that is a key question (and we should aim for "this doesn't break things", since most folks will just think of this as "making it more accessible, therefore compatible"), but IMO that is the second question, since no
i.e. can I change existing code that may have been fully settable (but perhaps using popsicle immutability internally) to code that expresses the new intent via |
} | ||
``` | ||
|
||
Adding `init` to these properties is a non-breaking change: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like a perfect lightbulb/analyzer suggestion.
My apologies if I missed a previous discussion about this, but I feel slightly unconfortable about the proposed syntax. Have you considered a simpler alternative form where e.g. // Property is only settable at initialization and in the constructor
public int PropA { init; }
// Property is only settable in the constructor
public int PropB { get; }
// Property is settable everywhere
public int PropC { get; set; }
// Can be read publicly, but only written-to inside the constructor or "internally" during object initialization
public int PropD { internal init; } |
@GoldenCrystal Set-only properties are allowed, so I don’t see why init-only properties, without a getter, shouldn’t be. So no, I wouldn’t say that a getter is implicit when you specify |
Sure, set-only properties are allowed by the language spec, but:
If anything, I'd even argue that R/W auto-properties should be allowed to be declared as simply
I addressed the idea of a different accessibility for implicit |
Yes. The .NET API review notes for the Whether or not to provide the more friendly APIs is a bit of a debate point right now. The upside is that it makes it easier to write meta programming tools but the downside is it has a tendency to veer reflection from being language neutral to being more C# centric.
Yes there will be no change to how the backing field is encoded here.
Yes
At the moment .NET Core doesn't have an IL verification story hence there is nothing to worry about right now. I did add a section in the document about what IL verification would look like for this feature but until we have an actual implementation and updated spec it's a bit speculative.
If we every get an IL verification story with .NET Core then I think this will become a necessity for us. Until we have that though I suspect we won't emit the hoisted fields in any special way. It would make state machines bigger without providing any value. Or at least from the perspective of the C# language there is no benifit from doing this. Is there a meta programming consumption story that I'm missing here? |
Lots of typos 😄 Co-Authored-By: Tiago César Oliveira <[email protected]> Co-Authored-By: Patrick Westerhoff <[email protected]> Co-Authored-By: Steve Ognibene <[email protected]> Co-Authored-By: Viacheslav Ivanov <[email protected]>
@GoldenCrystal you seem to be approaching this in exactly the opposite way from what we did in LDM, which is interesting to me. You propose |
Co-Authored-By: Julien Couvreur <[email protected]> Co-Authored-By: Fred Silberberg <[email protected]>
@333fred Not sure if I need to detail it, but my reasoning was the following: The only thing we currently miss from get-only auto-properties is the ability to be written from object initializers in addition to the type constructors, which this proposal intends to adress (hopefully on the way to C# records ?) With that in mind, when I see Also, I do not know how to word it properly, but I found the
I think this is solved nicely if we consider that specifying the In the end, as far as auto-properties are concerned, we could see things in two different, yet not necessarily exclusive, ways:
So, hmm… That'd be a related, but totally different proposal, so I can open a separate issue to discuss this |
See, that's actually not true at all. |
Thanks for all the feedback! |
init | ||
{ | ||
Field1 = 13; // okay | ||
Field2 = 13; // okay | ||
Prop1 = 13; // okay | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you want to add that, and what would be the advantage? To me, this is duplicating the job of a constructor, and I think this will only create less readable code. Maybe there is a use-case that I missed; if so, feel free to enlighten me.
Edit: oops, just noticed that you merged the PR 15h ago (the tab was open since yesterday); well my comment applies nonetheless
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe the motivation section of this proposal adequately outlines our reasoning: we want to support nominal, immutable construction patterns which we do not do today.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe the motivation section of this proposal adequately outlines our reasoning: we want to support nominal, immutable construction patterns which we do not do today.
Yes, I get the intent @333fred , which is a really good idea; less boilerplate is always welcome. I still remember the day you added auto-implemented properties; what a day!
But what is the difference between the following two blocks of code?
init
{
Field1 = 13; // okay
Field2 = 13; // okay
Prop1 = 13; // okay
}
public SomeCtor()
{
Field1 = 13; // okay
Field2 = 13; // okay
Prop1 = 13; // okay
}
Then what is the added value of that new way to initialize fields/props? (for C# users)
An intend is one thing, but does that help solve a problem or does it only create noise and possible confusion? Think about it, people will have one more place to put initialization logic in, does someone need that? If yes, who does? What use-case does this solve that was unsolvable before or that required more boilerplate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Think about optional parameters in a constructor today: you have a location that can default them if not provided, or throw if a combination of provided parameters is invalid. This adds the same logic for properties that are inited by the caller, but then immutable afterwards.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Think about optional parameters in a constructor today: you have a location that can default them if not provided, or throw if a combination of provided parameters is invalid. This adds the same logic for properties that are inited by the caller, but then immutable afterwards.
When I think about that I see types that could require some kind of possible value/props/null/not null combo but without any way to know it in advance from the API of that class/struct. People can be creative 😉
Then what about I add another property that initializes init
and readonly
props/fields as well?
class Complex
{
readonly int Field1;
int Field2;
int Prop1 { get; init ; }
int Prop2
{
get => 42;
init
{
Field1 = 13; // okay
Field2 = 13; // okay
Prop1 = 13; // okay
}
}
int Prop3
{
get => 123;
init
{
Field1 = 3;
Field2 = 3;
Prop1 = 3;
}
}
}
In what order are those going to be initialized?
Wouldn't it be clearer to do the following?
class Complex
{
readonly int Field1;
int Field2;
int Prop1 { get; init ; }
int Prop2 { get; init; }
int Prop3 { get; init; }
public Complex()
{
Field1 = 13;
Field2 = 13;
Prop1 = 13;
Prop2 = 42;
Prop3 = 123;
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You shouldn't necessarily take syntax examples from a proposal that merely illustrate what things are legal where as a good API design. More than likely, most init
properties would initialize one, and only one, readonly field. You can still write your example today, using set
instead of init
, and it provides a markedly worse experience if you wanted it to actually be an immutable object.
As to ordering, that's the thing here: this is about nominal object creation. Nominal creation is unordered, that's the whole point. If you want to control the order in which properties are set, then you need to use the existing tools we have today in positional creation (namely, standard constructor arguments).
Imagine you had an example like this:
public class PathObject
{
string ActualPath { get; init; }
bool SupportsLongPath { get; init; }
init
{
if (ActualPath == null || !supportsLongPath && ActualPath.Length >= 256)
{
throw new ArgumentException(nameof(ActualPath), $"Long path support not enabled, but path is {ActualPath.Length}!");
}
}
}
Imagine if you had to write that without init
support? You literally can't do it. There's no way today to ensure that ActualPath
was initialized by the caller. Further, for validation of the length of ActualPath
, you can't do that either: you need to ensure that SupportsLongPath
is set before attempting to validate the path length. The only way to handle that correctly would be to use a constructor, which guarantees ordering. However, that brings burdens of its own. Let's say that this is a public class in a library that you publish publicly. You decide that you want to add support for hidden path object creation. But there's a problem: you've already published this class. If you want to avoid breaking your customers, you're going to have to maintain this 2-parameter constructor forever more, because removing it would break existing customer binaries. In the init
world, this pain point is gone. You add a new init-only property, validate as necessary in the init
body, and no one who compiled against a previous version of your library has to care about this new optional parameter that you've added. For a very real example of this type of pain, look at https://github.com/dotnet/roslyn/blob/master/src/Compilers/CSharp/Portable/CSharpCompilationOptions.cs#L787-L913. We cannot remove these constructors, as existing binaries depend on them, even the constructor marked as Bad - DO NOT USE
. Those constructors will never be able to go away, unfortunately, but at least we won't have to add more of them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, I could write my messy code in a set
, so init
is similar in that way (well, its an "initializable readonly
property setter" after all).
That said, I did not see the "init constructor-like" that you used in your last sample anywhere in the proposal, maybe you should add that as it seems that will be needed.
If the initialization is unordered, how can you be sure that your constructor-like-init code block will be hit after the init setter of the other properties?
More on that, to make the usage clearer for users, maybe afterinit
would be easier to understand for non-compiler/csharplang-guru? It would also point people in the direction of centralizing initialization logic instead of splitting it everywhere. Maybe a beforeinit
could benefit centralizing initialization while afterinit
benefits validation?
Or to make it less "magic" and more discoverable, what about an IInitializable
interface that adds a void AfterInit();
and a void BeforeInit()
methods usable during the initialization process? Same idea here, you need additional logic, you implement the interface (or create the afterinit
block) and code your initialization logic there; otherwise, you don't pollute your code with it.
As I understand it, you seem to want to help people get into C#, so making usage and understanding of new (and existing) features as easy as possible for everyone would probably help that. Just for fun, ask people in what order are executed auto-property initializers and constructors to see how unclear it can be. With init
you will be adding another layer of "confusion" over that.
As for breaking changes, a new major version should allow you to clean stuff up and introduce breaking changes. Everyone makes mistakes but having to carry all of the mistakes of everyone forever ain't helping anyone 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That said, I did not see the "init constructor-like" that you used in your last sample anywhere in the proposal, maybe you should add that as it seems that will be needed.
It'll be coming in a separate proposal. It'll all part of a greater feature unity we're aiming for as part of records.
If the initialization is unordered, how can you be sure that your constructor-like-init code block will be hit after the init setter of the other properties?
The compiler will be required to generate a call to the validator (straw-man name we're using for now) after an object initializer is run, so ordering won't matter.
Don't take the syntax I used as "How it will look exactly". A future proposal with more validator design will be coming, and we haven't talked much about the specific syntax beyond "We like the general idea".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to know, thanks, I'll try to follow that up then.
The decision was made to move forward with `init` as a standalone accessor in | ||
the property accessor list. | ||
|
||
### Warn on failed init |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any plans to add some option or another keyword that would force property to be set in initializer or constructor?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We've touched on that topic in recent discussions, but haven't made any plans for that yet.
Will you be able to set an init only property from a nested object initializer? Allowing it would let you to set an initonly property of any arbitrary object: public class Foo
{
public int Bar { get; init; }
}
public struct InitSetter<T>
{
public InitSetter(T t) => Value = t;
public T Value { get; }
}
...
public void M(Foo foo)
{
new InitSetter<Foo>(foo)
{
Value =
{
Bar = 42,
}
}
} |
At that point the object is already created. So it's not really in the initialization context. I'd expect not. |
No description provided.