From e95e77d13159cc42147c28959baac191b704928b Mon Sep 17 00:00:00 2001 From: Sebastien Lebreton Date: Fri, 26 Jun 2026 13:32:14 +0200 Subject: [PATCH 1/3] Fix #3827: keep the initializer on a ref local hoisted out 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 --- .../TestCases/Pretty/RefLocalsAndReturns.cs | 28 +++++++++ .../CSharp/Transforms/DeclareVariables.cs | 59 +++++++++++++++++++ 2 files changed, 87 insertions(+) diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/RefLocalsAndReturns.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/RefLocalsAndReturns.cs index 3a4f266d78..ccf2ddb5b3 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/RefLocalsAndReturns.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/RefLocalsAndReturns.cs @@ -45,6 +45,13 @@ public static void Test() } } + public class Issue3827Node + { + public Issue3827Node next; + + public int value; + } + public delegate ref T RefFunc(); public delegate ref readonly T ReadOnlyRefFunc(); public delegate ref TReturn RefFunc(T1 param1); @@ -159,6 +166,27 @@ public void RefReadonlyCallVirt(RefLocalsAndReturns provider) private static int DefaultInt = 0; + public static Issue3827Node RefLocalUsedAfterForLoop(Issue3827Node[] buckets, int hash, Issue3827Node newNode) + { + // The ref local is used after the loop, so its declaration is hoisted in front of the + // for-statement; the for-initializer ref-assignment must stay on the declaration, because + // a ref local cannot be declared without an initializer. + ref Issue3827Node reference = ref buckets[hash & 3]; + for (; reference != null; reference = ref reference.next) + { + if (reference.value == hash) + { + reference = newNode; + break; + } + } + if (reference == null) + { + reference = newNode; + } + return reference; + } + public static ref T GetRef() { throw new NotImplementedException(); diff --git a/ICSharpCode.Decompiler/CSharp/Transforms/DeclareVariables.cs b/ICSharpCode.Decompiler/CSharp/Transforms/DeclareVariables.cs index d9ae39453e..7f5044a5a4 100644 --- a/ICSharpCode.Decompiler/CSharp/Transforms/DeclareVariables.cs +++ b/ICSharpCode.Decompiler/CSharp/Transforms/DeclareVariables.cs @@ -582,6 +582,39 @@ bool IsMatchingAssignment(VariableToDeclare v, [NotNullWhen(true)] out Assignmen && identExpr.TypeArguments.Count == 0; } + /// + /// Matches the case where the variable's only initialization is the first initializer of a + /// for-statement (`for (v = expr; ...)`) and the declaration has to be placed in front of the + /// for-statement (e.g. because the variable is used after the loop). Used to keep a ref local's + /// initializer on its declaration (a ref local may not be declared without one). + /// + bool IsMatchingForInitializerAssignment(VariableToDeclare v, [NotNullWhen(true)] out ForStatement? forStatement, + [NotNullWhen(true)] out Statement? initializerStatement, [NotNullWhen(true)] out AssignmentExpression? assignment) + { + forStatement = null; + initializerStatement = null; + assignment = null; + + if (v.InsertionPoint.nextNode is not ForStatement { Parent: BlockStatement } fs) + return false; + + if (fs.Initializers.FirstOrDefault() is not ExpressionStatement { Expression: AssignmentExpression a } firstInit) + return false; + + if (a.Operator != AssignmentOperatorType.Assign + || a.Left is not IdentifierExpression identExpr + || identExpr.Identifier != v.Name + || identExpr.TypeArguments.Count != 0) + { + return false; + } + + forStatement = fs; + initializerStatement = firstInit; + assignment = a; + return true; + } + bool CombineDeclarationAndInitializer(VariableToDeclare v, TransformContext context) { if (v.Type.IsByRefLike) @@ -633,6 +666,32 @@ void InsertVariableDeclarations(TransformContext context) } replacements.Add((v.InsertionPoint.nextNode, vds)); } + else if (v.Type.IsByRefLike && IsMatchingForInitializerAssignment(v, out var forStatement, out var forInitializerStatement, out var forInitAssignment)) + { + // A by-ref-like local that is used after the loop has its declaration hoisted in + // front of the for-statement, but its only initialization is the for-initializer + // ref-assignment. A ref local must be declared with an initializer (CS8174), so move + // the ref-assignment's value up into the declaration and drop the for-initializer. + AstType type = context.TypeSystemAstBuilder.ConvertType(v.Type); + if (v.ILVariable.IsRefReadOnly && type is ComposedType { HasRefSpecifier: true } composedType) + { + composedType.HasReadOnlySpecifier = true; + } + + var vds = new VariableDeclarationStatement(type, v.Name, forInitAssignment.Right.Detach()); + var init = vds.Variables.Single(); + init.AddAnnotation(forInitAssignment.Left.GetResolveResult()); + foreach (object annotation in forInitAssignment.Left.Annotations.Concat(forInitAssignment.Annotations)) + { + if (annotation is not ResolveResult) + { + init.AddAnnotation(annotation); + } + } + + forInitializerStatement.Remove(); + forStatement.Parent!.InsertChildBefore(forStatement, vds, Slots.Statement); + } else if (CanBeDeclaredAsOutVariable(v, out var dirExpr)) { // 'T v; SomeCall(out v);' can be combined to 'SomeCall(out T v);' From 59ba602b4aebb80a38769903fde225e3413208cf Mon Sep 17 00:00:00 2001 From: Sebastien Lebreton Date: Mon, 29 Jun 2026 09:45:25 +0200 Subject: [PATCH 2/3] Address review: keep while-loop instead of headless for for ref locals 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 --- .../TestCases/Pretty/RefLocalsAndReturns.cs | 9 +-- .../CSharp/Transforms/DeclareVariables.cs | 59 ------------------- .../Transforms/PatternStatementTransform.cs | 16 +++++ 3 files changed, 21 insertions(+), 63 deletions(-) diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/RefLocalsAndReturns.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/RefLocalsAndReturns.cs index ccf2ddb5b3..9c14435cbc 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/RefLocalsAndReturns.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/RefLocalsAndReturns.cs @@ -168,17 +168,18 @@ public void RefReadonlyCallVirt(RefLocalsAndReturns provider) public static Issue3827Node RefLocalUsedAfterForLoop(Issue3827Node[] buckets, int hash, Issue3827Node newNode) { - // The ref local is used after the loop, so its declaration is hoisted in front of the - // for-statement; the for-initializer ref-assignment must stay on the declaration, because - // a ref local cannot be declared without an initializer. + // The ref local is used after the loop, so its declaration is hoisted in front of the loop. + // The loop is kept as a while-loop (rather than converted to a headless for-loop) so the + // ref-assignment stays on the declaration -- a ref local cannot be declared without an initializer. ref Issue3827Node reference = ref buckets[hash & 3]; - for (; reference != null; reference = ref reference.next) + while (reference != null) { if (reference.value == hash) { reference = newNode; break; } + reference = ref reference.next; } if (reference == null) { diff --git a/ICSharpCode.Decompiler/CSharp/Transforms/DeclareVariables.cs b/ICSharpCode.Decompiler/CSharp/Transforms/DeclareVariables.cs index 7f5044a5a4..d9ae39453e 100644 --- a/ICSharpCode.Decompiler/CSharp/Transforms/DeclareVariables.cs +++ b/ICSharpCode.Decompiler/CSharp/Transforms/DeclareVariables.cs @@ -582,39 +582,6 @@ bool IsMatchingAssignment(VariableToDeclare v, [NotNullWhen(true)] out Assignmen && identExpr.TypeArguments.Count == 0; } - /// - /// Matches the case where the variable's only initialization is the first initializer of a - /// for-statement (`for (v = expr; ...)`) and the declaration has to be placed in front of the - /// for-statement (e.g. because the variable is used after the loop). Used to keep a ref local's - /// initializer on its declaration (a ref local may not be declared without one). - /// - bool IsMatchingForInitializerAssignment(VariableToDeclare v, [NotNullWhen(true)] out ForStatement? forStatement, - [NotNullWhen(true)] out Statement? initializerStatement, [NotNullWhen(true)] out AssignmentExpression? assignment) - { - forStatement = null; - initializerStatement = null; - assignment = null; - - if (v.InsertionPoint.nextNode is not ForStatement { Parent: BlockStatement } fs) - return false; - - if (fs.Initializers.FirstOrDefault() is not ExpressionStatement { Expression: AssignmentExpression a } firstInit) - return false; - - if (a.Operator != AssignmentOperatorType.Assign - || a.Left is not IdentifierExpression identExpr - || identExpr.Identifier != v.Name - || identExpr.TypeArguments.Count != 0) - { - return false; - } - - forStatement = fs; - initializerStatement = firstInit; - assignment = a; - return true; - } - bool CombineDeclarationAndInitializer(VariableToDeclare v, TransformContext context) { if (v.Type.IsByRefLike) @@ -666,32 +633,6 @@ void InsertVariableDeclarations(TransformContext context) } replacements.Add((v.InsertionPoint.nextNode, vds)); } - else if (v.Type.IsByRefLike && IsMatchingForInitializerAssignment(v, out var forStatement, out var forInitializerStatement, out var forInitAssignment)) - { - // A by-ref-like local that is used after the loop has its declaration hoisted in - // front of the for-statement, but its only initialization is the for-initializer - // ref-assignment. A ref local must be declared with an initializer (CS8174), so move - // the ref-assignment's value up into the declaration and drop the for-initializer. - AstType type = context.TypeSystemAstBuilder.ConvertType(v.Type); - if (v.ILVariable.IsRefReadOnly && type is ComposedType { HasRefSpecifier: true } composedType) - { - composedType.HasReadOnlySpecifier = true; - } - - var vds = new VariableDeclarationStatement(type, v.Name, forInitAssignment.Right.Detach()); - var init = vds.Variables.Single(); - init.AddAnnotation(forInitAssignment.Left.GetResolveResult()); - foreach (object annotation in forInitAssignment.Left.Annotations.Concat(forInitAssignment.Annotations)) - { - if (annotation is not ResolveResult) - { - init.AddAnnotation(annotation); - } - } - - forInitializerStatement.Remove(); - forStatement.Parent!.InsertChildBefore(forStatement, vds, Slots.Statement); - } else if (CanBeDeclaredAsOutVariable(v, out var dirExpr)) { // 'T v; SomeCall(out v);' can be combined to 'SomeCall(out T v);' diff --git a/ICSharpCode.Decompiler/CSharp/Transforms/PatternStatementTransform.cs b/ICSharpCode.Decompiler/CSharp/Transforms/PatternStatementTransform.cs index 2aa550794c..15e25c13d8 100644 --- a/ICSharpCode.Decompiler/CSharp/Transforms/PatternStatementTransform.cs +++ b/ICSharpCode.Decompiler/CSharp/Transforms/PatternStatementTransform.cs @@ -202,6 +202,12 @@ public override AstNode VisitTryCatchStatement(TryCatchStatement tryCatchStateme if (variable != m3.Get("ident").Single().GetILVariable()) return null; WhileStatement loop = (WhileStatement)next; + // Cannot convert to for loop, if the iteration variable is a ref local used after the loop: its + // declaration is hoisted in front, leaving a headless `for (; cond; v = ref ...)` whose only + // initialization is the for-initializer ref-assignment -- which can't be split from a ref local + // (CS8174). Keeping it a while-loop matches the source and keeps the initializer on the decl. + if (variable != null && variable.Type.IsByRefLike && IsVariableUsedAfter(loop, variable)) + return null; // Cannot convert to for loop, if any variable that is used in the "iterator" part of the pattern, // will be declared in the body of the while-loop. var iteratorStatement = m3.Get("iterator").Single(); @@ -244,6 +250,16 @@ bool ForStatementUsesVariable(ForStatement statement, IL.ILVariable? variable) return false; } + bool IsVariableUsedAfter(Statement loop, IL.ILVariable variable) + { + for (AstNode? sibling = loop.NextSibling; sibling != null; sibling = sibling.NextSibling) + { + if (sibling.DescendantsAndSelf.OfType().Any(ie => ie.GetILVariable() == variable)) + return true; + } + return false; + } + bool IteratorVariablesDeclaredInsideLoopBody(Statement iteratorStatement) { foreach (var id in iteratorStatement.DescendantsAndSelf.OfType()) From 819b0ed7c05abbd151b432d8cda667464f8a22a9 Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Sun, 5 Jul 2026 08:51:46 +0200 Subject: [PATCH 3/3] Keep ref iterator loops as while when live after loop 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 --- .../IL/Transforms/HighLevelLoopTransform.cs | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/ICSharpCode.Decompiler/IL/Transforms/HighLevelLoopTransform.cs b/ICSharpCode.Decompiler/IL/Transforms/HighLevelLoopTransform.cs index 78e4138266..83000fe100 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/HighLevelLoopTransform.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/HighLevelLoopTransform.cs @@ -395,6 +395,8 @@ bool MatchForLoop(BlockContainer loop, IfInstruction whileCondition, Block while // - increment block if (incrementBlock.Instructions.Count <= 1 || loop.Blocks.Count < 3) return false; + if (StoresRefLocalUsedAfterLoop(loop, incrementBlock)) + return false; context.Step("Transform to for loop: " + loop.EntryPoint.Label, loop); // move the block to the end of the loop: loop.Blocks.MoveElementToEnd(incrementBlock); @@ -467,6 +469,27 @@ bool MatchForLoop(BlockContainer loop, IfInstruction whileCondition, Block while return true; } + static bool StoresRefLocalUsedAfterLoop(BlockContainer loop, Block incrementBlock) + { + foreach (var store in incrementBlock.Instructions.SkipLast(1).SelectMany(inst => inst.Descendants).OfType()) + { + var variable = store.Variable; + if (variable.Type.IsByRefLike && IsVariableUsedAfterLoop(loop, variable)) + return true; + } + return false; + } + + static bool IsVariableUsedAfterLoop(BlockContainer loop, ILVariable variable) + { + foreach (var use in variable.LoadInstructions.Concat(variable.AddressInstructions).Concat(variable.StoreInstructions.Cast())) + { + if (loop.GetCommonParent(use) is Block { Kind: BlockKind.ControlFlow } && loop.IsBefore(use)) + return true; + } + return false; + } + bool IsAssignment(ILInstruction inst) { if (inst is StLoc)