Skip to content

Emit correct IL for exhaustive switch statements that use a separate Dag for lowering - #85873

Open
AlekseyTs wants to merge 2 commits into
dotnet:mainfrom
AlekseyTs:Issue85809
Open

AlekseyTs wants to merge 2 commits into
dotnet:mainfrom
AlekseyTs:Issue85809

Conversation

@AlekseyTs

@AlekseyTs AlekseyTs commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #85809

Microsoft Reviewers: Open in CodeFlow

Copilot AI balanced review requested due to automatic review settings October 1, 2026 15:41
@AlekseyTs
AlekseyTs requested a review from a team as a code owner October 1, 2026 15:41
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).
1 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

It changes compiler control-flow and IL generation, and the full CI suite is still running.

Review effort: Balanced
Findings: 2 Low severity

Open (2)
What changed in this PR

Fixes invalid IL from exhaustive switch statements when lowering requires a separate decision DAG.

Changes:

  • Emits an unreachable-path exception instead of falling through.
  • Reuses switch-expression exception selection logic.
  • Adds extensive union and list-pattern IL tests.

Minor comment/documentation corrections were identified.

File Description
CSharpTestBase.cs Updates DAG helper call.
PatternMatchingTests_ListPatterns.cs Adds list-pattern regression coverage.
UnionsTests.cs Adds union switch coverage.
SynthesizedThrowSwitchExpressionExceptionMethod.cs Extracts throw generation.
SynthesizedParameterlessThrowMethod.cs Extracts parameterless throw generation.
LocalRewriter_SwitchExpression.cs Shares exception-selection logic.
LocalRewriter_PatternSwitchStatement.cs Emits the fallback throw path.
LocalRewriter_BasePatternSwitchLocalRewriter.cs Centralizes exception strategy selection.
BoundSwitchStatement.cs Tracks lowering-only default labels.

// We get here when there is an explicit 'default:' label
// By definition, the switch is exhaustive, but it is possible, that according to
// the reachability Dag, the label is not reachable through the Dag (it might still
// be reachable through anexplicit goto).
Comment thread src/Compilers/CSharp/Test/CSharp15/UnionsTests.cs Outdated
Copilot AI balanced review requested due to automatic review settings October 1, 2026 16:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

It changes compiler control flow and emitted IL, and the latest CI run remains in progress.

Review effort: Balanced
Findings: 1 Low severity

Open (1)
Resolved since last review (1)
Previously missed (1)

In code that hasn't changed since last review

Low severity Comment incorrectly says unmatched input uses BreakLabel

src/​Compilers/​CSharp/​Portable/​BoundTree/​BoundSwitchStatement.cs:32

This branch assigns the declared default label, not BreakLabel; the comment currently misstates the destination for unmatched input.

@AlekseyTs

Copy link
Copy Markdown
Contributor Author

@RikkiGibson, @jjonescz, @dotnet/roslyn-compiler Please review

@AlekseyTs

Copy link
Copy Markdown
Contributor Author

@RikkiGibson, @jjonescz For a second review

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unions: exhaustive switch statement ending a method emits invalid IL in Release (InvalidProgramException)

3 participants