-
Notifications
You must be signed in to change notification settings - Fork 4.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
Preserve changes that are still relevant after revert of #66311 #66440
Conversation
@jcouv, @dotnet/roslyn-compiler For the second review |
_factory.Sequence(new BoundExpression[] { _factory.AssignmentExpression(cache, intermediateRef) }, cache), | ||
receiverType, | ||
isRef: true), | ||
#pragma warning disable format |
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.
nit: What's the formatting issue? Should we file a tracking issue? #Closed
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.
What's the formatting issue? Should we file a tracking issue?
IDE wants the initializer on a separate line. I think it is by design, but I do not like the design. I have no intention to fight for its change though.
private IsConditionalConstrainedCallThatMustUseTempForReferenceTypeReceiverWalker(BoundLoweredConditionalAccess conditionalAccess) | ||
: base() | ||
{ | ||
_conditionalAccess = conditionalAccess; |
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.
nit: it looks like the only part we need to hold onto is the Id
#WontFix
private bool? _result; | ||
|
||
private IsConditionalConstrainedCallThatMustUseTempForReferenceTypeReceiverWalker(BoundLoweredConditionalAccess conditionalAccess) | ||
: base() |
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.
nit: is the base initializer needed? #Closed
@@ -564,6 +569,60 @@ private void EmitLoweredConditionalAccessExpression(BoundLoweredConditionalAcces | |||
} | |||
} | |||
|
|||
private sealed class IsConditionalConstrainedCallThatMustUseTempForReferenceTypeReceiverWalker : BoundTreeWalkerWithStackGuardWithoutRecursionOnTheLeftOfBinaryOperator |
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.
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.
LGTM Thanks (iteration 1) with some nits to consider
Related to #63221.