-
Notifications
You must be signed in to change notification settings - Fork 1.8k
C#: Replace initializer splitting with an ObjectInitMethod. #20922
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
base: main
Are you sure you want to change the base?
Conversation
49eed3a to
0ef7bee
Compare
michaelnebel
left a comment
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.
Uh, very neat!
Just some quick initial comments. It appears that there are some tests failing.
| { | ||
| internal sealed class ObjectInitMethod : CachedEntity, IMethodEntity | ||
| { | ||
| Type ContainingType { get; } |
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.
private?
| this.ContainingType = containingType; | ||
| } | ||
|
|
||
| public string Name => "<object initializer>"; |
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.
Can be made a readonly field.
|
Do you also intend to model some synthetic assignments in the new synthetic method? |
The assignments are already extracted as top-level expressions in the enclosing class. We could move them into this new method - that's what Java does, but Java also allow one to write a chunk of code that goes directly into the object initializer, so the situation there is slightly different. However, I opted not to do that since it would be a much more invasive change and require us to change any current QL code that relies on identifying field initializers as they would have moved and would look different. Though, if we were building from scratch then I would have put the assignments inside the method, but as it stands I don't think it's worth it to make the change. |
bed13c3 to
f7ff3f5
Compare
f7ff3f5 to
3ee3160
Compare
No description provided.