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.
In the process of updating
features/param-nullcheckingfor updated specs (!!).Changes: