From 46d21ed9a51740e7b5141f456b62281c018affb3 Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Thu, 10 Sep 2026 18:16:16 +0200 Subject: [PATCH 1/2] Accept a deconstructed backing-field store only without a setter A constructor store to an auto-property's backing field is expressible after the field declaration is gone in one of two ways: ReplaceBackingFieldUsage rewrites it to an assignment of a setter-less property, or TransformFieldAndConstructorInitializers lifts it into a property initializer. A deconstruction target assigns several members at once, so it can never take the second route, and a property that kept a setter would invoke that setter instead of storing the field. Without the restriction the declaration is removed while the store keeps referencing it. Assisted-by: Claude:claude-opus-5:Claude Code --- .../Transforms/PatternStatementTransform.cs | 33 ++++++++++++++++--- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/ICSharpCode.Decompiler/CSharp/Transforms/PatternStatementTransform.cs b/ICSharpCode.Decompiler/CSharp/Transforms/PatternStatementTransform.cs index 1584d0111..b5a40d081 100644 --- a/ICSharpCode.Decompiler/CSharp/Transforms/PatternStatementTransform.cs +++ b/ICSharpCode.Decompiler/CSharp/Transforms/PatternStatementTransform.cs @@ -1019,6 +1019,12 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms return td.HasFlag(System.Reflection.TypeAttributes.BeforeFieldInit); } + /// + /// Maps each backing field to whether its references outside the owning property's + /// accessors can still be expressed once the field declaration is gone: true for + /// rescuable constructor stores only, false for anything else. A field absent from + /// the map has no outside references at all and is therefore also expressible. + /// Dictionary BuildOutsideReferenceIndex(AstNode root) { var verdicts = new Dictionary(); @@ -1190,10 +1196,12 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms } /// - /// True when is the left-hand side of a plain assignment to + /// True when is a target of a plain assignment to /// inside a constructor of the field's declaring type - the /// only outside reference the "field" keyword can still express (as a property - /// initializer, or an assignment to a setter-less property). + /// initializer, or an assignment to a setter-less property). A deconstruction target + /// counts only in the second form: it assigns several members at once, so it can never + /// move into an initializer, and a property that kept a setter would invoke it. /// /// /// Shared by , which decides whether the @@ -1205,8 +1213,25 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms /// static bool IsConstructorStore(AstNode node, IField field, IMethod? enclosingMethod) { - if (node.Parent is not AssignmentExpression { Operator: AssignmentOperatorType.Assign } assignment - || assignment.Left != node) + AstNode currentNode = node; + bool viaDeconstruction = false; + while (true) + { + if (currentNode.Parent is AssignmentExpression { Operator: AssignmentOperatorType.Assign } + && currentNode.Slot == AssignmentExpression.LeftSlot) + { + break; + } + if (currentNode.Parent is TupleExpression) + { + viaDeconstruction = true; + currentNode = currentNode.Parent; + continue; + } + return false; + } + if (viaDeconstruction + && (!IsBackingFieldOfAutomaticProperty(field, out var property) || property.CanSet)) { return false; } From 4a0918cbe5f6a34ef5babff4083a904d676fb614 Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Thu, 10 Sep 2026 18:16:38 +0200 Subject: [PATCH 2/2] Look past comment placeholders when matching statement sequences The decompiler emits comments - //IL_ warnings, "Could not convert BlockContainer", "try-fault", a Nop's comment - as an EmptyStatement in the middle of a statement sequence. Every transform that walks such a sequence then stops recognizing its pattern the moment one of those lands in it: constructor initializers stay in the body, `using var` and `for` are not introduced, and a finalizer keeps its `override Finalize` shape, which does not compile at all. The destructor matcher moves the placeholders it skipped into the body that replaces the old one, so the warning that caused the problem is not dropped along with the statement carrying it. Assisted-by: Claude:claude-opus-5:Claude Code --- .../CSharp/Syntax/SyntaxExtensions.cs | 31 +++++++ .../CSharp/Transforms/FlattenSwitchBlocks.cs | 5 +- .../Transforms/PatternStatementTransform.cs | 82 +++++++++++++++---- ...ransformFieldAndConstructorInitializers.cs | 12 +-- 4 files changed, 104 insertions(+), 26 deletions(-) diff --git a/ICSharpCode.Decompiler/CSharp/Syntax/SyntaxExtensions.cs b/ICSharpCode.Decompiler/CSharp/Syntax/SyntaxExtensions.cs index daae52821..c52969a05 100644 --- a/ICSharpCode.Decompiler/CSharp/Syntax/SyntaxExtensions.cs +++ b/ICSharpCode.Decompiler/CSharp/Syntax/SyntaxExtensions.cs @@ -61,6 +61,37 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax return (Statement?)next; } + /// + /// The first statement of that is not an + /// , or null if there is none. + /// + /// + /// An empty statement is either a stray ';' or a placeholder carrying a comment the + /// decompiler emitted (a warning, an unconvertible block, a "try-fault" marker, ...). + /// Neither is a statement in the sense a transform matching a statement sequence means, + /// so every such transform has to look past them or it silently stops recognizing its + /// pattern as soon as one of those comments lands in the middle of the sequence. + /// + public static Statement? GetFirstNonEmptyStatementOrDefault(this AstNodeCollection statements) + { + return statements.FirstOrNull(statement => statement is not EmptyStatement); + } + + /// + /// The next statement after that is not an + /// , or null if there is none. + /// + /// + /// See for why the skip is needed. + /// + public static Statement? GetNextNonEmptyStatement(this Statement statement) + { + var next = statement.GetNextStatement(); + while (next is EmptyStatement) + next = next.GetNextStatement(); + return next; + } + public static bool IsArgList(this AstType? type) { var simpleType = type as SimpleType; diff --git a/ICSharpCode.Decompiler/CSharp/Transforms/FlattenSwitchBlocks.cs b/ICSharpCode.Decompiler/CSharp/Transforms/FlattenSwitchBlocks.cs index 73d62258c..99e49bde0 100644 --- a/ICSharpCode.Decompiler/CSharp/Transforms/FlattenSwitchBlocks.cs +++ b/ICSharpCode.Decompiler/CSharp/Transforms/FlattenSwitchBlocks.cs @@ -33,10 +33,11 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms { foreach (var switchSection in rootNode.Descendants.OfType()) { - if (switchSection.Statements.Count != 1) + var onlyStatement = switchSection.Statements.GetFirstNonEmptyStatementOrDefault(); + if (onlyStatement == null || onlyStatement.GetNextNonEmptyStatement() != null) continue; - var blockStatement = switchSection.Statements.First() as BlockStatement; + var blockStatement = onlyStatement as BlockStatement; if (blockStatement == null || blockStatement.Statements.Any(ContainsLocalDeclaration)) continue; diff --git a/ICSharpCode.Decompiler/CSharp/Transforms/PatternStatementTransform.cs b/ICSharpCode.Decompiler/CSharp/Transforms/PatternStatementTransform.cs index b5a40d081..e82fb461c 100644 --- a/ICSharpCode.Decompiler/CSharp/Transforms/PatternStatementTransform.cs +++ b/ICSharpCode.Decompiler/CSharp/Transforms/PatternStatementTransform.cs @@ -197,7 +197,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms if (!m1.Success) return null; var variable = m1.Get("variable").Single().GetILVariable(); - AstNode? next = node.NextSibling; + AstNode? next = node.GetNextNonEmptyStatement(); if (next == null) return null; if (next is ForStatement forStatement && ForStatementUsesVariable(forStatement, variable)) @@ -598,7 +598,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms Match m = default(Match); while (i < upperBounds.Length && MatchLowerBound(i, out var indexVariable, collection, stmt)) { - m = forOnArrayMultiDimPattern.Match(stmt.GetNextStatement()); + m = forOnArrayMultiDimPattern.Match(stmt.GetNextNonEmptyStatement()); if (!m.Success) return false; var upperBound = m.Get("upperBoundVariable").Single().GetILVariable(); @@ -654,7 +654,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms if (!int.TryParse(m.Get("index").Single().Value?.ToString() ?? "", out int index) || index != i) break; upperBounds[i] = m.Get("variable").Single().GetILVariable()!; - stmt = stmt.GetNextStatement(); + stmt = stmt.GetNextNonEmptyStatement(); i++; } while (stmt != null && upperBounds != null && i < upperBounds.Length); @@ -664,7 +664,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms return null; statementsToDelete.Add(stmt); // The matched multi-dimensional foreach pattern guarantees a statement after stmt. - statementsToDelete.Add(stmt.GetNextStatement()!); + statementsToDelete.Add(stmt.GetNextNonEmptyStatement()!); var itemVariable = foreachVariable.GetILVariable(); if (itemVariable == null || !itemVariable.IsSingleDefinition || (itemVariable.Kind != IL.VariableKind.Local && itemVariable.Kind != IL.VariableKind.StackSlot) @@ -1261,34 +1261,79 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms #endregion #region Destructor - static readonly BlockStatement destructorBodyPattern = new BlockStatement { - new TryCatchStatement { - TryBlock = new AnyNode("body"), - FinallyBlock = new BlockStatement { - new InvocationExpression(new MemberReferenceExpression(new BaseReferenceExpression(), "Finalize")) - } - } + static readonly TryCatchStatement destructorTryFinallyPattern = new TryCatchStatement { + TryBlock = new AnyNode("body"), + FinallyBlock = new AnyNode("finallyBlock") }; + static readonly Statement baseFinalizeCallPattern = new ExpressionStatement( + new InvocationExpression(new MemberReferenceExpression(new BaseReferenceExpression(), "Finalize"))); + static readonly MethodDeclaration destructorPattern = new MethodDeclaration { Attributes = { new Repeat(new AnyNode()) }, Modifiers = Modifiers.Any, ReturnType = new PrimitiveType("void"), Name = "Finalize", - Body = destructorBodyPattern + Body = new AnyNode() }; + /// + /// Matches the body a compiler emits for a destructor - a single try statement whose + /// finally block does nothing but call base.Finalize() - and returns the try block + /// holding the user-written code, or null if has another + /// shape. Comment placeholders around the two statements are skipped: leaving the method + /// in its "override Finalize" shape over a decompiler warning produces output that does + /// not compile (CS0249). + /// + static BlockStatement? MatchDestructorBody(BlockStatement body) + { + var statement = body.Statements.GetFirstNonEmptyStatementOrDefault(); + if (statement is not TryCatchStatement || statement.GetNextNonEmptyStatement() != null) + return null; + Match m = destructorTryFinallyPattern.Match(statement); + if (!m.Success) + return null; + var finalizeCall = m.Get("finallyBlock").Single() + .Statements.GetFirstNonEmptyStatementOrDefault(); + if (finalizeCall == null || finalizeCall.GetNextNonEmptyStatement() != null + || !baseFinalizeCallPattern.IsMatch(finalizeCall)) + { + return null; + } + return m.Get("body").Single(); + } + + /// + /// Moves the comment placeholders of to the front of + /// , which replaces it. They describe the member, so dropping + /// them with the body they happen to sit in would lose a decompiler warning. + /// + static void MovePlaceholderComments(BlockStatement oldBody, BlockStatement newBody) + { + var anchor = newBody.Statements.FirstOrNull(); + foreach (var placeholder in oldBody.Statements.OfType().ToList()) + { + placeholder.Detach(); + if (anchor != null) + newBody.Statements.InsertBefore(anchor, placeholder); + else + newBody.Statements.Add(placeholder); + } + } + DestructorDeclaration? TransformDestructor(MethodDeclaration methodDef) { Match m = destructorPattern.Match(methodDef); - if (m.Success) + if (m.Success && methodDef.Body is BlockStatement oldBody + && MatchDestructorBody(oldBody) is BlockStatement tryBlock) { context.Step("Convert Finalize method to destructor", methodDef); DestructorDeclaration dd = new DestructorDeclaration(); methodDef.Attributes.MoveTo(dd.Attributes); dd.CopyAnnotationsFrom(methodDef); dd.Modifiers = methodDef.Modifiers & ~(Modifiers.Protected | Modifiers.Override); - dd.Body = m.Get("body").Single().Detach(); + MovePlaceholderComments(oldBody, tryBlock); + dd.Body = tryBlock.Detach(); // A destructor only appears inside a type declaration, so the context tracker // has an enclosing type at this point. dd.Name = currentTypeDefinition!.Name; @@ -1301,11 +1346,12 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms DestructorDeclaration? TransformDestructorBody(DestructorDeclaration dtorDef) { - Match m = destructorBodyPattern.Match(dtorDef.Body); - if (m.Success) + if (dtorDef.Body is BlockStatement oldBody + && MatchDestructorBody(oldBody) is BlockStatement tryBlock) { context.Step("Simplify destructor body", dtorDef); - dtorDef.Body = m.Get("body").Single().Detach(); + MovePlaceholderComments(oldBody, tryBlock); + dtorDef.Body = tryBlock.Detach(); return dtorDef; } return null; @@ -1455,7 +1501,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms if (!context.Settings.UseEnhancedUsing) return usingStatement; - if (usingStatement.GetNextStatement() != null || !(usingStatement.Parent is BlockStatement)) + if (usingStatement.GetNextNonEmptyStatement() != null || !(usingStatement.Parent is BlockStatement)) return usingStatement; if (!(usingStatement.ResourceAcquisition is VariableDeclarationStatement)) diff --git a/ICSharpCode.Decompiler/CSharp/Transforms/TransformFieldAndConstructorInitializers.cs b/ICSharpCode.Decompiler/CSharp/Transforms/TransformFieldAndConstructorInitializers.cs index b7d379594..b287b541c 100644 --- a/ICSharpCode.Decompiler/CSharp/Transforms/TransformFieldAndConstructorInitializers.cs +++ b/ICSharpCode.Decompiler/CSharp/Transforms/TransformFieldAndConstructorInitializers.cs @@ -130,7 +130,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms bool skippedStmts = false; Statement? stmt; - for (stmt = ctor.Body?.Statements.FirstOrDefault(); stmt != null; stmt = stmt.GetNextStatement()) + for (stmt = ctor.Body?.Statements.GetFirstNonEmptyStatementOrDefault(); stmt != null; stmt = stmt.GetNextNonEmptyStatement()) { var m = memberInitializerPattern.Match(stmt); if (!m.Success) @@ -178,7 +178,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms : ThisCallClassPattern.Match(stmt); if (m.Success) { - sequence.CoversFullBody = stmt.GetNextStatement() == null; + sequence.CoversFullBody = stmt.GetNextNonEmptyStatement() == null; } } } @@ -228,7 +228,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms if (ctor.Body is null) return false; var stmts = ctor.Body.Statements; - var otherStmt = stmts.FirstOrDefault(); + var otherStmt = stmts.GetFirstNonEmptyStatementOrDefault(); foreach (var (stmt, member, initializer, _) in Statements) { var m = memberInitializerPattern.Match(otherStmt); @@ -249,7 +249,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms StatementToOtherCtorsMap[stmt] = list; } list.Add((otherStmt, otherInitializer)); - otherStmt = otherStmt.GetNextStatement(); + otherStmt = otherStmt.GetNextNonEmptyStatement(); } return true; } @@ -369,7 +369,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms else { // find this-ctor call - var stmt = ctor.Body?.Statements.FirstOrDefault(); + var stmt = ctor.Body?.Statements.GetFirstNonEmptyStatementOrDefault(); var m = ctorMethod.DeclaringType.Kind == TypeKind.Struct ? ThisCallStructPattern.Match(stmt) : ThisCallClassPattern.Match(stmt); @@ -527,7 +527,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms { if (constructorDeclaration.Body is null) return false; - Statement stmt = constructorDeclaration.Body.Statements.FirstOrDefault()!; + Statement stmt = constructorDeclaration.Body.Statements.GetFirstNonEmptyStatementOrDefault()!; var isValueType = ctorMethod.DeclaringType.Kind == TypeKind.Struct; // value types may omit the constructor initializer completely