Skip to content

Fix #3827: keep the while-loop for a ref local used after the loop (avoid an uninitialized hoisted ref decl)#3830

Open
sailro wants to merge 3 commits into
icsharpcode:masterfrom
sailro:fix-reflocal-foriterator
Open

Fix #3827: keep the while-loop for a ref local used after the loop (avoid an uninitialized hoisted ref decl)#3830
sailro wants to merge 3 commits into
icsharpcode:masterfrom
sailro:fix-reflocal-foriterator

Conversation

@sailro

@sailro sailro commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Fixes #3827.

A ref local used after a while-loop has its declaration hoisted in front of the
loop. PatternStatementTransform then rewrote the while into a for, moving the
sole ref-assignment into the for-initializer — leaving ref T x; (no initializer)
in front of a headless for (; cond; x = ref …), which does not compile
(CS8174: a by-reference variable must have an initializer).

Fix

Per @siegfriedpammer's review, instead of emitting a headless for-loop,
PatternStatementTransform.TransformFor now leaves the loop as a while when the
iteration variable is a by-ref-like local used after the loop. The ref-assignment
stays on the hoisted declaration and the output matches the original source — both
simpler and free of the not-so-nice headless-for form.

Before (does not recompile — CS8174):

ref Node reference;
for (reference = ref _buckets[hash & 3]; reference != null; reference = ref reference.next)
{
    ...
}

After:

ref Node reference = ref _buckets[hash & 3];
while (reference != null)
{
    ...
    reference = ref reference.next;
}

Test

Pretty/RefLocalsAndReturns.RefLocalUsedAfterForLoop round-trips the hoisted-decl +
while-loop form.

Real-world occurrence: Enums.NET (net461) hash-bucket walk. Same symptom as the
long-closed #1630 (stale-bot closed, no fix); the if/else ref-assignment variant
from #1520 is not addressed here.

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.

Pull request overview

This PR fixes a C# decompilation/codegen edge case where a ref local whose declaration is hoisted ahead of a for loop could lose its initializer (ending up as ref T x;), producing invalid C# (CS8174). The change teaches DeclareVariables to detect when the only initialization is the first for-initializer assignment and to move that initializer into the hoisted declaration while removing the for initializer.

Changes:

  • Add detection for for (v = …; …) as the sole initialization when the declaration must be hoisted before the for statement.
  • Emit ref T v = ref …; before the loop and remove the corresponding for initializer to preserve valid C# for ref locals.
  • Add a pretty test case covering the “ref local used after loop” scenario (Issue #3827) to ensure round-tripping.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
ICSharpCode.Decompiler/CSharp/Transforms/DeclareVariables.cs Detects and rewrites the “hoisted ref local + for-initializer assignment” pattern to keep required initializers on ref local declarations.
ICSharpCode.Decompiler.Tests/TestCases/Pretty/RefLocalsAndReturns.cs Adds a regression test case that exercises the fixed pattern and validates the emitted form.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

… of a for-loop

A ref local that is used after a for-loop has its declaration hoisted in front of
the loop, but its only initialization is the for-initializer ref-assignment. The
declaration was then emitted without an initializer (`ref T x;`), which does not
compile (CS8174).

When a by-ref-like local's matching assignment is the first for-initializer, move
the ref-assignment's value up into the declaration (`ref T x = ref expr;`) and
drop the for-initializer.

Assisted-by: Copilot:claude-opus-4.8:GitHub Copilot CLI
@sailro sailro force-pushed the fix-reflocal-foriterator branch from 59e4c4a to e95e77d Compare June 26, 2026 13:11
Comment thread ICSharpCode.Decompiler.Tests/TestCases/Pretty/RefLocalsAndReturns.cs Outdated
Per @siegfriedpammer: rather than rescue the hoisted for-initializer in
DeclareVariables, don't form the for-loop at all. TransformFor now bails when the
loop variable is a by-ref-like local used after the loop, leaving the while-loop
(matching source); the ref decl keeps its initializer, no CS8174. Reverts the
DeclareVariables change; test now expects the while form.

Assisted-by: Copilot:claude-opus-4.8:GitHub Copilot CLI
@sailro sailro requested a review from siegfriedpammer June 30, 2026 08:44
@sailro sailro changed the title Fix #3827: keep the initializer on a ref local hoisted out of a for-loop Fix #3827: keep the while-loop for a ref local used after the loop (avoid an uninitialized hoisted ref decl) Jul 3, 2026
The loop-shape decision can use ILVariable use lists before AST lowering, so avoid creating a for-loop when its iterator updates a byref local that must remain usable after the loop.

Assisted-by: OpenAI:openai/gpt-5.5:OpenCode
@siegfriedpammer siegfriedpammer force-pushed the fix-reflocal-foriterator branch from ad39122 to 819b0ed Compare July 5, 2026 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants