ISwitchExpressionOperation.IsExhaustive API extension#52703
ISwitchExpressionOperation.IsExhaustive API extension#52703333fred merged 33 commits intodotnet:mainfrom
Conversation
|
@333fred or @CyrusNajmabadi please review |
|
Something to add or change? |
src/Compilers/CSharp/Portable/Operations/CSharpOperationFactory.cs
Outdated
Show resolved
Hide resolved
src/Compilers/CSharp/Test/IOperation/IOperation/IOperationTests_ISwitchExpression.cs
Show resolved
Hide resolved
Co-authored-by: Fred Silberberg <fred@silberberg.xyz>
It looks like we can safely change this type to In reply to: 827918882 In reply to: 827918882 Refers to: src/Compilers/CSharp/Portable/Operations/CSharpOperationFactory.cs:2153 in 955cd47. [](commit_id = 955cd47, deletion_comment = False) |
src/Compilers/CSharp/Portable/Operations/CSharpOperationFactory.cs
Outdated
Show resolved
Hide resolved
|
It looks like ControlFlowGraphBuilder always adds SwitchExpressionException branch for a switch expression. Should we adjust that to be conditional based on the new property? In reply to: 827926846 |
…/bernd5/roslyn into ioperation_switchexpr_exhaustive
|
@AlekseyTs : I tried to follow your comment - but I'm not quite sure if it is correct |
src/Compilers/Core/Portable/Operations/ControlFlowGraphBuilder.cs
Outdated
Show resolved
Hide resolved
src/Compilers/Core/Portable/Operations/ControlFlowGraphBuilder.cs
Outdated
Show resolved
Hide resolved
src/Compilers/Core/Portable/Operations/ControlFlowGraphBuilder.cs
Outdated
Show resolved
Hide resolved
src/Compilers/Core/Portable/Operations/ControlFlowGraphBuilder.cs
Outdated
Show resolved
Hide resolved
src/Compilers/Core/Portable/Operations/ControlFlowGraphBuilder.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: AlekseyTs <AlekseyTs@users.noreply.github.com>
Co-authored-by: AlekseyTs <AlekseyTs@users.noreply.github.com>
Co-authored-by: AlekseyTs <AlekseyTs@users.noreply.github.com>
|
@AlekseyTs : thanks for your commits. |
The best way to deal with test that are failing due to baseline difference is to update the baselines. Then we can review the changes and confirm that the baseline changes are expected. |
Co-authored-by: AlekseyTs <AlekseyTs@users.noreply.github.com>
|
Alright, I've dug through the test failures here. The problem is that we are introducing a branch for the following (example) tree: Note how B6 is using flow capture 1, which is not initialized by B4. It feels like we need to skip branches for pure discard branches in the flow tree. |
What unit-test is that? |
src/Compilers/Core/Portable/Operations/ControlFlowGraphBuilder.cs
Outdated
Show resolved
Hide resolved
src/Compilers/CSharp/Test/IOperation/IOperation/IOperationTests_ISwitchExpression.cs
Outdated
Show resolved
Hide resolved
src/Compilers/Core/Portable/Operations/ControlFlowGraphBuilder.cs
Outdated
Show resolved
Hide resolved
src/Compilers/Core/Portable/Operations/ControlFlowGraphBuilder.cs
Outdated
Show resolved
Hide resolved
src/Compilers/Core/Portable/Operations/ControlFlowGraphBuilder.cs
Outdated
Show resolved
Hide resolved
…, not just the top level.
|
Alright, looking at the test failures there is quite a bit more that would need to be done to eliminate false jumps here. For example, |
|
Thanks for the patience and persistence @bernd5! |
fixes #52577