-
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
Allow suppressing foreach iteration variable nullability mismatch #75432
Conversation
@@ -8703,7 +8704,7 @@ private TypeWithState VisitConversion( | |||
bool canConvertNestedNullability = true; | |||
bool isSuppressed = false; | |||
|
|||
if (conversionOperand.IsSuppressed == true) | |||
if (suppressed || conversionOperand.IsSuppressed) |
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.
Could you share some details on the set up of the situation? I gather that we have two stacked conversions that are not part of the same conversion group and the operand is suppressed, but I don't understand why.
If this setup is expected, should we just dig through conversions here when looking for the IsSuppressed
flag? #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.
We are getting here from VisitForEachIterationVariables
where we call VisitConversion
on node.IterationVariableType
- there is no suppression in that node subtree. The suppression is in the node.Expression
subtree.
Note that the pre-existing ref case similarly checks for suppression via node.Expression is not BoundConversion { Operand.IsSuppressed: true }
.
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)
@dotnet/roslyn-compiler for second review. Thanks |
@@ -8703,7 +8704,7 @@ private TypeWithState VisitConversion( | |||
bool canConvertNestedNullability = true; | |||
bool isSuppressed = false; | |||
|
|||
if (conversionOperand.IsSuppressed == true) | |||
if (suppressed || conversionOperand.IsSuppressed) |
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.
Would it be possible to unify the suppressed
parameter with the isSuppressed
local?
@RikkiGibson Please take another look. Thanks |
@RikkiGibson Please take another look. Thanks |
Fixes #75430.