Bring features/param-nullchecking up to date #46398
Conversation
| { | ||
| _factory.CurrentFunction = node.Symbol; | ||
| var visited = (BoundLambda)base.VisitLambda(node); | ||
| var visited = (BoundLambda)base.VisitLambda(node)!; |
There was a problem hiding this comment.
I would prefer a Debug.Assert(visited is object); instead of a suppression.
There was a problem hiding this comment.
Replaced the suppression with a Debug.Assert. An additional warning, but it should be fine. 👍
There was a problem hiding this comment.
Thought it was a warning, turns out it was an error:
LocalRewriter.cs(257,31): error CS8600: Converting null literal or possible null value to non-nullable type.
Reverted it back to what it was for now.
There was a problem hiding this comment.
It is a warning locally but we push all warnings to errors in the CI runs.
There was a problem hiding this comment.
The warning is because you need to cast it to BoundLambda?, not BoundLambda.
src/Compilers/CSharp/Portable/Lowering/StateMachineRewriter/StateMachineRewriter.cs
Outdated
Show resolved
Hide resolved
| IL_000d: throw | ||
| IL_000e: ldc.i4.s -2 | ||
| IL_0010: newobj ""C.<GetChars>d__0..ctor(int)"" | ||
| IL_0000: ldc.i4.s -2 |
There was a problem hiding this comment.
This reordering seems incorrect. I wouldn't necessarily try to fix it in this PR, just prototype it. But we should be doing the null checks first not after creating the enumerable closure object.
src/Compilers/CSharp/Test/IOperation/IOperation/IOperationTests_INullCheckedParameters.cs
Outdated
Show resolved
Hide resolved
src/Compilers/CSharp/Test/IOperation/IOperation/IOperationTests_INullCheckedParameters.cs
Show resolved
Hide resolved
src/Compilers/Core/Portable/Operations/ControlFlowGraphBuilder.cs
Outdated
Show resolved
Hide resolved
3 participants
In the process of updating
features/param-nullcheckingfor updated specs (!!).Changes: