Await should check null-state of expression #31981
Conversation
jcouv
added
Area-Compilers
Feature - Nullable Reference Types
| { | ||
| async void M(System.Threading.Tasks.Task<string>? task) | ||
| { | ||
| await task; // warn |
There was a problem hiding this comment.
Yes, I'll add.
I expect this produces a safety warning for awaiting something that could be null, but that we don't update the state of x to non-null (if the await didn't throw). So there will be cascading diagnostics, which we could suppress.
jcouv
added
the
PR For Personal Review Only
|
Worth adding tests for nullable of ValueTask as well? |
jcouv
force-pushed
the
await-nullable
branch
from
40f660a to
d2429a8
Compare
jcouv
force-pushed
the
await-nullable
branch
from
d2429a8 to
d6b4363
Compare
jcouv
removed
the
PR For Personal Review Only
| int slot = MakeSlot(receiverOpt); | ||
| if (slot > 0) | ||
|
|
||
| if (receiverOpt != null) |
There was a problem hiding this comment.
No need for this check, we're already in an if with this condition.
Consider adding tests for the other users of Refers to: src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs:4887 in 5537bea. [](commit_id = 5537bea, deletion_comment = False) |
|
Done review pass (commit 2) |
| @@ -4491,6 +4491,7 @@ private TypeSymbolWithAnnotations InferResultNullabilityOfBinaryLogicalOperator( | |||
| public override BoundNode VisitAwaitExpression(BoundAwaitExpression node) | |||
| { | |||
| var result = base.VisitAwaitExpression(node); | |||
| CheckPossibleNullReceiver(node.Expression); | |||
There was a problem hiding this comment.
What about the case where GetAwaiter is an extension method which takes a nullable receiver? What should the code do in that case and is it handled / tested here?
There was a problem hiding this comment.
I'll add a test, but I expect the behavior is the same as for invocations. Namely, we still warn. That was an explicit decision in a previous PR, but we could adjust later.
Fixes #31519
I'm going to wait for #31978 to make some improvements to this PR.