-
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
Input param postconditions #49576
Input param postconditions #49576
Conversation
3474ceb
to
8f8f037
Compare
8f8f037
to
e552f25
Compare
@dotnet/roslyn-compiler Please review |
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 Please review |
@@ -5308,7 +5324,10 @@ private VisitArgumentResult VisitArgumentEvaluate(BoundExpression argument, RefK | |||
stateForLambda: result.StateForLambda); | |||
|
|||
// If the parameter has annotations, we perform an additional check for nullable value types |
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.
nullable value types [](start = 96, length = 20)
Is this just for nullable value types?
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.
Yes, for example in the case of a parameter [DisallowNull] string? s
we can adjust the type to just string
and then visit the conversion. But in the case of [DisallowNull] int? i
we can't adjust the type in that way. SharpLab. Note particularly the difference in diagnostic IDs.
The validation run passes CloudBuild so will merge this now. |
Closes #48605
Closes #48134
Closes #49136
Implements the change described in dotnet/csharplang#4102 (comment). Also fixes a bug where a null-conditional expression with a type
T where T : class?
had flow stateMaybeNull
when it should actually have flow stateMaybeDefault
.Note that while arguments to user-defined conversions have their state updated when the conversion causes a nullable warning, we don't actually honor the postcondition on the conversion parameter. Filed #49575 to follow-up-- I think parameter postconditions on a user-defined conversion are a pretty edge case, so probably won't prioritize it.