Fix #3891: don't reduce nesting when there is no else block - #3898
Merged
Conversation
ReduceNesting walks an else-if chain to its innermost if and asks ShouldReduceNesting whether to extract the else block, which ExtractElseBlock does by casting the block to Block. A chain with no trailing else reaches this with a bare Nop, yet the heuristic still approved it (its stats count a Nop as one statement), so the cast threw InvalidCastException. Take a Block in ShouldReduceNesting and skip the reduction at the call site when the else is absent. Assisted-by: Claude:claude-opus-4-8:Claude Code
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #3891 —
System.InvalidCastException: Unable to cast object of type 'ICSharpCode.Decompiler.IL.Nop' to type 'ICSharpCode.Decompiler.IL.Block'thrown fromReduceNestingTransform.ExtractElseBlockwhile decompiling a method.Root cause
ReduceNestingwalks an else-if chain to its innermostifand, if the nesting heuristic approves, extracts the else block viaExtractElseBlock, which does(Block)ifInst.FalseInst. When the chain has no trailing else, thatFalseInstis a bareNop, so the cast throws.The real defect is upstream of the cast:
ShouldReduceNestingwas handed thatNopand returnedtrue—UpdateStats(Nop)counts it as one statement, and withmaxStatements == 0themaxStatements2 >= 2 * maxStatementsterm becomes1 >= 0. So the heuristic gave a nonsensical "yes, reduce" for something that isn't a reducible block, and the cast is just where that detonated.Fix
ShouldReduceNestingnow takes aBlockinstead of anILInstruction, so the precondition is enforced by the type rather than discovered at runtime, and the "is there an else block" check moves to the call site inReduceNesting(a chain with no trailing else simply isn't reduced). The switch caller already passed a realBlock, so it is unchanged.Verification
FalseInstis aBlock(every genuine else-if reduction and the switch/default path), so working output is byte-identical. Confirmed by decompilingSystem.Private.CoreLibandSystem.Linq.Expressionswith and without the change — identical error counts.Note on tests: the trigger requires an else-if chain with no final else whose branches score zero statements, which no C# compiler emits from ordinary source (it collapses empty branches, turns unreachable ones into sibling
ifs, etc.) — the reported assembly is IL-woven. A compiled ILPretty fixture therefore isn't reachable without the original IL, so no test is included here.🤖 Generated with Claude Code