From 022b4df2109fc811a16c025a0e451437a057be73 Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Thu, 6 Aug 2026 17:12:51 +0200 Subject: [PATCH 01/11] Enable nullable reference types in DeconstructionTransform The transform is about to be extended substantially; annotating it first keeps the null contracts of the matcher explicit, where "no match" is expressed by a null out-argument throughout. The matching state fields are non-null only while a match is in progress, which the codebase's null! idiom expresses; MatchConversion additionally gets the null check its caller's ElementAtOrDefault already implies. Assisted-by: Claude:claude-opus-5:Claude Code --- .../IL/Transforms/DeconstructionTransform.cs | 64 ++++++++++--------- 1 file changed, 35 insertions(+), 29 deletions(-) diff --git a/ICSharpCode.Decompiler/IL/Transforms/DeconstructionTransform.cs b/ICSharpCode.Decompiler/IL/Transforms/DeconstructionTransform.cs index f9938f3ed4..809fdd7607 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/DeconstructionTransform.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/DeconstructionTransform.cs @@ -16,10 +16,13 @@ // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER // DEALINGS IN THE SOFTWARE. +#nullable enable + using System; using System.Collections.Generic; using System.Collections.Immutable; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.Linq; using System.Resources; @@ -34,11 +37,11 @@ namespace ICSharpCode.Decompiler.IL.Transforms /// class DeconstructionTransform : IStatementTransform { - StatementTransformContext context; + StatementTransformContext context = null!; readonly Dictionary deconstructionResultsLookup = new Dictionary(); - ILVariable[] deconstructionResults; - ILVariable tupleVariable; - TupleType tupleType; + ILVariable?[] deconstructionResults = null!; + ILVariable? tupleVariable; + TupleType? tupleType; /* stloc tuple(call MakeIntIntTuple(ldloc this)) @@ -81,7 +84,7 @@ void IStatementTransform.Run(Block block, int pos, StatementTransformContext con } finally { - this.context = null; + this.context = null!; Reset(); } } @@ -91,20 +94,20 @@ private void Reset() this.deconstructionResultsLookup.Clear(); this.tupleVariable = null; this.tupleType = null; - this.deconstructionResults = null; + this.deconstructionResults = null!; } struct ConversionInfo { - public IType inputType; - public Conv conv; + public IType? inputType; + public Conv? conv; } /// /// Get index of deconstruction result or tuple element /// Returns -1 on failure. /// - int FindIndex(ILInstruction inst, out Action delayedActions) + int FindIndex(ILInstruction inst, out Action? delayedActions) { delayedActions = null; if (inst.MatchLdLoc(out var v)) @@ -129,7 +132,7 @@ int FindIndex(ILInstruction inst, out Action delayedActi this.tupleType = (TupleType)tupleType; this.deconstructionResults = new ILVariable[this.tupleType.Cardinality]; } - if (this.tupleType.Cardinality < 2) + if (this.tupleType!.Cardinality < 2) return -1; if (v != tupleVariable || !this.tupleType.Equals(tupleType)) return -1; @@ -140,7 +143,7 @@ int FindIndex(ILInstruction inst, out Action delayedActi this.deconstructionResults[index] = freshVar; } delayedActions += _ => { - inst.ReplaceWith(new LdLoc(this.deconstructionResults[index])); + inst.ReplaceWith(new LdLoc(this.deconstructionResults[index]!)); }; return index; } @@ -190,9 +193,9 @@ bool InlineDeconstructionInitializer(Block block, int pos) bool TransformDeconstruction(Block block, int pos) { int startPos = pos; - Action delayedActions = null; - if (MatchDeconstruction(block.Instructions[pos], out IMethod deconstructMethod, - out ILInstruction rootTestedOperand)) + Action? delayedActions = null; + if (MatchDeconstruction(block.Instructions[pos], out IMethod? deconstructMethod, + out ILInstruction? rootTestedOperand)) { pos++; } @@ -210,8 +213,8 @@ bool TransformDeconstruction(Block block, int pos) IType deconstructedType; if (deconstructMethod == null) { - deconstructedType = this.tupleType; - rootTestedOperand = new LdLoc(this.tupleVariable); + deconstructedType = this.tupleType!; + rootTestedOperand = new LdLoc(this.tupleVariable!); } else { @@ -225,17 +228,17 @@ bool TransformDeconstruction(Block block, int pos) } } var rootTempVariable = context.Function.RegisterVariable(VariableKind.PatternLocal, deconstructedType); - replacement.Pattern = new MatchInstruction(rootTempVariable, deconstructMethod, rootTestedOperand) { + replacement.Pattern = new MatchInstruction(rootTempVariable, deconstructMethod, rootTestedOperand!) { IsDeconstructCall = deconstructMethod != null, IsDeconstructTuple = this.tupleType != null }; int index = 0; - foreach (ILVariable v in deconstructionResults) + foreach (ILVariable? v in deconstructionResults) { var result = v; if (result == null) { - var freshVar = new ILVariable(VariableKind.PatternLocal, this.tupleType.ElementTypes[index]) { Name = "E_" + index }; + var freshVar = new ILVariable(VariableKind.PatternLocal, this.tupleType!.ElementTypes[index]) { Name = "E_" + index }; context.Function.Variables.Add(freshVar); result = freshVar; } @@ -264,12 +267,12 @@ bool TransformDeconstruction(Block block, int pos) return true; } - bool MatchDeconstruction(ILInstruction inst, out IMethod deconstructMethod, - out ILInstruction testedOperand) + bool MatchDeconstruction(ILInstruction inst, [NotNullWhen(true)] out IMethod? deconstructMethod, + [NotNullWhen(true)] out ILInstruction? testedOperand) { testedOperand = null; deconstructMethod = null; - deconstructionResults = null; + deconstructionResults = null!; if (!(inst is CallInstruction call)) return false; if (!MatchInstruction.IsDeconstructMethod(call.Method)) @@ -306,7 +309,7 @@ bool MatchDeconstruction(ILInstruction inst, out IMethod deconstructMethod, bool MatchConversions(Block block, ref int pos, out Dictionary conversions, out List conversionStLocs, - ref Action delayedActions) + ref Action? delayedActions) { conversions = new Dictionary(); conversionStLocs = new List(); @@ -330,11 +333,14 @@ bool MatchConversions(Block block, ref int pos, return true; } - bool MatchConversion(ILInstruction inst, out ILInstruction inputInstruction, - out ILVariable outputVariable, out ConversionInfo info) + bool MatchConversion(ILInstruction? inst, [NotNullWhen(true)] out ILInstruction? inputInstruction, + [NotNullWhen(true)] out ILVariable? outputVariable, out ConversionInfo info) { info = default; inputInstruction = null; + outputVariable = null; + if (inst == null) + return false; if (!inst.MatchStLoc(out outputVariable, out var value)) return false; if (!(value is Conv conv)) @@ -350,7 +356,7 @@ bool MatchConversion(ILInstruction inst, out ILInstruction inputInstruction, bool MatchAssignments(Block block, ref int pos, Dictionary conversions, List conversionStLocs, - ref Action delayedActions) + ref Action? delayedActions) { int previousIndex = -1; int conversionStLocIndex = 0; @@ -374,7 +380,7 @@ bool MatchAssignments(Block block, ref int pos, && conversionInfo.conv == null) { delayedActions += _ => { - assignmentTarget.Type = conversionInfo.inputType; + assignmentTarget.Type = conversionInfo.inputType!; }; } else @@ -449,7 +455,7 @@ int GetAssignmentIndex(ILInstruction inst) return int.MaxValue; } - void AddMissingAssignmentsForConversions(int index, ref Action delayedActions) + void AddMissingAssignmentsForConversions(int index, ref Action? delayedActions) { while (conversionStLocIndex < conversionStLocs.Count) { @@ -472,7 +478,7 @@ void AddMissingAssignmentsForConversions(int index, ref Action addAssignment) + bool MatchAssignment(ILInstruction? inst, [NotNullWhen(true)] out IType? targetType, [NotNullWhen(true)] out ILInstruction? valueInst, [NotNullWhen(true)] out Action? addAssignment) { targetType = null; valueInst = null; From 28f1a0d289de9fd926c43439e394fa7779be57bf Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Thu, 6 Aug 2026 17:13:48 +0200 Subject: [PATCH 02/11] Fix TupleType.FromUnderlyingType crash on non-tuple input GetTupleElementTypes returns a default ImmutableArray when the type is not tuple-compatible, so reading Length threw NullReferenceException instead of taking the documented return-null path. Assisted-by: Claude:claude-fable-5:Claude Code --- ICSharpCode.Decompiler/TypeSystem/TupleType.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ICSharpCode.Decompiler/TypeSystem/TupleType.cs b/ICSharpCode.Decompiler/TypeSystem/TupleType.cs index 0da4777f1e..a1267f6bff 100644 --- a/ICSharpCode.Decompiler/TypeSystem/TupleType.cs +++ b/ICSharpCode.Decompiler/TypeSystem/TupleType.cs @@ -147,7 +147,7 @@ public static bool IsTupleCompatible(IType type, out int tupleCardinality) public static TupleType FromUnderlyingType(ICompilation compilation, IType type) { var elementTypes = GetTupleElementTypes(type); - if (elementTypes.Length > 0) + if (!elementTypes.IsDefaultOrEmpty) { return new TupleType( compilation, From 72ec11e27a2f08c1f9ffb6606c0c9a39dbb319ab Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Fri, 7 Aug 2026 13:40:17 +0200 Subject: [PATCH 03/11] Require System.ValueTuple to be a struct to be tuple compatible C# only accepts System.ValueTuple as a tuple when it is a struct, so a class of that name is an unrelated type and rendering it with tuple syntax describes it as something it is not. It also made a tuple appear to contain itself, which no struct can, and the deconstruction transform then registered the same variable as a node of its tuple tree twice and threw ArgumentException, failing the whole method instead of leaving the statements alone. The check has accepted classes since tuples were added to the type system, alongside a name comparison against "ValueType" that was corrected later. Assisted-by: Claude:claude-opus-5[1m]:Claude Code --- ICSharpCode.Decompiler/TypeSystem/TupleType.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ICSharpCode.Decompiler/TypeSystem/TupleType.cs b/ICSharpCode.Decompiler/TypeSystem/TupleType.cs index a1267f6bff..7576e817d6 100644 --- a/ICSharpCode.Decompiler/TypeSystem/TupleType.cs +++ b/ICSharpCode.Decompiler/TypeSystem/TupleType.cs @@ -115,8 +115,9 @@ public static bool IsTupleCompatible(IType type, out int tupleCardinality) case TypeKind.Tuple: tupleCardinality = ((TupleType)type).ElementTypes.Length; return true; - case TypeKind.Class: case TypeKind.Struct: + // C# requires System.ValueTuple to be a struct, so a class of that name is + // some other type that happens to share it and must not become tuple syntax. if (type.Namespace == "System" && type.Name == "ValueTuple") { int tpc = type.TypeParameterCount; From d7e19172f3aaed7f0b94e297507bb3b98e2952ff Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Thu, 6 Aug 2026 17:14:18 +0200 Subject: [PATCH 04/11] Support deconstruction into pointer targets Deconstruction into a pointer target ((*p, value) = tuple;) stayed an explicit Deconstruct call: a store through a pointer (or through a target whose pointer type got erased in a stack slot) does not infer a ByReferenceType, so IsAssignment reported an unknown expected type and the transform's conversion check rejected the assignment. The type of the store itself is just as precise, so use it as the expected type. Of the three IsAssignment call sites only the transform's MatchAssignment consumes the expected type; CheckInvariant and GetAssignmentIndex discard it, so this widens what the transform accepts without weakening the invariant check. Assisted-by: Claude:claude-opus-5:Claude Code --- .../TestCases/Pretty/DeconstructionTests.cs | 27 ++++++++++++++++ .../IL/Instructions/DeconstructInstruction.cs | 31 ++++++++++++++++++- 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/DeconstructionTests.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/DeconstructionTests.cs index 1e1802eb26..28ea00d55b 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/DeconstructionTests.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/DeconstructionTests.cs @@ -322,6 +322,33 @@ public void LocalVariable_NoConversion_Struct_Custom() Console.WriteLine(value2); } + public unsafe void Pointer_NoConversion_Tuple(int* p) + { + int value; + (*p, value) = GetTuple(); + Console.WriteLine(value); + Console.WriteLine(value); + } + + // The store opcode is sign-agnostic - stind.i4 reports int for a uint target and + // stind.i1 reports sbyte for a byte one - so the element type of the target cannot + // be taken from it: doing so refuses every one of these deconstructions. + public unsafe void Pointer_NoConversion_Tuple_UInt(uint* p) + { + int value; + (*p, value) = GetTuple(); + Console.WriteLine(value); + Console.WriteLine(value); + } + + public unsafe void Pointer_NoConversion_Tuple_Byte(byte* p) + { + int value; + (*p, value) = GetTuple(); + Console.WriteLine(value); + Console.WriteLine(value); + } + public void Property_NoConversion_Custom() { (Get(0).NMy, Get(1).My) = GetSource(); diff --git a/ICSharpCode.Decompiler/IL/Instructions/DeconstructInstruction.cs b/ICSharpCode.Decompiler/IL/Instructions/DeconstructInstruction.cs index f2993a7f84..3d13f228da 100644 --- a/ICSharpCode.Decompiler/IL/Instructions/DeconstructInstruction.cs +++ b/ICSharpCode.Decompiler/IL/Instructions/DeconstructInstruction.cs @@ -259,7 +259,14 @@ internal static bool IsAssignment(ILInstruction inst, ICompilation typeSystem, o if (stobj.Target.InferType(typeSystem) is ByReferenceType brt) expectedType = brt.ElementType; else - expectedType = SpecialType.UnknownType; + { + // Pointer targets do not infer a ByReferenceType. stobj.Type cannot stand + // in for the element type: it comes from the store opcode, which is + // sign-agnostic, so a uint* and an int* both report int32 and a byte* + // reports sbyte. Recover the declared type instead, and only fall back to + // the store where the target is not a pointer at all. + expectedType = GetPointerElementType(stobj.Target, typeSystem) ?? stobj.Type; + } value = stobj.Value; return true; default: @@ -267,6 +274,28 @@ internal static bool IsAssignment(ILInstruction inst, ICompilation typeSystem, o } } + /// + /// The element type of a pointer-typed target. A pointer passing through a stack slot + /// is typed IntPtr there, so the declared type has to be taken from the definition the + /// slot was filled from. Returns null if the target is not a pointer. + /// + static IType GetPointerElementType(ILInstruction target, ICompilation typeSystem) + { + // Bounded because a definition chain could be cyclic in invalid IL. + for (int step = 0; step < 4; step++) + { + if (target.InferType(typeSystem) is PointerType pointerType) + return pointerType.ElementType; + if (!target.MatchLdLoc(out var v) || !v.IsSingleDefinition + || v.StoreInstructions.Count != 1 || !(v.StoreInstructions[0] is StLoc store)) + { + return null; + } + target = store.Value; + } + return null; + } + internal override void CheckInvariant(ILPhase phase) { base.CheckInvariant(phase); From 481a4de3576cb870756f4856000ec4868209f801 Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Thu, 6 Aug 2026 17:14:49 +0200 Subject: [PATCH 05/11] End a call-rooted deconstruction pattern at an unrelated assignment An assignment whose value is not one of the deconstruction's elements used to reject the whole match, so a custom deconstruction followed by any unrelated assignment stayed an explicit Deconstruct call. For a pattern rooted in a Deconstruct call the element list is fixed by the call's out-arguments, so such an assignment simply ends the pattern and stays after the deconstruct instruction. Tuple-rooted patterns keep rejecting: their element list is discovered from the assignments, so ending early would misread a suffix of the assignments as the whole pattern and fabricate discards for the elements before it. Assisted-by: Claude:claude-opus-5:Claude Code --- .../TestCases/Pretty/DeconstructionTests.cs | 14 ++++++++++++++ .../IL/Transforms/DeconstructionTransform.cs | 16 ++++++++++++++-- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/DeconstructionTests.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/DeconstructionTests.cs index 28ea00d55b..503dbac354 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/DeconstructionTests.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/DeconstructionTests.cs @@ -159,6 +159,11 @@ private ref T GetRef() return default((T, T2, T3)); } + private int GetInt() + { + return 0; + } + private AssignmentTargets Get(int i) { return null; @@ -171,6 +176,15 @@ public void LocalVariable_NoConversion_Custom() Console.WriteLine(myInt4); } + public void LocalVariable_NoConversion_Custom_UnrelatedAssignmentAfter() + { + var (myInt3, myInt4) = GetSource(); + int value = GetInt(); + Console.WriteLine(myInt3); + Console.WriteLine(myInt4); + Console.WriteLine(value); + } + public void LocalVariable_NoConversion_Tuple() { var (myInt, myInt2) = GetTuple(); diff --git a/ICSharpCode.Decompiler/IL/Transforms/DeconstructionTransform.cs b/ICSharpCode.Decompiler/IL/Transforms/DeconstructionTransform.cs index 809fdd7607..1881146a3a 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/DeconstructionTransform.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/DeconstructionTransform.cs @@ -202,7 +202,8 @@ bool TransformDeconstruction(Block block, int pos) if (!MatchConversions(block, ref pos, out var conversions, out var conversionStLocs, ref delayedActions)) return false; - if (!MatchAssignments(block, ref pos, conversions, conversionStLocs, ref delayedActions)) + if (!MatchAssignments(block, ref pos, conversions, conversionStLocs, ref delayedActions, + allowUnrelatedAssignments: deconstructMethod != null)) return false; // first tuple element may not be discarded, // otherwise we would run this transform on a suffix of the actual pattern. @@ -356,7 +357,8 @@ bool MatchConversion(ILInstruction? inst, [NotNullWhen(true)] out ILInstruction? bool MatchAssignments(Block block, ref int pos, Dictionary conversions, List conversionStLocs, - ref Action? delayedActions) + ref Action? delayedActions, + bool allowUnrelatedAssignments) { int previousIndex = -1; int conversionStLocIndex = 0; @@ -364,6 +366,16 @@ bool MatchAssignments(Block block, ref int pos, while (MatchAssignment(block.Instructions.ElementAtOrDefault(pos), out var targetType, out var valueInst, out var addAssignment)) { int index = FindIndex(valueInst, out var tupleAccessAdjustment); + if (index < 0 && allowUnrelatedAssignments) + { + // For a Deconstruct call the element list is fixed by the call's + // out-arguments, so an assignment whose value is unrelated to the + // deconstruction just ends the pattern and stays after the deconstruct + // instruction. (For tuples the elements are discovered from the + // assignments, so ending early would misread a suffix as the pattern: + // keep rejecting there.) + break; + } if (index <= previousIndex) return false; AddMissingAssignmentsForConversions(index, ref delayedActions); From 8117cf59c3de040367816d6e7e58cf1eeea95716 Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Thu, 6 Aug 2026 17:16:03 +0200 Subject: [PATCH 06/11] Reconstruct nested Deconstruct-call designations A nested designation, var (x, (a, b)) = o;, is lowered to a chain of Deconstruct calls - the inner call taking the outer call's out-argument as its target, through a defensive copy where the element is a struct - and decompiled as a flat deconstruction followed by an explicit Deconstruct call. The IL pattern node, its invariants and the C# builders already support nested patterns; only the transform never built them. MatchDeconstruction now consumes the chain into a tree of match patterns. The leaves get flat indices in depth-first order, which is the order in which StatementBuilder and ExpressionBuilder pair pattern variables with assignments, so the conversion and assignment matching runs unchanged on top of a nested pattern. Two matching rules follow from the chain being consumed: a call pattern no longer needs a matched assignment, because single-use leaves are covered by the forwarding fixup in MatchAssignments; and a pattern is not rooted on an element of an enclosing deconstruction, because blocks are processed back to front, so the inner call is visited first and would otherwise consume the pattern piecemeal, starving the outer call. That guard runs the enclosing match as a dry run, which is precise: a barrier statement between the calls or an element with further uses makes it fail, and the inner deconstruction is then still transformed on its own. Assisted-by: Claude:claude-opus-5:Claude Code --- .../Correctness/DeconstructionTests.cs | 209 ++++++ .../TestCases/Pretty/DeconstructionTests.cs | 191 ++++++ .../IL/Transforms/DeconstructionTransform.cs | 630 +++++++++++++----- 3 files changed, 878 insertions(+), 152 deletions(-) diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Correctness/DeconstructionTests.cs b/ICSharpCode.Decompiler.Tests/TestCases/Correctness/DeconstructionTests.cs index 15c49cb9a8..dc7cdb091e 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Correctness/DeconstructionTests.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Correctness/DeconstructionTests.cs @@ -15,6 +15,15 @@ public static void Deconstruct(this KeyValuePair pai } } + static class TupleClassExtensions + { + public static void Deconstruct(this Tuple tuple, out T1 item1, out T2 item2) + { + item1 = tuple.Item1; + item2 = tuple.Item2; + } + } + class DeconstructionTests { public static void Main() @@ -153,6 +162,206 @@ public void Test() new NestedOuter { Value = 2 } }); NestedDeconstruction_DiscardedElement(new KeyValuePair("key", default(DiscardData))); + NestedDeconstruction_ClassInner(new ClassInnerOuter { Value = 7 }); + NestedDeconstruction_Depth3(new DeepOuter { Value = 3 }); + NestedDeconstruction_LhsSideEffects_DeconstructionOrder_Assignments(); + NestedDeconstruction_Conversions_AfterAllDeconstructCalls(); + NestedDeconstruction_TypedDeclaration_Conversions(new NestedOuter { Value = 5 }); + NestedDeconstruction_DiscardWithSideEffectTargets(); + NestedDeconstruction_SystemTupleSource(Tuple.Create(8, new NestedInner { Value = 4 })); + NestedDeconstruction_CheckedConversions(new NestedOuter { Value = 9 }); + NestedDeconstruction_GenericConstraintSource(new ConstrainedSource { Value = 11 }); + NestedDeconstruction_InParameterSource(new NestedOuter { Value = 12 }); + NestedDeconstruction_ConditionalSource(c: true, new NestedOuter { Value = 13 }, new NestedOuter { Value = 14 }); + NestedDeconstruction_TupleOuterConversions((15, new NestedInner { Value = 6 })); + NestedDeconstruction_TypedConversions_UnrelatedCallAfter(new NestedOuter { Value = 16 }); + NestedDeconstruction_NullableConversions(new NestedOuter { Value = 17 }); + NestedDeconstruction_MyIntConversionOnNestedLeaves(new NestedOuter { Value = 18 }); + NestedDeconstruction_ForEachDictionary_Conversions(new Dictionary { + { "k1", new NestedInner { Value = 19 } } + }); + } + + public class ConstrainedSource + { + public int Value; + + public void Deconstruct(out int a, out NestedInner inner) + { + Console.WriteLine("ConstrainedSource.Deconstruct"); + a = Value; + inner = new NestedInner { Value = Value * 10 }; + } + } + + public void NestedDeconstruction_SystemTupleSource(Tuple tup) + { + Console.WriteLine("NestedDeconstruction_SystemTupleSource:"); + (long x, (long a, long b)) = tup; + int z = Side(); + Console.WriteLine(x + " " + a + " " + b + " " + z); + } + + public void NestedDeconstruction_CheckedConversions(NestedOuter o) + { + Console.WriteLine("NestedDeconstruction_CheckedConversions:"); + checked + { + (long x, (long a, long b)) = o; + Console.WriteLine(x + " " + a + " " + b); + } + } + + public void NestedDeconstruction_GenericConstraintSource(T o) where T : ConstrainedSource + { + Console.WriteLine("NestedDeconstruction_GenericConstraintSource:"); + var (a, (c, d)) = o; + Console.WriteLine(a + " " + c + " " + d); + } + + public void NestedDeconstruction_InParameterSource(in NestedOuter o) + { + Console.WriteLine("NestedDeconstruction_InParameterSource:"); + (long x, (long a, long b)) = o; + Console.WriteLine(x + " " + a + " " + b); + } + + public void NestedDeconstruction_ConditionalSource(bool c, NestedOuter o1, NestedOuter o2) + { + Console.WriteLine("NestedDeconstruction_ConditionalSource:"); + (long x, (int a, int b)) = c ? o1 : o2; + int z = Side(); + Console.WriteLine(x + " " + a + " " + b + " " + z); + } + + public void NestedDeconstruction_TupleOuterConversions((int, NestedInner) tup) + { + Console.WriteLine("NestedDeconstruction_TupleOuterConversions:"); + (long x, (long a, long b)) = tup; + Console.WriteLine(x + " " + a + " " + b); + } + + public void NestedDeconstruction_TypedConversions_UnrelatedCallAfter(NestedOuter o) + { + Console.WriteLine("NestedDeconstruction_TypedConversions_UnrelatedCallAfter:"); + (long x, (long a, long b)) = o; + int z = Side(); + Console.WriteLine(x + " " + a + " " + b + " " + z); + } + + public void NestedDeconstruction_NullableConversions(NestedOuter o) + { + Console.WriteLine("NestedDeconstruction_NullableConversions:"); + (long? x, (long? a, int? b)) = o; + Console.WriteLine(x + " " + a + " " + b); + } + + public void NestedDeconstruction_MyIntConversionOnNestedLeaves(NestedOuter o) + { + Console.WriteLine("NestedDeconstruction_MyIntConversionOnNestedLeaves:"); + (MyInt x, (MyInt a, long b)) = o; + Console.WriteLine(x + " " + a + " " + b); + } + + public void NestedDeconstruction_ForEachDictionary_Conversions(Dictionary d) + { + Console.WriteLine("NestedDeconstruction_ForEachDictionary_Conversions:"); + foreach ((string k, (long a, long b)) in d) + { + Console.WriteLine(k + " " + a + " " + b); + } + } + + // The evaluation order of a deconstruction-assignment is: (1) all side-effects of + // the left-hand-side targets, (2) all Deconstruct invocations, (3) conversions, + // (4) assignments. Get(i), the Deconstruct methods, MyInt's implicit conversions, + // and the property setters all print, so any phase reordering breaks the output diff. + public void NestedDeconstruction_LhsSideEffects_DeconstructionOrder_Assignments() + { + Console.WriteLine("NestedDeconstruction_LhsSideEffects_DeconstructionOrder_Assignments:"); + (Get(0).IntProperty, (Get(1).IntProperty, Get(2).IntProperty)) = new NestedOuter { Value = 11 }; + } + + public void NestedDeconstruction_Conversions_AfterAllDeconstructCalls() + { + Console.WriteLine("NestedDeconstruction_Conversions_AfterAllDeconstructCalls:"); + (Get(0).My, (Get(1).IntProperty, Get(2).My)) = new NestedOuter { Value = 21 }; + } + + public void NestedDeconstruction_TypedDeclaration_Conversions(NestedOuter o) + { + Console.WriteLine("NestedDeconstruction_TypedDeclaration_Conversions:"); + (MyInt x, (long a, MyInt b)) = o; + Console.WriteLine(x); + Console.WriteLine(a); + Console.WriteLine(b); + } + + public void NestedDeconstruction_DiscardWithSideEffectTargets() + { + Console.WriteLine("NestedDeconstruction_DiscardWithSideEffectTargets:"); + (Get(0).IntProperty, (_, Get(1).My)) = new NestedOuter { Value = 31 }; + } + + public int Side() + { + Console.WriteLine("Side()"); + return 5; + } + + public class NestedClassInner + { + public int Value; + + public void Deconstruct(out int a, out int b) + { + Console.WriteLine("NestedClassInner.Deconstruct"); + a = Value + 1; + b = Value + 2; + } + } + + public struct ClassInnerOuter + { + public int Value; + + public void Deconstruct(out int x, out NestedClassInner inner) + { + Console.WriteLine("ClassInnerOuter.Deconstruct"); + x = Value; + inner = new NestedClassInner { Value = Value * 10 }; + } + } + + public void NestedDeconstruction_ClassInner(ClassInnerOuter o) + { + Console.WriteLine("NestedDeconstruction_ClassInner:"); + var (x, (a, b)) = o; + Console.WriteLine(x); + Console.WriteLine(a); + Console.WriteLine(b); + } + + public struct DeepOuter + { + public int Value; + + public void Deconstruct(out int x, out ClassInnerOuter mid) + { + Console.WriteLine("DeepOuter.Deconstruct"); + x = Value; + mid = new ClassInnerOuter { Value = Value * 100 }; + } + } + + public void NestedDeconstruction_Depth3(DeepOuter o) + { + Console.WriteLine("NestedDeconstruction_Depth3:"); + var (x, (y, (a, b))) = o; + Console.WriteLine(x); + Console.WriteLine(y); + Console.WriteLine(a); + Console.WriteLine(b); } public struct DiscardData diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/DeconstructionTests.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/DeconstructionTests.cs index 503dbac354..e873d6968b 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/DeconstructionTests.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/DeconstructionTests.cs @@ -29,6 +29,12 @@ public static void Deconstruct(this KeyValuePair pair, out K key, ou key = pair.Key; value = pair.Value; } + + public static void Deconstruct(this Tuple tuple, out T1 item1, out T2 item2) + { + item1 = tuple.Item1; + item2 = tuple.Item2; + } } internal class DeconstructionTests @@ -159,11 +165,26 @@ private ref T GetRef() return default((T, T2, T3)); } + private List GetList() + { + return null; + } + private int GetInt() { return 0; } + private Tuple GetTupleClass() + { + return null; + } + + private Dictionary GetStringDictionary() + { + return null; + } + private AssignmentTargets Get(int i) { return null; @@ -336,6 +357,167 @@ public void LocalVariable_NoConversion_Struct_Custom() Console.WriteLine(value2); } + public void LocalVariable_Nested_ClassInner() + { + var (myInt3, (myInt4, value)) = GetSource>(); + Console.WriteLine(myInt3); + Console.WriteLine(myInt4); + Console.WriteLine(value); + } + + public void LocalVariable_Nested_StructInner() + { + var (myInt3, (myInt4, value)) = GetSource>(); + Console.WriteLine(myInt3); + Console.WriteLine(myInt4); + Console.WriteLine(value); + } + + public void LocalVariable_Nested_StructOuterAndInner() + { + var (myInt3, (myInt4, value)) = GetStructSource>(); + Console.WriteLine(myInt3); + Console.WriteLine(myInt4); + Console.WriteLine(value); + } + + public void LocalVariable_Nested_BothElementsNested() + { + var ((myInt3, value), (myInt4, value2)) = GetSource, StructDeconstructionSource>(); + Console.WriteLine(myInt3); + Console.WriteLine(value); + Console.WriteLine(myInt4); + Console.WriteLine(value2); + } + + public void LocalVariable_Nested_Depth3() + { + var (myInt3, (myInt4, (value, value2))) = GetSource>>(); + Console.WriteLine(myInt3); + Console.WriteLine(myInt4); + Console.WriteLine(value); + Console.WriteLine(value2); + } + + public void LocalVariable_Nested_DiscardInnerElement() + { + var (myInt3, (myInt4, _)) = GetSource>(); + Console.WriteLine(myInt3); + Console.WriteLine(myInt4); + } + + public void LocalVariable_Nested_SystemTupleSource() + { + var (myInt3, (myInt4, value)) = GetTupleClass>(); + Console.WriteLine(myInt3); + Console.WriteLine(myInt4); + Console.WriteLine(value); + } + + // Nested deconstruction of a tuple element (ldfld chains, no Deconstruct call) + // is not re-sugared: var (value, (value2, value3)) = GetTuple(); + public void LocalVariable_Nested_TupleInner() + { + (int, (int, int)) tuple = GetTuple(); + (int, int) item = tuple.Item2; + var (value, _) = tuple; + var (value2, value3) = item; + Console.WriteLine(value); + Console.WriteLine(value2); + Console.WriteLine(value3); + } + + public void ForEach_Nested_TupleInner() + { + foreach (var item2 in GetList<(int, (int, int))>()) + { + (int, int) item = item2.Item2; + var (value, _) = item2; + var (value2, value3) = item; + Console.WriteLine(value); + Console.WriteLine(value2); + Console.WriteLine(value3); + } + } + + public void LocalVariable_Nested_TypedConversions_UnrelatedCallAfter() + { + long value; + MyInt myInt2; + long value2; + (value, (myInt2, value2)) = GetSource>(); + int value3 = GetInt(); + Console.WriteLine(value); + Console.WriteLine(myInt2); + Console.WriteLine(value2); + Console.WriteLine(value3); + } + + public void LocalVariable_Nested_IntToLongConversion() + { + int value; + MyInt myInt2; + long value2; + (value, (myInt2, value2)) = GetSource>(); + Console.WriteLine(value); + Console.WriteLine(myInt2); + Console.WriteLine(value2); + } + + public void LocalVariable_Nested_ElementDeconstructedAfterBarrier() + { + GetSource>().Deconstruct(out var a, out var b); + Console.WriteLine(a); + var (myInt2, value) = b; + Console.WriteLine(myInt2); + Console.WriteLine(value); + } + + public void LocalVariable_Nested_OuterElementUsedTwice() + { + GetSource>().Deconstruct(out var a, out var b); + var (myInt2, value) = b; + Console.WriteLine(a); + Console.WriteLine(a); + Console.WriteLine(myInt2); + Console.WriteLine(value); + } + + public void ForEach_Nested() + { + foreach (var (myInt3, (myInt4, value)) in GetList>>()) + { + Console.WriteLine(myInt3); + Console.WriteLine(myInt4); + Console.WriteLine(value); + } + } + + public void ForEach_Nested_KeyValuePair() + { + foreach (var (value, (myInt2, value2)) in GetStringDictionary>()) + { + Console.WriteLine(value); + Console.WriteLine(myInt2); + Console.WriteLine(value2); + } + } + + public void Property_Nested_NoConversion() + { + (Get(0).Int, (Get(1).My, Get(2).String)) = GetSource>(); + } + + public void Property_Nested_IntToLongConversion() + { + (Get(0).Int, (Get(1).My, Get(2).Long)) = GetSource>(); + } + + public void Property_Nested_DiscardInnerElement() + { + (Get(0).NMy, (_, Get(1).My)) = GetSource>(); + } + public unsafe void Pointer_NoConversion_Tuple(int* p) { int value; @@ -363,6 +545,15 @@ public unsafe void Pointer_NoConversion_Tuple_Byte(byte* p) Console.WriteLine(value); } + public unsafe void Pointer_Nested_Custom(int* p) + { + MyInt myInt2; + int value; + (*p, (myInt2, value)) = GetSource>(); + Console.WriteLine(myInt2); + Console.WriteLine(value); + } + public void Property_NoConversion_Custom() { (Get(0).NMy, Get(1).My) = GetSource(); diff --git a/ICSharpCode.Decompiler/IL/Transforms/DeconstructionTransform.cs b/ICSharpCode.Decompiler/IL/Transforms/DeconstructionTransform.cs index 1881146a3a..cbd566d865 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/DeconstructionTransform.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/DeconstructionTransform.cs @@ -20,11 +20,9 @@ using System; using System.Collections.Generic; -using System.Collections.Immutable; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Linq; -using System.Resources; using ICSharpCode.Decompiler.CSharp.Resolver; using ICSharpCode.Decompiler.TypeSystem; @@ -33,8 +31,47 @@ namespace ICSharpCode.Decompiler.IL.Transforms { /// - /// + /// Detects that a run of statements is a lowered deconstruction assignment - rooted in a + /// Deconstruct call or in tuple element reads, including nested designations - and folds + /// it into a single DeconstructInstruction. /// + /* + stloc tuple(call MakeIntIntTuple(ldloc this)) + ---- + stloc myInt(call op_Implicit(ldfld Item2(ldloca tuple))) + stloc a(ldfld Item1(ldloca tuple)) + stloc b(ldloc myInt) + ==> + deconstruct { + init: + + deconstruct: + match.deconstruct(temp = ldloca tuple) { + match(result0 = deconstruct.result 0(temp)), + match(result1 = deconstruct.result 1(temp)) + } + conversions: { + stloc conv2(call op_Implicit(ldloc result1)) + } + assignments: { + stloc a(ldloc result0) + stloc b(ldloc conv2) + } + } + + A nested designation over Deconstruct calls (var (x, (a, b)) = o;) chains the calls, + with a defensive copy for struct elements: + call Deconstruct(ldloc o, ldloca x', ldloca inner) + call Deconstruct(ldloca inner, ldloca a', ldloca b') + ...conversions/assignments over the leaves x', a', b'... + + A nested designation over tuples (var (x, (a, b)) = t;) is lowered to one temporary + per nested designation, followed by element reads in depth-first leaf order: + stloc inner(ldobj(ldflda Item2(ldloca t))) + stloc x(ldobj(ldflda Item1(ldloca t))) + stloc a(ldobj(ldflda Item1(ldloca inner))) + stloc b(ldobj(ldflda Item2(ldloca inner))) + * */ class DeconstructionTransform : IStatementTransform { StatementTransformContext context = null!; @@ -43,30 +80,6 @@ class DeconstructionTransform : IStatementTransform ILVariable? tupleVariable; TupleType? tupleType; - /* - stloc tuple(call MakeIntIntTuple(ldloc this)) - ---- - stloc myInt(call op_Implicit(ldfld Item2(ldloca tuple))) - stloc a(ldfld Item1(ldloca tuple)) - stloc b(ldloc myInt) - ==> - deconstruct { - init: - - deconstruct: - match.deconstruct(temp = ldloca tuple) { - match(result0 = deconstruct.result 0(temp)), - match(result1 = deconstruct.result 1(temp)) - } - conversions: { - stloc conv2(call op_Implicit(ldloc result1)) - } - assignments: { - stloc a(ldloc result0) - stloc b(ldloc conv2) - } - } - * */ void IStatementTransform.Run(Block block, int pos, StatementTransformContext context) { if (!context.Settings.Deconstruction) @@ -97,57 +110,127 @@ private void Reset() this.deconstructionResults = null!; } - struct ConversionInfo - { - public IType? inputType; - public Conv? conv; - } - /// - /// Get index of deconstruction result or tuple element - /// Returns -1 on failure. + /// call Deconstruct(target, ldloca out0, ...) [+ nested Deconstruct calls] + /// | stloc temp(ldobj(ldflda ItemN(ldloca tuple))) ... [nested tuple designations] + /// stloc conv0(conv(...)) ... + /// assignments ... + /// => + /// deconstruct { init: pattern: conversions: assignments: } (see class comment) /// - int FindIndex(ILInstruction inst, out Action? delayedActions) + bool TransformDeconstruction(Block block, int pos) { - delayedActions = null; - if (inst.MatchLdLoc(out var v)) + int startPos = pos; + // Blocks are processed back to front, so the inner parts of a nested deconstruction + // are visited before the position its matching starts at; matching them on their own + // would consume the pattern piecemeal. Defer to the enclosing attempt where one + // exists (see the guard for the precision guarantees). + if (IsConsumableByEnclosingDeconstruction(block, pos)) + return false; + if (!MatchDeconstructionSequence(block, startPos, out pos, out var rootCall, + out var rootTestedOperand, out var conversionStLocs, out var delayedActions)) { - if (!deconstructionResultsLookup.TryGetValue(v, out int index)) - return -1; - return index; + return false; } - if (inst.MatchLdFld(out _, out _)) + context.Step("Deconstruction", block.Instructions[startPos]); + DeconstructInstruction replacement = new DeconstructInstruction(); + IMethod? deconstructMethod = rootCall?.Method; + IType deconstructedType; + if (deconstructMethod == null) { - if (!TupleTransform.MatchTupleFieldAccess((LdFlda)((LdObj)inst).Target, out var tupleType, out var target, out int index)) - return -1; - // Item fields are one-based, we use zero-based indexing. - index--; - // normalize tuple type - tupleType = TupleType.FromUnderlyingType(context.TypeSystem, tupleType); - if (!target.MatchLdLoca(out v)) - return -1; - if (this.tupleVariable == null) + deconstructedType = this.tupleType!; + rootTestedOperand = new LdLoc(this.tupleVariable!); + } + else + { + if (deconstructMethod.IsStatic) { - this.tupleVariable = v; - this.tupleType = (TupleType)tupleType; - this.deconstructionResults = new ILVariable[this.tupleType.Cardinality]; + deconstructedType = deconstructMethod.Parameters[0].Type; } - if (this.tupleType!.Cardinality < 2) - return -1; - if (v != tupleVariable || !this.tupleType.Equals(tupleType)) - return -1; - if (this.deconstructionResults[index] == null) + else { - var freshVar = new ILVariable(VariableKind.StackSlot, this.tupleType.ElementTypes[index]) { Name = "E_" + index }; - delayedActions += _ => context.Function.Variables.Add(freshVar); - this.deconstructionResults[index] = freshVar; + deconstructedType = deconstructMethod.DeclaringType; } - delayedActions += _ => { - inst.ReplaceWith(new LdLoc(this.deconstructionResults[index]!)); + } + var rootTempVariable = context.Function.RegisterVariable(VariableKind.PatternLocal, deconstructedType); + if (rootCall != null) + { + replacement.Pattern = BuildPatternMatch(rootCall, rootTempVariable, rootTestedOperand!); + } + else + { + replacement.Pattern = new MatchInstruction(rootTempVariable, method: null, rootTestedOperand!) { + IsDeconstructTuple = true }; - return index; + for (int i = 0; i < deconstructionResults.Length; i++) + { + var result = deconstructionResults[i]; + if (result == null) + { + var freshVar = new ILVariable(VariableKind.PatternLocal, this.tupleType!.ElementTypes[i]) { Name = "E_" + i }; + context.Function.Variables.Add(freshVar); + result = freshVar; + } + else + { + result.Kind = VariableKind.PatternLocal; + } + replacement.Pattern.SubPatterns.Add( + new MatchInstruction( + result, + new DeconstructResultInstruction(i, result.StackType, new LdLoc(rootTempVariable)) + ) + ); + } + } + replacement.Conversions = new Block(BlockKind.DeconstructionConversions); + foreach (var convInst in conversionStLocs) + { + replacement.Conversions.Instructions.Add(convInst); } - return -1; + replacement.Assignments = new Block(BlockKind.DeconstructionAssignments); + delayedActions?.Invoke(replacement); + block.Instructions[startPos] = replacement; + block.Instructions.RemoveRange(startPos + 1, pos - startPos - 1); + context.EndStep(replacement); + return true; + } + + /// + /// Matches the full statement sequence of one deconstruction, starting at startPos: + /// [Deconstruct call + nested calls | nested tuple designation temporaries] + /// [conversions] + /// [assignments] + /// On success, endPos is the position after the last consumed statement. + /// The block is not modified; all rewrites are accumulated in delayedActions. + /// + bool MatchDeconstructionSequence(Block block, int startPos, out int endPos, + out DeconstructionCall? rootCall, out ILInstruction? rootTestedOperand, + out List conversionStLocs, out Action? delayedActions) + { + Reset(); + endPos = startPos; + int pos = startPos; + delayedActions = null; + MatchDeconstruction(block, ref pos, out rootCall, out rootTestedOperand); + if (!MatchConversions(block, ref pos, out var conversions, out conversionStLocs, ref delayedActions)) + return false; + if (!MatchAssignments(block, ref pos, conversions, conversionStLocs, ref delayedActions, + allowUnrelatedAssignments: rootCall != null, out bool anyAssignments)) + { + return false; + } + // Without any assignment the statement is a plain Deconstruct call, unless a nested + // deconstruction was consumed: then all leaves are single-use elements handled by + // the forwarding fixup in MatchAssignments. + if (!anyAssignments && !(rootCall != null && rootCall.NestedCalls.Any(c => c != null))) + return false; + // first tuple element may not be discarded, + // otherwise we would run this transform on a suffix of the actual pattern. + if (deconstructionResults[0] == null) + return false; + endPos = pos; + return true; } /// @@ -160,7 +243,7 @@ bool InlineDeconstructionInitializer(Block block, int pos) { if (!block.Instructions[pos].MatchStLoc(out var v, out var value)) return false; - if (!(v.IsSingleDefinition && v.LoadCount == 1)) + if (!(v.IsSingleDefinition && v.LoadInstructions is [var loadInst])) return false; if (pos + 1 >= block.Instructions.Count) return false; @@ -168,7 +251,6 @@ bool InlineDeconstructionInitializer(Block block, int pos) if (result.Type != ILInlining.FindResultType.Deconstruction) return false; var deconstruction = (DeconstructInstruction)result.LoadInst; - LdLoc loadInst = v.LoadInstructions[0]; if (!loadInst.IsDescendantOf(deconstruction.Assignments)) return false; if (loadInst.SlotInfo == StObj.TargetSlot) @@ -179,8 +261,7 @@ bool InlineDeconstructionInitializer(Block block, int pos) if (deconstruction.Init.Count > 0) { var a = deconstruction.Init[0].Variable.LoadInstructions.Single(); - var b = v.LoadInstructions.Single(); - if (!b.IsBefore(a)) + if (!loadInst.IsBefore(a)) return false; } context.Step("InlineDeconstructionInitializer", block.Instructions[pos]); @@ -190,123 +271,238 @@ bool InlineDeconstructionInitializer(Block block, int pos) return true; } - bool TransformDeconstruction(Block block, int pos) + /// + /// Whether the statement at pos belongs to a deconstruction whose matching starts at an + /// earlier position in the block, in either nesting shape: + /// + /// call Deconstruct(..., ldloca inner, ...) at enclosingPos + /// ... + /// call Deconstruct(ldloc(a) inner, ...) at pos + /// + /// stloc temp(ldobj(ldflda ItemN(ldloc(a) outer))) at enclosingPos + /// ... + /// stloc x([conv](ldobj(ldflda ItemK(ldloc(a) temp)))) at pos + /// + /// Both shapes are decided by the same dry run of the enclosing match: only a match that + /// reaches beyond pos absorbs the statement there. A barrier statement between the two + /// positions, an element with uses the nesting cannot consume, or a conversion or + /// assignment the enclosing pattern does not account for makes the dry run stop short, + /// and the deconstruction at pos is then still transformed on its own. What the dry run + /// cannot promise is that the enclosing attempt still matches once the walk reaches it: + /// the positions in between are visited first and may rewrite the block. The back-to-front + /// walk gives this position no second chance, but losing the match there only costs + /// sugar, never correctness. + /// + bool IsConsumableByEnclosingDeconstruction(Block block, int pos) { - int startPos = pos; - Action? delayedActions = null; - if (MatchDeconstruction(block.Instructions[pos], out IMethod? deconstructMethod, - out ILInstruction? rootTestedOperand)) - { - pos++; - } - if (!MatchConversions(block, ref pos, out var conversions, out var conversionStLocs, ref delayedActions)) + if (!TryFindEnclosingDeconstructionCall(block, pos, out int enclosingPos)) return false; + // The dry run leaves the matcher state behind, which is safe because it runs before + // the attempt at this position, and both that attempt and Run reset it. It does not + // modify the block: all rewrites are delayed actions. + return MatchDeconstructionSequence(block, enclosingPos, out int endPos, out _, out _, out _, out _) + && endPos > pos; + } - if (!MatchAssignments(block, ref pos, conversions, conversionStLocs, ref delayedActions, - allowUnrelatedAssignments: deconstructMethod != null)) + /// + /// call Deconstruct(..., ldloca v, ...) at enclosingPos + /// [stloc copy(ldloc v)] defensive copy of a struct element + /// ... + /// call Deconstruct(ldloc(a) v|copy, ...) at pos + /// + static bool TryFindEnclosingDeconstructionCall(Block block, int pos, out int enclosingPos) + { + enclosingPos = -1; + if (!(block.Instructions[pos] is CallInstruction call)) return false; - // first tuple element may not be discarded, - // otherwise we would run this transform on a suffix of the actual pattern. - if (deconstructionResults[0] == null) + if (!MatchInstruction.IsDeconstructMethod(call.Method) || call.Arguments.Count == 0) return false; - context.Step("Deconstruction", block.Instructions[startPos]); - DeconstructInstruction replacement = new DeconstructInstruction(); - IType deconstructedType; - if (deconstructMethod == null) + var target = call.Arguments[0]; + if (!MatchLdLocOrLdLoca(target, out var v)) + return false; + // look through the defensive copy of a struct element + if (v.StoreInstructions is [StLoc copy] && copy.Value.MatchLdLoc(out var copySource)) { - deconstructedType = this.tupleType!; - rootTestedOperand = new LdLoc(this.tupleVariable!); + v = copySource; } - else + // StoreCount also counts the initial value of parameters, on purpose + if (v.StoreCount != 0) + return false; + if (!(v.AddressInstructions is [{ Parent: CallInstruction enclosingCall } addressLoad] + && addressLoad.ChildIndex > 0 + && MatchInstruction.IsDeconstructMethod(enclosingCall.Method))) { - if (deconstructMethod.IsStatic) - { - deconstructedType = deconstructMethod.Parameters[0].Type; - } - else - { - deconstructedType = deconstructMethod.DeclaringType; - } + return false; } - var rootTempVariable = context.Function.RegisterVariable(VariableKind.PatternLocal, deconstructedType); - replacement.Pattern = new MatchInstruction(rootTempVariable, deconstructMethod, rootTestedOperand!) { - IsDeconstructCall = deconstructMethod != null, - IsDeconstructTuple = this.tupleType != null - }; - int index = 0; - foreach (ILVariable? v in deconstructionResults) + if (enclosingCall.Parent != block) + return false; + enclosingPos = enclosingCall.ChildIndex; + return enclosingPos >= 0 && enclosingPos < pos; + } + + /// + /// A matched Deconstruct call: one node of the (possibly nested) deconstruction pattern. + /// + sealed class DeconstructionCall + { + public IMethod Method = null!; + /// Pattern variable of this match node; null for the root (which gets a fresh temp). + public ILVariable? Receiver; + /// The out-argument variable per element. + public ILVariable[] Results = null!; + /// Nested deconstruction per element; null = leaf element. + public DeconstructionCall?[] NestedCalls = null!; + } + + /// + /// call Deconstruct(target, ldloca x, ldloca inner) the root call, at pos + /// [nested Deconstruct calls, see MatchNestedDeconstructions] + /// On success, the leaf out-variables carry flat indices in depth-first order: this is + /// the order in which StatementBuilder/ExpressionBuilder pair pattern variables with + /// assignments, so the index checks in MatchConversions/MatchAssignments work unchanged + /// for nested patterns. + /// + void MatchDeconstruction(Block block, ref int pos, out DeconstructionCall? rootCall, + out ILInstruction? testedOperand) + { + rootCall = MatchDeconstructionCall(block.Instructions[pos], out testedOperand); + if (rootCall == null) + return; + pos++; + MatchNestedDeconstructions(block, ref pos, rootCall); + // Assign flat indices to the leaves in depth-first order: this is the order in which + // StatementBuilder/ExpressionBuilder pair pattern variables with assignments, so the + // index checks in MatchConversions/MatchAssignments work unchanged for nested patterns. + var leaves = new List(); + CollectLeaves(rootCall, leaves); + deconstructionResults = leaves.ToArray(); + for (int i = 0; i < deconstructionResults.Length; i++) { - var result = v; - if (result == null) - { - var freshVar = new ILVariable(VariableKind.PatternLocal, this.tupleType!.ElementTypes[index]) { Name = "E_" + index }; - context.Function.Variables.Add(freshVar); - result = freshVar; - } - else - { - result.Kind = VariableKind.PatternLocal; - } - replacement.Pattern.SubPatterns.Add( - new MatchInstruction( - result, - new DeconstructResultInstruction(index, result.StackType, new LdLoc(rootTempVariable)) - ) - ); - index++; + deconstructionResultsLookup.Add(deconstructionResults[i]!, i); } - replacement.Conversions = new Block(BlockKind.DeconstructionConversions); - foreach (var convInst in conversionStLocs) + + static void CollectLeaves(DeconstructionCall call, List leaves) { - replacement.Conversions.Instructions.Add(convInst); + for (int i = 0; i < call.Results.Length; i++) + { + if (call.NestedCalls[i] is DeconstructionCall nested) + CollectLeaves(nested, leaves); + else + leaves.Add(call.Results[i]); + } } - replacement.Assignments = new Block(BlockKind.DeconstructionAssignments); - delayedActions?.Invoke(replacement); - block.Instructions[startPos] = replacement; - block.Instructions.RemoveRange(startPos + 1, pos - startPos - 1); - context.EndStep(replacement); - return true; } - bool MatchDeconstruction(ILInstruction inst, [NotNullWhen(true)] out IMethod? deconstructMethod, - [NotNullWhen(true)] out ILInstruction? testedOperand) + /// + /// call(virt) Deconstruct(target, ldloca out0, ldloca out1, ...) + /// where every out-argument is a single-use temporary. + /// + DeconstructionCall? MatchDeconstructionCall(ILInstruction inst, out ILInstruction? testedOperand) { testedOperand = null; - deconstructMethod = null; - deconstructionResults = null!; if (!(inst is CallInstruction call)) - return false; + return null; if (!MatchInstruction.IsDeconstructMethod(call.Method)) - return false; + return null; if (call.Method.IsStatic || call.Method.DeclaringType.IsReferenceType == false) { if (!(call is Call)) - return false; + return null; } else { if (!(call is CallVirt)) - return false; + return null; } if (call.Arguments.Count < 3) - return false; - deconstructionResults = new ILVariable[call.Arguments.Count - 1]; - for (int i = 0; i < deconstructionResults.Length; i++) + return null; + var results = new ILVariable[call.Arguments.Count - 1]; + for (int i = 0; i < results.Length; i++) { if (!call.Arguments[i + 1].MatchLdLoca(out var v)) - return false; + return null; // TODO v.LoadCount may be 2 if the deconstruction is assigned to a tuple variable // or 0? because of discards if (!(v.StoreCount == 0 && v.AddressCount == 1 && v.LoadCount <= 1)) - return false; - deconstructionResultsLookup.Add(v, i); - deconstructionResults[i] = v; + return null; + results[i] = v; } testedOperand = call.Arguments[0]; - deconstructMethod = call.Method; - return true; + return new DeconstructionCall { + Method = call.Method, + Results = results, + NestedCalls = new DeconstructionCall[results.Length] + }; + } + + /// + /// Per element of the parent call, in order: + /// [stloc copy(ldloc result)] defensive copy for a struct element + /// call Deconstruct(ldloc(a) result|copy, ldloca ...) recursing into its elements + /// C# evaluates nested Deconstruct calls left-to-right, directly after the parent call, + /// before any conversions or assignments: the elements are visited depth-first, and the + /// stack of pending elements takes the place of recursing into a matched nested call. + /// + void MatchNestedDeconstructions(Block block, ref int pos, DeconstructionCall rootCall) + { + var pendingElements = new Stack<(DeconstructionCall Call, int ElementIndex)>(); + pendingElements.Push((rootCall, 0)); + while (pendingElements.Count > 0) + { + var (parent, i) = pendingElements.Pop(); + if (i + 1 < parent.Results.Length) + pendingElements.Push((parent, i + 1)); + ILVariable result = parent.Results[i]; + int savedPos = pos; + ILVariable receiver = result; + var inst = block.Instructions.ElementAtOrDefault(pos); + if (inst != null && inst.MatchStLoc(out var copy, out var copiedValue) + && copiedValue.MatchLdLoc(result) + && copy.StoreCount == 1 + && copy.LoadCount + copy.AddressCount == 1) + { + receiver = copy; + pos++; + inst = block.Instructions.ElementAtOrDefault(pos); + } + var nested = inst == null ? null : MatchDeconstructionCall(inst, out _); + if (nested == null || !IsReceiverReference(((CallInstruction)inst!).Arguments[0], receiver)) + { + pos = savedPos; + continue; + } + if (receiver != result && result.LoadCount != 1) + { + // the copy must be the element's only use + pos = savedPos; + continue; + } + pos++; + nested.Receiver = receiver; + parent.NestedCalls[i] = nested; + // its elements are evaluated before the parent's remaining ones + pendingElements.Push((nested, 0)); + } + + static bool IsReceiverReference(ILInstruction target, ILVariable receiver) + { + return MatchLdLocOrLdLoca(target, out var v) && v == receiver; + } + } + + struct ConversionInfo + { + public IType? inputType; + public Conv? conv; } + /// + /// stloc conv0(conv(FindIndex-resolvable value)) + /// stloc conv1(conv(...)) + /// ... + /// The run of single-use conversion temporaries following the deconstruction, in flat + /// leaf index order. + /// bool MatchConversions(Block block, ref int pos, out Dictionary conversions, out List conversionStLocs, @@ -334,6 +530,9 @@ bool MatchConversions(Block block, ref int pos, return true; } + /// + /// stloc output(conv(input)) + /// bool MatchConversion(ILInstruction? inst, [NotNullWhen(true)] out ILInstruction? inputInstruction, [NotNullWhen(true)] out ILVariable? outputVariable, out ConversionInfo info) { @@ -354,12 +553,21 @@ bool MatchConversion(ILInstruction? inst, [NotNullWhen(true)] out ILInstruction? return true; } + /// + /// assignment(FindIndex-resolvable value) see MatchAssignment + /// ... + /// The run of assignments following the conversions, in flat leaf index order. + /// Single-use elements without an assignment are forwarded through a fresh variable + /// assigned inside the deconstruction. + /// bool MatchAssignments(Block block, ref int pos, Dictionary conversions, List conversionStLocs, ref Action? delayedActions, - bool allowUnrelatedAssignments) + bool allowUnrelatedAssignments, + out bool anyAssignments) { + anyAssignments = false; int previousIndex = -1; int conversionStLocIndex = 0; int startPos = pos; @@ -450,7 +658,8 @@ bool MatchAssignments(Block block, ref int pos, } } - return startPos != pos; + anyAssignments = startPos != pos; + return true; int GetAssignmentIndex(ILInstruction inst) { @@ -490,6 +699,12 @@ void AddMissingAssignmentsForConversions(int index, ref Action + /// stloc v(value) | stobj(target, value) | call set_Property(target, value) + /// or the result-used form + /// stloc s(Block CallInlineAssign { call set_Property(target, stloc tmp(value)); final: ldloc tmp }) + /// where the setter call is moved into the assignments block. + /// bool MatchAssignment(ILInstruction? inst, [NotNullWhen(true)] out IType? targetType, [NotNullWhen(true)] out ILInstruction? valueInst, [NotNullWhen(true)] out Action? addAssignment) { targetType = null; @@ -525,6 +740,50 @@ bool MatchAssignment(ILInstruction? inst, [NotNullWhen(true)] out IType? targetT } } + /// + /// ldloc result a registered result or conversion output + /// ldobj(ldflda ItemN(ldloc(a) v)) an element read of the tuple + /// Resolves the value of a conversion or assignment to its element index. + /// Returns -1 on failure. + /// + int FindIndex(ILInstruction inst, out Action? delayedActions) + { + delayedActions = null; + if (inst.MatchLdLoc(out var v)) + { + if (!deconstructionResultsLookup.TryGetValue(v, out int index)) + return -1; + return index; + } + if (!MatchTupleElementRead(inst, out var container, out var containerType, out int elementIndex)) + return -1; + var normalizedType = TupleType.FromUnderlyingType(context.TypeSystem, containerType); + if (this.tupleVariable == null) + { + this.tupleVariable = container; + this.tupleType = (TupleType)normalizedType; + this.deconstructionResults = new ILVariable[this.tupleType.Cardinality]; + } + if (this.tupleType!.Cardinality < 2) + return -1; + if (container != tupleVariable || !this.tupleType.Equals(normalizedType)) + return -1; + if (this.deconstructionResults[elementIndex] == null) + { + var freshVar = new ILVariable(VariableKind.StackSlot, this.tupleType.ElementTypes[elementIndex]) { Name = "E_" + elementIndex }; + delayedActions += _ => context.Function.Variables.Add(freshVar); + this.deconstructionResults[elementIndex] = freshVar; + } + delayedActions += _ => { + inst.ReplaceWith(new LdLoc(this.deconstructionResults[elementIndex]!)); + }; + return elementIndex; + } + + /// + /// Gets whether the matched conv instruction (or its absence) is the lowering of the + /// implicit conversion from the input type to the assignment's target type. + /// bool IsCompatibleImplicitConversion(IType targetType, ConversionInfo conversionInfo) { var c = CSharpConversions.Get(context.TypeSystem) @@ -555,5 +814,72 @@ bool IsCompatibleImplicitConversion(IType targetType, ConversionInfo conversionI } return false; } + + /// + /// Builds, recursing into nested calls: + /// match.deconstruct[Method] (matchVariable = testedOperand) { + /// match(result_i = deconstruct.result i(ldloc matchVariable)), + /// match.deconstruct[...] (receiver_j = deconstruct.result j(ldloc matchVariable)) { ... } + /// } + /// + MatchInstruction BuildPatternMatch(DeconstructionCall call, ILVariable matchVariable, ILInstruction testedOperand) + { + matchVariable.Kind = VariableKind.PatternLocal; + var match = new MatchInstruction(matchVariable, call.Method, testedOperand) { + IsDeconstructCall = true + }; + for (int i = 0; i < call.Results.Length; i++) + { + var nested = call.NestedCalls[i]; + if (nested != null) + { + var receiver = nested.Receiver!; + match.SubPatterns.Add(BuildPatternMatch(nested, receiver, + new DeconstructResultInstruction(i, receiver.StackType, new LdLoc(matchVariable)))); + } + else + { + var result = call.Results[i]; + result.Kind = VariableKind.PatternLocal; + match.SubPatterns.Add( + new MatchInstruction( + result, + new DeconstructResultInstruction(i, result.StackType, new LdLoc(matchVariable)) + ) + ); + } + } + return match; + } + + /// + /// ldobj(ldflda ItemN(ldloc(a) container)) + /// The returned index is zero-based; Rest chains of long tuples are flattened. + /// Non-escaping element reads may have been rewritten from ldloca to ldloc, + /// so both load kinds are accepted. + /// + static bool MatchTupleElementRead(ILInstruction inst, [NotNullWhen(true)] out ILVariable? container, [NotNullWhen(true)] out IType? containerType, out int index) + { + container = null; + containerType = null; + index = -1; + if (!(inst is LdObj ldobj && ldobj.Target is LdFlda ldflda)) + return false; + if (ldobj.UnalignedPrefix != 0 || ldobj.IsVolatile) + return false; + if (!TupleTransform.MatchTupleFieldAccess(ldflda, out containerType, out var target, out int position)) + return false; + // Item fields are one-based, we use zero-based indexing. + index = position - 1; + return MatchLdLocOrLdLoca(target, out container); + } + + /// + /// ldloc variable | ldloca variable + /// + static bool MatchLdLocOrLdLoca(ILInstruction inst, [NotNullWhen(true)] out ILVariable? variable) + { + return inst.MatchLdLoc(out variable) || inst.MatchLdLoca(out variable); + } } } From 84ed5e2ec88861295f4babbf389c8afffc7b7db6 Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Thu, 6 Aug 2026 17:16:34 +0200 Subject: [PATCH 07/11] Keep the Deconstruct call explicit when the element type hides it A nested designation rebinds Deconstruct on the element's static type when the output is recompiled, while the explicit call it replaces is bound at the call site. Where a derived element type declares a Deconstruct of the same arity as the called method, and the source deconstructs through a base-typed view, the two bindings differ, so the sugared output calls the wrong method - a divergence the runtime fixture demonstrates on optimized builds, where copy propagation elides the view. Nesting is therefore only applied when the method the call binds to is the one a designation would rebind to; otherwise the call stays explicit, where its receiver cast preserves the binding. Assisted-by: Claude:claude-opus-5:Claude Code --- .../Correctness/DeconstructionTests.cs | 46 +++++++++++++++++++ .../TestCases/Pretty/DeconstructionTests.cs | 42 +++++++++++++++++ .../IL/Transforms/DeconstructionTransform.cs | 35 ++++++++++++++ 3 files changed, 123 insertions(+) diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Correctness/DeconstructionTests.cs b/ICSharpCode.Decompiler.Tests/TestCases/Correctness/DeconstructionTests.cs index dc7cdb091e..4411b6b5e7 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Correctness/DeconstructionTests.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Correctness/DeconstructionTests.cs @@ -168,6 +168,7 @@ public void Test() NestedDeconstruction_Conversions_AfterAllDeconstructCalls(); NestedDeconstruction_TypedDeclaration_Conversions(new NestedOuter { Value = 5 }); NestedDeconstruction_DiscardWithSideEffectTargets(); + NestedDeconstruction_HiddenDeconstructMethod(default(HidingOuter)); NestedDeconstruction_SystemTupleSource(Tuple.Create(8, new NestedInner { Value = 4 })); NestedDeconstruction_CheckedConversions(new NestedOuter { Value = 9 }); NestedDeconstruction_GenericConstraintSource(new ConstrainedSource { Value = 11 }); @@ -303,6 +304,51 @@ public void NestedDeconstruction_DiscardWithSideEffectTargets() (Get(0).IntProperty, (_, Get(1).My)) = new NestedOuter { Value = 31 }; } + public class HidingBase + { + public int Value; + + public void Deconstruct(out string a, out double b) + { + Console.WriteLine("HidingBase.Deconstruct"); + a = "base" + Value; + b = 0.5; + } + } + + public class HidingDerived : HidingBase + { + public new void Deconstruct(out string a, out double b) + { + Console.WriteLine("HidingDerived.Deconstruct"); + a = "derived"; + b = 99.5; + } + } + + public struct HidingOuter + { + public void Deconstruct(out int x, out HidingDerived d) + { + Console.WriteLine("HidingOuter.Deconstruct"); + x = 1; + d = new HidingDerived { Value = 5 }; + } + } + + // The base-typed view forces the call to bind to HidingBase.Deconstruct; a nested + // designation cannot express that, because it rebinds on the element's static type, + // where the hiding method wins. + public void NestedDeconstruction_HiddenDeconstructMethod(HidingOuter o) + { + Console.WriteLine("NestedDeconstruction_HiddenDeconstructMethod:"); + var (_, d) = o; + HidingBase b = d; + var (a, c) = b; + Console.WriteLine(a); + Console.WriteLine(c); + } + public int Side() { Console.WriteLine("Side()"); diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/DeconstructionTests.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/DeconstructionTests.cs index e873d6968b..eaea65d219 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/DeconstructionTests.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/DeconstructionTests.cs @@ -22,6 +22,14 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty { + public class DeconstructionBase + { + } + + public class DeconstructionDerived : DeconstructionBase + { + } + public static class DeconstructionExt { public static void Deconstruct(this KeyValuePair pair, out K key, out V value) @@ -35,6 +43,27 @@ public static void Deconstruct(this Tuple tuple, out T1 item1, o item1 = tuple.Item1; item2 = tuple.Item2; } + + public static void Deconstruct(this DeconstructionBase b, out int a, out int c) + { + a = 1; + c = 2; + } + + public static void Deconstruct(this DeconstructionDerived d, out int a, out int c) + { + a = 3; + c = 4; + } + } + + public class DeconstructionOuter + { + public void Deconstruct(out int x, out DeconstructionDerived d) + { + x = 1; + d = new DeconstructionDerived(); + } } internal class DeconstructionTests @@ -529,6 +558,19 @@ public unsafe void Pointer_NoConversion_Tuple(int* p) // The store opcode is sign-agnostic - stind.i4 reports int for a uint target and // stind.i1 reports sbyte for a byte one - so the element type of the target cannot // be taken from it: doing so refuses every one of these deconstructions. + // The IL calls the extension declared on the base type, forced by the cast. Folding + // this into a nested designation would rebind Deconstruct on the element's static + // type, where the extension declared on the derived type wins and returns different + // values, so the call has to stay explicit. + public void Nested_CompetingExtensionDeconstruct(DeconstructionOuter o) + { + o.Deconstruct(out var x, out var d); + ((DeconstructionBase)d).Deconstruct(out int a, out int c); + Console.WriteLine(x); + Console.WriteLine(a); + Console.WriteLine(c); + } + public unsafe void Pointer_NoConversion_Tuple_UInt(uint* p) { int value; diff --git a/ICSharpCode.Decompiler/IL/Transforms/DeconstructionTransform.cs b/ICSharpCode.Decompiler/IL/Transforms/DeconstructionTransform.cs index cbd566d865..ac287d673a 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/DeconstructionTransform.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/DeconstructionTransform.cs @@ -477,6 +477,14 @@ void MatchNestedDeconstructions(Block block, ref int pos, DeconstructionCall roo pos = savedPos; continue; } + if (!BindsOnElementType(nested.Method, result.Type)) + { + // A nested designation rebinds Deconstruct on the element's static type + // when recompiled; if that picks a different method (member hiding), the + // call must stay explicit, where a cast can preserve the binding. + pos = savedPos; + continue; + } pos++; nested.Receiver = receiver; parent.NestedCalls[i] = nested; @@ -488,6 +496,33 @@ static bool IsReceiverReference(ILInstruction target, ILVariable receiver) { return MatchLdLocOrLdLoca(target, out var v) && v == receiver; } + + static bool BindsOnElementType(IMethod method, IType elementType) + { + int outParamCount = method.Parameters.Count - (method.IsStatic ? 1 : 0); + IType type = elementType; + while (type != null) + { + if (!method.IsStatic && NormalizeTypeVisitor.TypeErasure.EquivalentTypes(type, method.DeclaringType)) + return true; + if (type.GetMethods(m => m.Name == "Deconstruct", GetMemberOptions.IgnoreInheritedMembers) + .Any(m => !m.IsStatic && m.Parameters.Count == outParamCount)) + { + // An instance Deconstruct of the same arity is declared on a type more + // derived than the called method's declaring type: it hides the called + // method (and wins over a called extension method). + return false; + } + type = type.DirectBaseTypes.FirstOrDefault(t => t.Kind == TypeKind.Class)!; + } + // The chain ended without seeing the declaring type, so an instance method's + // binding cannot be verified. An extension method is reached by its receiver + // type, and one declared on a more derived type wins over it; which extensions + // are in scope where the output is compiled is not known here, so the binding + // is only certain when the element type is the receiver type itself. + return method.IsStatic + && NormalizeTypeVisitor.TypeErasure.EquivalentTypes(elementType, method.Parameters[0].Type); + } } struct ConversionInfo From 17109071a31e8e830e5d793796566eed0b992f2d Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Thu, 6 Aug 2026 17:17:05 +0200 Subject: [PATCH 08/11] Do not absorb tuple element reads into a call-rooted pattern Element index resolution serves both pattern roots: a registered result of a Deconstruct call, or an element read of a tuple, which it discovers on first sight and then owns. In an attempt rooted in a Deconstruct call the tuple branch must not engage - it overwrites the call's result bookkeeping and rewires the element read to a fresh variable that the pattern never defines. The shape that reaches it is a tuple whose element is custom-deconstructed with discarded leaves, followed by an unrelated assignment: the tuple-rooted attempt fails, the call-rooted one runs at the element's position, and, now that an unrelated assignment ends a call pattern instead of rejecting it, the mixed match is no longer rejected on the way out. Assisted-by: Claude:claude-opus-5:Claude Code --- .../TestCases/Correctness/DeconstructionTests.cs | 12 ++++++++++++ .../IL/Transforms/DeconstructionTransform.cs | 10 ++++++++++ 2 files changed, 22 insertions(+) diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Correctness/DeconstructionTests.cs b/ICSharpCode.Decompiler.Tests/TestCases/Correctness/DeconstructionTests.cs index 4411b6b5e7..d35a1a440a 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Correctness/DeconstructionTests.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Correctness/DeconstructionTests.cs @@ -169,6 +169,7 @@ public void Test() NestedDeconstruction_TypedDeclaration_Conversions(new NestedOuter { Value = 5 }); NestedDeconstruction_DiscardWithSideEffectTargets(); NestedDeconstruction_HiddenDeconstructMethod(default(HidingOuter)); + NestedDeconstruction_TupleWithCustomElement((7, new NestedInner { Value = 3 })); NestedDeconstruction_SystemTupleSource(Tuple.Create(8, new NestedInner { Value = 4 })); NestedDeconstruction_CheckedConversions(new NestedOuter { Value = 9 }); NestedDeconstruction_GenericConstraintSource(new ConstrainedSource { Value = 11 }); @@ -355,6 +356,17 @@ public int Side() return 5; } + // A tuple deconstruction whose element is custom-deconstructed, followed by an + // unrelated assignment: the tuple part must not be consumed into a pattern rooted + // in the element's Deconstruct call. + public void NestedDeconstruction_TupleWithCustomElement((int, NestedInner) tup) + { + Console.WriteLine("NestedDeconstruction_TupleWithCustomElement:"); + var (x, (_, _)) = tup; + int z = Side(); + Console.WriteLine(x * x + z); + } + public class NestedClassInner { public int Value; diff --git a/ICSharpCode.Decompiler/IL/Transforms/DeconstructionTransform.cs b/ICSharpCode.Decompiler/IL/Transforms/DeconstructionTransform.cs index ac287d673a..878415d0fb 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/DeconstructionTransform.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/DeconstructionTransform.cs @@ -79,6 +79,7 @@ class DeconstructionTransform : IStatementTransform ILVariable?[] deconstructionResults = null!; ILVariable? tupleVariable; TupleType? tupleType; + bool rootedInDeconstructCall; void IStatementTransform.Run(Block block, int pos, StatementTransformContext context) { @@ -108,6 +109,7 @@ private void Reset() this.tupleVariable = null; this.tupleType = null; this.deconstructionResults = null!; + this.rootedInDeconstructCall = false; } /// @@ -368,6 +370,7 @@ void MatchDeconstruction(Block block, ref int pos, out DeconstructionCall? rootC rootCall = MatchDeconstructionCall(block.Instructions[pos], out testedOperand); if (rootCall == null) return; + rootedInDeconstructCall = true; pos++; MatchNestedDeconstructions(block, ref pos, rootCall); // Assign flat indices to the leaves in depth-first order: this is the order in which @@ -790,6 +793,13 @@ int FindIndex(ILInstruction inst, out Action? delayedAct return -1; return index; } + if (rootedInDeconstructCall) + { + // A pattern rooted in a Deconstruct call must not absorb tuple element + // accesses: discovering the tuple here would overwrite the call's result + // bookkeeping and destroy the rewritten tuple access on failure. + return -1; + } if (!MatchTupleElementRead(inst, out var container, out var containerType, out int elementIndex)) return -1; var normalizedType = TupleType.FromUnderlyingType(context.TypeSystem, containerType); From d6c82c5edf94fa0553d7920c3a920bb4cec85de2 Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Thu, 6 Aug 2026 17:17:38 +0200 Subject: [PATCH 09/11] Reconstruct nested tuple designations A nested designation over tuples, var (x, (a, b)) = t;, is lowered to one temporary per nested designation - parents before children - followed by the element reads in depth-first leaf order, and decompiled as a flat deconstruction plus separate element statements. The temporaries are now consumed into a tree of tuple nodes before the conversions and assignments are matched, and the leaves get the same flat depth-first indices the Deconstruct-call chain hands out, so conversion and assignment matching runs unchanged. Two properties of the lowered IL shape the matcher to it: earlier transforms rewrite non-escaping element reads from ldloca to ldloc, and the temporaries are stack slots whose type is imprecise, so the container's element type is authoritative and the match variable is retyped to keep the tuple pattern's invariant. An element that escapes the deconstruction - used after the statement, so the pattern cannot consume all its reads - demotes back to a designator leaf and the match is retried, which restores the flat deconstruction the escaping read needs. The guard against consuming a pattern piecemeal extends to the new shape: an element read whose container is stored by an earlier element read defers to the match starting at that store. Assisted-by: Claude:claude-opus-5:Claude Code --- .../Correctness/DeconstructionTests.cs | 45 +++ .../TestCases/Pretty/DeconstructionTests.cs | 58 ++- .../IL/Transforms/DeconstructionTransform.cs | 377 ++++++++++++++---- 3 files changed, 402 insertions(+), 78 deletions(-) diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Correctness/DeconstructionTests.cs b/ICSharpCode.Decompiler.Tests/TestCases/Correctness/DeconstructionTests.cs index d35a1a440a..ca599ab5c6 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Correctness/DeconstructionTests.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Correctness/DeconstructionTests.cs @@ -182,6 +182,51 @@ public void Test() NestedDeconstruction_ForEachDictionary_Conversions(new Dictionary { { "k1", new NestedInner { Value = 19 } } }); + NestedTupleDeconstruction_Values((20, (21, 22))); + NestedTupleDeconstruction_Depth3((23, (24, (25, 26)))); + NestedTupleDeconstruction_Conversions((27, (28, 29))); + NestedTupleDeconstruction_ElementUsedOutside((30, (31, 32))); + NestedTupleDeconstruction_ForEach(new List<(int, (int, int))> { + (33, (34, 35)), + (36, (37, 38)) + }); + } + + public void NestedTupleDeconstruction_Values((int, (int, int)) t) + { + Console.WriteLine("NestedTupleDeconstruction_Values:"); + var (x, (a, b)) = t; + Console.WriteLine(x + " " + a + " " + b); + } + + public void NestedTupleDeconstruction_Depth3((int, (int, (int, int))) t) + { + Console.WriteLine("NestedTupleDeconstruction_Depth3:"); + var (x, (a, (b, c))) = t; + Console.WriteLine(x + " " + a + " " + b + " " + c); + } + + public void NestedTupleDeconstruction_Conversions((int, (int, int)) t) + { + Console.WriteLine("NestedTupleDeconstruction_Conversions:"); + (long x, (long a, long b)) = t; + Console.WriteLine(x + " " + a + " " + b); + } + + public void NestedTupleDeconstruction_ElementUsedOutside((int, (int, int)) t) + { + Console.WriteLine("NestedTupleDeconstruction_ElementUsedOutside:"); + var (x, inner) = t; + Console.WriteLine(x + " " + inner.Item1 + " " + inner.Item2); + } + + public void NestedTupleDeconstruction_ForEach(List<(int, (int, int))> list) + { + Console.WriteLine("NestedTupleDeconstruction_ForEach:"); + foreach (var (x, (a, b)) in list) + { + Console.WriteLine(x + " " + a + " " + b); + } } public class ConstrainedSource diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/DeconstructionTests.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/DeconstructionTests.cs index eaea65d219..95cf793e8a 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/DeconstructionTests.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/DeconstructionTests.cs @@ -443,26 +443,64 @@ public void LocalVariable_Nested_SystemTupleSource() Console.WriteLine(value); } - // Nested deconstruction of a tuple element (ldfld chains, no Deconstruct call) - // is not re-sugared: var (value, (value2, value3)) = GetTuple(); public void LocalVariable_Nested_TupleInner() { - (int, (int, int)) tuple = GetTuple(); - (int, int) item = tuple.Item2; - var (value, _) = tuple; - var (value2, value3) = item; + var (value, (value2, value3)) = GetTuple(); + Console.WriteLine(value); + Console.WriteLine(value2); + Console.WriteLine(value3); + } + + public void LocalVariable_Nested_TupleInner_Depth3() + { + var (value, (value2, (value3, value4))) = GetTuple(); + Console.WriteLine(value); + Console.WriteLine(value2); + Console.WriteLine(value3); + Console.WriteLine(value4); + } + + public void LocalVariable_Nested_TupleInner_BothElements() + { + var ((value, value2), (value3, value4)) = GetTuple<(int, int), (int, int)>(); Console.WriteLine(value); Console.WriteLine(value2); Console.WriteLine(value3); + Console.WriteLine(value4); + } + + public void LocalVariable_Nested_TupleInner_Conversions() + { + int value; + long value2; + long value3; + (value, (value2, value3)) = GetTuple(); + Console.WriteLine(value); + Console.WriteLine(value2); + Console.WriteLine(value3); + } + + // The element variable escapes the deconstruction, so it must stay a designator + // leaf instead of becoming a nested designation. + public void LocalVariable_TupleInner_ElementUsedOutside() + { +#if OPT + (int, (int, int)) tuple = GetTuple(); + int item = tuple.Item1; + (int, int) item2 = tuple.Item2; + Console.WriteLine(item); + Console.WriteLine(item2.Item1); +#else + var (value, tuple2) = GetTuple(); + Console.WriteLine(value); + Console.WriteLine(tuple2.Item1); +#endif } public void ForEach_Nested_TupleInner() { - foreach (var item2 in GetList<(int, (int, int))>()) + foreach (var (value, (value2, value3)) in GetList<(int, (int, int))>()) { - (int, int) item = item2.Item2; - var (value, _) = item2; - var (value2, value3) = item; Console.WriteLine(value); Console.WriteLine(value2); Console.WriteLine(value3); diff --git a/ICSharpCode.Decompiler/IL/Transforms/DeconstructionTransform.cs b/ICSharpCode.Decompiler/IL/Transforms/DeconstructionTransform.cs index 878415d0fb..d4d2cc0611 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/DeconstructionTransform.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/DeconstructionTransform.cs @@ -76,9 +76,9 @@ class DeconstructionTransform : IStatementTransform { StatementTransformContext context = null!; readonly Dictionary deconstructionResultsLookup = new Dictionary(); + readonly Dictionary tupleNodes = new Dictionary(); ILVariable?[] deconstructionResults = null!; - ILVariable? tupleVariable; - TupleType? tupleType; + TupleNode? tupleRoot; bool rootedInDeconstructCall; void IStatementTransform.Run(Block block, int pos, StatementTransformContext context) @@ -106,8 +106,8 @@ void IStatementTransform.Run(Block block, int pos, StatementTransformContext con private void Reset() { this.deconstructionResultsLookup.Clear(); - this.tupleVariable = null; - this.tupleType = null; + this.tupleNodes.Clear(); + this.tupleRoot = null; this.deconstructionResults = null!; this.rootedInDeconstructCall = false; } @@ -140,8 +140,8 @@ bool TransformDeconstruction(Block block, int pos) IType deconstructedType; if (deconstructMethod == null) { - deconstructedType = this.tupleType!; - rootTestedOperand = new LdLoc(this.tupleVariable!); + deconstructedType = tupleRoot!.Type; + rootTestedOperand = new LdLoc(tupleRoot.Variable); } else { @@ -161,29 +161,7 @@ bool TransformDeconstruction(Block block, int pos) } else { - replacement.Pattern = new MatchInstruction(rootTempVariable, method: null, rootTestedOperand!) { - IsDeconstructTuple = true - }; - for (int i = 0; i < deconstructionResults.Length; i++) - { - var result = deconstructionResults[i]; - if (result == null) - { - var freshVar = new ILVariable(VariableKind.PatternLocal, this.tupleType!.ElementTypes[i]) { Name = "E_" + i }; - context.Function.Variables.Add(freshVar); - result = freshVar; - } - else - { - result.Kind = VariableKind.PatternLocal; - } - replacement.Pattern.SubPatterns.Add( - new MatchInstruction( - result, - new DeconstructResultInstruction(i, result.StackType, new LdLoc(rootTempVariable)) - ) - ); - } + replacement.Pattern = BuildTuplePatternMatch(tupleRoot!, rootTempVariable, rootTestedOperand!); } replacement.Conversions = new Block(BlockKind.DeconstructionConversions); foreach (var convInst in conversionStLocs) @@ -210,29 +188,61 @@ bool MatchDeconstructionSequence(Block block, int startPos, out int endPos, out DeconstructionCall? rootCall, out ILInstruction? rootTestedOperand, out List conversionStLocs, out Action? delayedActions) { - Reset(); - endPos = startPos; - int pos = startPos; - delayedActions = null; - MatchDeconstruction(block, ref pos, out rootCall, out rootTestedOperand); - if (!MatchConversions(block, ref pos, out var conversions, out conversionStLocs, ref delayedActions)) - return false; - if (!MatchAssignments(block, ref pos, conversions, conversionStLocs, ref delayedActions, - allowUnrelatedAssignments: rootCall != null, out bool anyAssignments)) + HashSet? doNotNest = null; + while (true) { - return false; + Reset(); + endPos = startPos; + int pos = startPos; + delayedActions = null; + MatchDeconstruction(block, ref pos, out rootCall, out rootTestedOperand); + if (rootCall == null) + MatchNestedTupleDesignations(block, ref pos, doNotNest); + if (!MatchConversions(block, ref pos, out var conversions, out conversionStLocs, ref delayedActions)) + return false; + if (!MatchAssignments(block, ref pos, conversions, conversionStLocs, ref delayedActions, + allowUnrelatedAssignments: rootCall != null, out bool anyAssignments)) + { + return false; + } + // Without any assignment the statement is a plain Deconstruct call, unless a nested + // deconstruction was consumed: then all leaves are single-use elements handled by + // the forwarding fixup in MatchAssignments. + if (!anyAssignments && !(rootCall != null && rootCall.NestedCalls.Any(c => c != null))) + return false; + // first tuple element may not be discarded, + // otherwise we would run this transform on a suffix of the actual pattern. + if (deconstructionResults[0] == null) + return false; + // A nested tuple designation only holds if the pattern consumed every read of + // its temporary; a remaining read means the value escapes the designation. + // Retry with the variable as a plain designator leaf, which restores the flat + // deconstruction the escaping read needs. + var escaped = EscapedTupleNodes(); + if (escaped == null) + { + endPos = pos; + return true; + } + doNotNest ??= new HashSet(); + doNotNest.UnionWith(escaped); + } + + List? EscapedTupleNodes() + { + List? escaped = null; + foreach (var node in tupleNodes.Values) + { + if (node == tupleRoot) + continue; + if (node.MatchedAccessCount != node.Variable.LoadCount + node.Variable.AddressCount) + { + escaped ??= new List(); + escaped.Add(node.Variable); + } + } + return escaped; } - // Without any assignment the statement is a plain Deconstruct call, unless a nested - // deconstruction was consumed: then all leaves are single-use elements handled by - // the forwarding fixup in MatchAssignments. - if (!anyAssignments && !(rootCall != null && rootCall.NestedCalls.Any(c => c != null))) - return false; - // first tuple element may not be discarded, - // otherwise we would run this transform on a suffix of the actual pattern. - if (deconstructionResults[0] == null) - return false; - endPos = pos; - return true; } /// @@ -297,8 +307,11 @@ bool InlineDeconstructionInitializer(Block block, int pos) /// bool IsConsumableByEnclosingDeconstruction(Block block, int pos) { - if (!TryFindEnclosingDeconstructionCall(block, pos, out int enclosingPos)) + if (!TryFindEnclosingDeconstructionCall(block, pos, out int enclosingPos) + && !TryFindEnclosingTupleDesignation(block, pos, out enclosingPos)) + { return false; + } // The dry run leaves the matcher state behind, which is safe because it runs before // the attempt at this position, and both that attempt and Run reset it. It does not // modify the block: all rewrites are delayed actions. @@ -342,6 +355,34 @@ static bool TryFindEnclosingDeconstructionCall(Block block, int pos, out int enc return enclosingPos >= 0 && enclosingPos < pos; } + /// + /// stloc temp(ldobj(ldflda ItemN(ldloc(a) outer))) at enclosingPos + /// ... + /// stloc x([conv](ldobj(ldflda ItemK(ldloc(a) temp)))) at pos + /// The statement at pos reads an element of a tuple stored by an earlier statement that + /// is itself an element read, i.e. a candidate nested designation temporary. The + /// enclosing pattern's matching starts at the first store of the run that store belongs + /// to, because the temporaries of a nested designation are stored back to back. + /// + static bool TryFindEnclosingTupleDesignation(Block block, int pos, out int enclosingPos) + { + enclosingPos = -1; + if (!block.Instructions[pos].MatchStLoc(out _, out var value)) + return false; + if (value is Conv conv) + value = conv.Argument; + if (!MatchTupleElementRead(value, out var container, out _, out _)) + return false; + if (!(container.StoreInstructions is [StLoc store]) || store.Parent != block) + return false; + if (!MatchTupleElementStore(store, out _, out _, out _, out _)) + return false; + enclosingPos = store.ChildIndex; + while (enclosingPos > 0 && MatchTupleElementStore(block.Instructions[enclosingPos - 1], out _, out _, out _, out _)) + enclosingPos--; + return enclosingPos < pos; + } + /// /// A matched Deconstruct call: one node of the (possibly nested) deconstruction pattern. /// @@ -528,6 +569,137 @@ static bool BindsOnElementType(IMethod method, IType elementType) } } + /// + /// A tuple variable being deconstructed: one node of the (possibly nested) designation. + /// Nested nodes are the temporaries a nested tuple designation is lowered to. + /// + sealed class TupleNode + { + public readonly ILVariable Variable; + public readonly TupleType Type; + /// Nested designation per element; null = leaf element. + public readonly TupleNode[] NestedElements; + /// Flat leaf index (depth-first) of each element. + public int[] ElementFlatIndex = null!; + /// Number of element reads of consumed by the pattern. + public int MatchedAccessCount; + + public TupleNode(ILVariable variable, TupleType type) + { + Variable = variable; + Type = type; + NestedElements = new TupleNode[type.Cardinality]; + } + } + + /// + /// stloc temp(ldobj(ldflda ItemN(ldloc(a) container))) one per nested designation + /// ... + /// The temporaries a nested tuple designation is lowered to: parents before children, + /// all evaluated before any conversions or assignments. The consumed variables form the + /// tuple node tree rooted at the outermost tuple. + /// + void MatchNestedTupleDesignations(Block block, ref int pos, HashSet? doNotNest) + { + while (MatchTupleElementStore(block.Instructions.ElementAtOrDefault(pos), + out var temp, out var container, out var containerType, out int index)) + { + if (doNotNest != null && doNotNest.Contains(temp)) + break; + if (!(temp.StoreCount == 1 && temp.LoadCount + temp.AddressCount >= 1)) + break; + // Every use of the temporary must itself be a tuple element read, + // 'ldobj(ldflda ItemK(...temp...))', so that the pattern can consume them all; + // reads it does not consume are rejected by the escape check afterwards. + if (!AllUsesAreTupleElementReads(temp)) + break; + var containerNode = ResolveTupleContainer(container, containerType); + if (containerNode == null) + break; + if (index >= containerNode.NestedElements.Length || containerNode.NestedElements[index] != null) + break; + // The container's element type is authoritative for the temporary's tuple type: + // a stack slot's own type can be imprecise. A temporary with a precise type must + // agree with the element type. + var elementType = containerNode.Type.ElementTypes[index]; + if (TupleType.GetTupleElementTypes(elementType).IsDefaultOrEmpty) + break; + var tempType = TupleType.FromUnderlyingType(context.TypeSystem, elementType); + if (tempType == null || tempType.Cardinality < 2) + break; + if (!TupleType.GetTupleElementTypes(temp.Type).IsDefaultOrEmpty + && !NormalizeTypeVisitor.TypeErasure.EquivalentTypes(elementType, temp.Type)) + { + break; + } + var node = new TupleNode(temp, tempType); + containerNode.NestedElements[index] = node; + // The temporary's store reads one element of the container. + containerNode.MatchedAccessCount++; + this.tupleNodes.Add(temp, node); + InitializeFlatLeafIndices(); + pos++; + } + + static bool AllUsesAreTupleElementReads(ILVariable temp) + { + foreach (var use in temp.AddressInstructions.Concat(temp.LoadInstructions)) + { + if (!(use.Parent is LdFlda elementAccess && elementAccess.Parent is LdObj)) + return false; + } + return true; + } + } + + /// + /// Resolves the container of a tuple element access against the tree of tuple nodes; + /// the first access establishes its container as the root. Returns null if the + /// container is not part of the tree or its type does not fit a deconstruction. + /// + TupleNode? ResolveTupleContainer(ILVariable container, IType containerType) + { + var normalizedType = TupleType.FromUnderlyingType(context.TypeSystem, containerType); + if (normalizedType == null || normalizedType.Cardinality < 2) + return null; + if (tupleRoot == null) + { + tupleRoot = new TupleNode(container, normalizedType); + tupleNodes.Add(container, tupleRoot); + InitializeFlatLeafIndices(); + } + if (!tupleNodes.TryGetValue(container, out var node)) + return null; + return node.Type.Equals(normalizedType) ? node : null; + } + + /// + /// Assigns depth-first flat leaf indices to every element of the tuple node tree and + /// allocates the flat results array. Depth-first order is the order in which the + /// consumers pair pattern variables with conversions and assignments. Called whenever + /// the tree grows; the results array is still empty then, because the tree is complete + /// before MatchConversions/MatchAssignments start populating it. + /// + void InitializeFlatLeafIndices() + { + int totalLeaves = AssignFlatIndices(tupleRoot!, 0); + this.deconstructionResults = new ILVariable[totalLeaves]; + + static int AssignFlatIndices(TupleNode node, int nextLeafIndex) + { + node.ElementFlatIndex = new int[node.Type.Cardinality]; + for (int i = 0; i < node.Type.Cardinality; i++) + { + node.ElementFlatIndex[i] = nextLeafIndex; + if (node.NestedElements[i] != null) + nextLeafIndex = AssignFlatIndices(node.NestedElements[i], nextLeafIndex); + else + nextLeafIndex++; + } + return nextLeafIndex; + } + } + struct ConversionInfo { public IType? inputType; @@ -780,8 +952,8 @@ bool MatchAssignment(ILInstruction? inst, [NotNullWhen(true)] out IType? targetT /// /// ldloc result a registered result or conversion output - /// ldobj(ldflda ItemN(ldloc(a) v)) an element read of the tuple - /// Resolves the value of a conversion or assignment to its element index. + /// ldobj(ldflda ItemN(ldloc(a) v)) an element read on the tuple node tree + /// Resolves the value of a conversion or assignment to its flat leaf index. /// Returns -1 on failure. /// int FindIndex(ILInstruction inst, out Action? delayedActions) @@ -793,6 +965,8 @@ int FindIndex(ILInstruction inst, out Action? delayedAct return -1; return index; } + if (!MatchTupleElementRead(inst, out var container, out var containerType, out int elementIndex)) + return -1; if (rootedInDeconstructCall) { // A pattern rooted in a Deconstruct call must not absorb tuple element @@ -800,29 +974,27 @@ int FindIndex(ILInstruction inst, out Action? delayedAct // bookkeeping and destroy the rewritten tuple access on failure. return -1; } - if (!MatchTupleElementRead(inst, out var container, out var containerType, out int elementIndex)) + var node = ResolveTupleContainer(container, containerType); + if (node == null) return -1; - var normalizedType = TupleType.FromUnderlyingType(context.TypeSystem, containerType); - if (this.tupleVariable == null) + if (elementIndex >= node.NestedElements.Length || node.NestedElements[elementIndex] != null) { - this.tupleVariable = container; - this.tupleType = (TupleType)normalizedType; - this.deconstructionResults = new ILVariable[this.tupleType.Cardinality]; - } - if (this.tupleType!.Cardinality < 2) - return -1; - if (container != tupleVariable || !this.tupleType.Equals(normalizedType)) + // The element is bound to a nested designation; a direct read of it would + // be a second consumption of the same element. return -1; - if (this.deconstructionResults[elementIndex] == null) + } + int flatIndex = node.ElementFlatIndex[elementIndex]; + node.MatchedAccessCount++; + if (this.deconstructionResults[flatIndex] == null) { - var freshVar = new ILVariable(VariableKind.StackSlot, this.tupleType.ElementTypes[elementIndex]) { Name = "E_" + elementIndex }; + var freshVar = new ILVariable(VariableKind.StackSlot, node.Type.ElementTypes[elementIndex]) { Name = "E_" + flatIndex }; delayedActions += _ => context.Function.Variables.Add(freshVar); - this.deconstructionResults[elementIndex] = freshVar; + this.deconstructionResults[flatIndex] = freshVar; } delayedActions += _ => { - inst.ReplaceWith(new LdLoc(this.deconstructionResults[elementIndex]!)); + inst.ReplaceWith(new LdLoc(this.deconstructionResults[flatIndex]!)); }; - return elementIndex; + return flatIndex; } /// @@ -897,6 +1069,57 @@ MatchInstruction BuildPatternMatch(DeconstructionCall call, ILVariable matchVari return match; } + /// + /// Builds, recursing into nested designations: + /// match.tuple (matchVariable = testedOperand) { + /// match(result_i = deconstruct.result i(ldloc matchVariable)), + /// match.tuple (temp_j = deconstruct.result j(ldloc matchVariable)) { ... } + /// } + /// Unassigned leaf elements get a fresh, load-free pattern variable (a discard). + /// + MatchInstruction BuildTuplePatternMatch(TupleNode node, ILVariable matchVariable, ILInstruction testedOperand) + { + matchVariable.Kind = VariableKind.PatternLocal; + var match = new MatchInstruction(matchVariable, method: null, testedOperand) { + IsDeconstructTuple = true + }; + for (int i = 0; i < node.Type.Cardinality; i++) + { + var nested = node.NestedElements[i]; + if (nested != null) + { + // A stack-slot temporary can have an imprecise type; the match variable of + // a tuple pattern must have the tuple type. + if (TupleType.GetTupleElementTypes(nested.Variable.Type).IsDefaultOrEmpty) + nested.Variable.Type = nested.Type; + match.SubPatterns.Add(BuildTuplePatternMatch(nested, nested.Variable, + new DeconstructResultInstruction(i, nested.Variable.StackType, new LdLoc(matchVariable)))); + } + else + { + int flatIndex = node.ElementFlatIndex[i]; + var result = deconstructionResults[flatIndex]; + if (result == null) + { + var freshVar = new ILVariable(VariableKind.PatternLocal, node.Type.ElementTypes[i]) { Name = "E_" + flatIndex }; + context.Function.Variables.Add(freshVar); + result = freshVar; + } + else + { + result.Kind = VariableKind.PatternLocal; + } + match.SubPatterns.Add( + new MatchInstruction( + result, + new DeconstructResultInstruction(i, result.StackType, new LdLoc(matchVariable)) + ) + ); + } + } + return match; + } + /// /// ldobj(ldflda ItemN(ldloc(a) container)) /// The returned index is zero-based; Rest chains of long tuples are flattened. @@ -919,6 +1142,24 @@ static bool MatchTupleElementRead(ILInstruction inst, [NotNullWhen(true)] out IL return MatchLdLocOrLdLoca(target, out container); } + /// + /// stloc temp(ldobj(ldflda ItemN(ldloc(a) container))) + /// The store of a nested tuple designation temporary. + /// + static bool MatchTupleElementStore(ILInstruction? inst, [NotNullWhen(true)] out ILVariable? temp, [NotNullWhen(true)] out ILVariable? container, [NotNullWhen(true)] out IType? containerType, out int index) + { + if (inst is StLoc store && MatchTupleElementRead(store.Value, out container, out containerType, out index)) + { + temp = store.Variable; + return true; + } + temp = null; + container = null; + containerType = null; + index = -1; + return false; + } + /// /// ldloc variable | ldloca variable /// From 266ac8a28e4c09dccb294a496f19d3f724c53d66 Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Fri, 7 Aug 2026 12:08:53 +0200 Subject: [PATCH 10/11] Decide deferral to an enclosing deconstruction in O(1) Deferring an inner deconstruction to its enclosing one used to be decided by matching the enclosing pattern in full, once per inner statement of the same pattern, discarding everything but the end position. The same decisions are available without it. A nested Deconstruct call can only be consumed by an enclosing one that is the immediately preceding statement, looking through the defensive copy of a struct element; anything else in between is a barrier that stops the enclosing from reaching this position, so it matches here instead. That leaves the case where the enclosing call is adjacent but cannot match anyway, which is decided by the constraint MatchDeconstructionCall already places on its out-parameters. The tuple-designation branch no longer needs the position the enclosing run starts at, so the backward walk that searched for it is gone with it. The added fixtures pin reconstruction across adjacent deconstructions, whose element stores that walk used to step through. Assisted-by: Claude:claude-opus-5[1m]:Claude Code Only defer to an enclosing designation that can reach this position The temporaries and element reads of a nested tuple designation are stored back to back, so a statement of any other kind between the temporary and a read of it stops the enclosing pattern from consuming that read. Deferring anyway lost the deconstruction entirely: the enclosing attempt fails and the back-to-front walk does not return to the position that stepped aside for it, so the reads were left as the plain element accesses they came from, which master reconstructs. Assisted-by: Claude:claude-opus-5[1m]:Claude Code --- .../TestCases/Pretty/DeconstructionTests.cs | 54 +++++++++++++ .../IL/Transforms/DeconstructionTransform.cs | 79 ++++++++++++------- 2 files changed, 103 insertions(+), 30 deletions(-) diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/DeconstructionTests.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/DeconstructionTests.cs index 95cf793e8a..494bf801b9 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/DeconstructionTests.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/DeconstructionTests.cs @@ -469,6 +469,60 @@ public void LocalVariable_Nested_TupleInner_BothElements() Console.WriteLine(value4); } + // Both sources are already materialized, so the element stores of the two + // deconstructions are adjacent with nothing in between. Locating the enclosing + // designation of the second one must not walk into the first one's stores. + public void LocalVariable_Nested_TupleInner_AfterAdjacentDeconstruction((int, (int, int)) source, (int, (int, int)) source2) + { + var (value, (value2, value3)) = source; + var (value4, (value5, value6)) = source2; + Console.WriteLine(value); + Console.WriteLine(value2); + Console.WriteLine(value3); + Console.WriteLine(value4); + Console.WriteLine(value5); + Console.WriteLine(value6); + } + + // A statement that is not part of the designation sits between the temporary and + // the reads of it, so the enclosing pattern cannot reach them; they have to be + // reconstructed on their own rather than deferred to a match that never happens. + public void LocalVariable_Nested_TupleInner_BarrierBeforeInnerReads((int, (int, int)) source) + { + (int, int) item = source.Item2; + Console.WriteLine(source.Item1); + var (value, value2) = item; + Console.WriteLine(value); + Console.WriteLine(value2); + } + + // Same, but the barrier sits between the temporary and the outer element read. + public void LocalVariable_Nested_TupleInner_BarrierAfterTemporary((int, (int, int)) source) + { + (int, int) item = source.Item2; + Console.WriteLine(GetInt()); + var (value, _) = source; + var (value2, value3) = item; + Console.WriteLine(value); + Console.WriteLine(value2); + Console.WriteLine(value3); + } + + // Same, but the preceding statements are plain element reads that keep the inner + // tuple whole, so they are element stores without being a deconstruction. Locating + // the enclosing designation walks back over them; the run they belong to is itself + // a deconstruction, so both are reconstructed. + public void LocalVariable_Nested_TupleInner_AfterAdjacentElementReads((int, (int, int)) source, (int, (int, int)) source2) + { + var (value, tuple2) = source; + var (value2, (value3, value4)) = source2; + Console.WriteLine(value); + Console.WriteLine(tuple2); + Console.WriteLine(value2); + Console.WriteLine(value3); + Console.WriteLine(value4); + } + public void LocalVariable_Nested_TupleInner_Conversions() { int value; diff --git a/ICSharpCode.Decompiler/IL/Transforms/DeconstructionTransform.cs b/ICSharpCode.Decompiler/IL/Transforms/DeconstructionTransform.cs index d4d2cc0611..4e1eeaa0ea 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/DeconstructionTransform.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/DeconstructionTransform.cs @@ -288,35 +288,49 @@ bool InlineDeconstructionInitializer(Block block, int pos) /// earlier position in the block, in either nesting shape: /// /// call Deconstruct(..., ldloca inner, ...) at enclosingPos - /// ... - /// call Deconstruct(ldloc(a) inner, ...) at pos + /// [stloc copy(ldloc inner)] defensive copy of a struct element + /// call Deconstruct(ldloc(a) inner|copy, ...) at pos /// - /// stloc temp(ldobj(ldflda ItemN(ldloc(a) outer))) at enclosingPos + /// stloc temp(ldobj(ldflda ItemN(ldloc(a) outer))) earlier in the block /// ... /// stloc x([conv](ldobj(ldflda ItemK(ldloc(a) temp)))) at pos /// - /// Both shapes are decided by the same dry run of the enclosing match: only a match that - /// reaches beyond pos absorbs the statement there. A barrier statement between the two - /// positions, an element with uses the nesting cannot consume, or a conversion or - /// assignment the enclosing pattern does not account for makes the dry run stop short, - /// and the deconstruction at pos is then still transformed on its own. What the dry run - /// cannot promise is that the enclosing attempt still matches once the walk reaches it: - /// the positions in between are visited first and may rewrite the block. The back-to-front - /// walk gives this position no second chance, but losing the match there only costs - /// sugar, never correctness. + /// The chained calls are emitted back to back, so an enclosing call that is not the + /// preceding statement has something between it and pos that stops it from reaching + /// here; the deconstruction at pos is then matched on its own. Nested designation + /// temporaries are stored before the enclosing run's own element reads, so the two are + /// not adjacent and only the store has to be found. + /// + /// Deferring is worth it only if the enclosing attempt can succeed, so the constraint + /// MatchDeconstructionCall places on out-parameters is checked here as well: without it + /// an element used more than once would defer this position to an attempt that then + /// rejects the call, and the back-to-front walk gives it no second chance. + /// + /// Getting this wrong costs sugar, never correctness: the statement at pos is either + /// folded into the enclosing deconstruction or decompiled as the explicit calls and + /// element reads it came from. /// bool IsConsumableByEnclosingDeconstruction(Block block, int pos) { - if (!TryFindEnclosingDeconstructionCall(block, pos, out int enclosingPos) - && !TryFindEnclosingTupleDesignation(block, pos, out enclosingPos)) + if (TryFindEnclosingDeconstructionCall(block, pos, out int enclosingPos)) { - return false; + if (enclosingPos != pos - 1 + && !(enclosingPos == pos - 2 && block.Instructions[pos - 1] is StLoc { Value: LdLoc })) + { + return false; + } + var enclosingCall = (CallInstruction)block.Instructions[enclosingPos]; + for (int i = 1; i < enclosingCall.Arguments.Count; i++) + { + if (!enclosingCall.Arguments[i].MatchLdLoca(out var outParam) + || !(outParam.StoreCount == 0 && outParam.AddressCount == 1 && outParam.LoadCount <= 1)) + { + return false; + } + } + return true; } - // The dry run leaves the matcher state behind, which is safe because it runs before - // the attempt at this position, and both that attempt and Run reset it. It does not - // modify the block: all rewrites are delayed actions. - return MatchDeconstructionSequence(block, enclosingPos, out int endPos, out _, out _, out _, out _) - && endPos > pos; + return HasEnclosingTupleDesignation(block, pos); } /// @@ -356,17 +370,14 @@ static bool TryFindEnclosingDeconstructionCall(Block block, int pos, out int enc } /// - /// stloc temp(ldobj(ldflda ItemN(ldloc(a) outer))) at enclosingPos + /// stloc temp(ldobj(ldflda ItemN(ldloc(a) outer))) earlier in the block /// ... /// stloc x([conv](ldobj(ldflda ItemK(ldloc(a) temp)))) at pos /// The statement at pos reads an element of a tuple stored by an earlier statement that - /// is itself an element read, i.e. a candidate nested designation temporary. The - /// enclosing pattern's matching starts at the first store of the run that store belongs - /// to, because the temporaries of a nested designation are stored back to back. + /// is itself an element read, i.e. a candidate nested designation temporary. /// - static bool TryFindEnclosingTupleDesignation(Block block, int pos, out int enclosingPos) + static bool HasEnclosingTupleDesignation(Block block, int pos) { - enclosingPos = -1; if (!block.Instructions[pos].MatchStLoc(out _, out var value)) return false; if (value is Conv conv) @@ -377,10 +388,18 @@ static bool TryFindEnclosingTupleDesignation(Block block, int pos, out int enclo return false; if (!MatchTupleElementStore(store, out _, out _, out _, out _)) return false; - enclosingPos = store.ChildIndex; - while (enclosingPos > 0 && MatchTupleElementStore(block.Instructions[enclosingPos - 1], out _, out _, out _, out _)) - enclosingPos--; - return enclosingPos < pos; + if (store.ChildIndex >= pos) + return false; + // The temporaries and element reads of one designation are stored back to back. + // A statement of any other kind in between stops the enclosing pattern from + // reaching this position, and deferring to it would lose the deconstruction here + // as well, because the back-to-front walk does not come back. + for (int between = store.ChildIndex + 1; between < pos; between++) + { + if (!MatchTupleElementStore(block.Instructions[between], out _, out _, out _, out _)) + return false; + } + return true; } /// From f7eeb451866ba4044f5d78e85e2dbdb3f6553688 Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Fri, 7 Aug 2026 15:36:20 +0200 Subject: [PATCH 11/11] Demote an escaping first element instead of giving up on the pattern A nested designation whose temporary is still read elsewhere is retried with that variable demoted to a designator leaf. The check that the first tuple element must be assigned ran before that retry, and every leaf of a wrongly nested first element precedes the assigned ones, so the pattern looked like it started mid-way and was rejected before the retry could restore it. The flat deconstruction was lost for a shape that has one. Assisted-by: Claude:claude-opus-5[1m]:Claude Code --- .../TestCases/Pretty/DeconstructionTests.cs | 10 ++++++++ .../IL/Transforms/DeconstructionTransform.cs | 24 +++++++++++-------- 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/DeconstructionTests.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/DeconstructionTests.cs index 494bf801b9..1fc53d916a 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/DeconstructionTests.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/DeconstructionTests.cs @@ -551,6 +551,16 @@ public void LocalVariable_TupleInner_ElementUsedOutside() #endif } + // Same, but the escaping element is in the first position. Every leaf of the + // wrongly nested node precedes the assigned ones there, so the retry that demotes + // it has to be reached before the pattern is judged to start mid-way. + public void LocalVariable_TupleInner_FirstElementUsedOutside() + { + var (tuple2, value) = GetTuple<(int, int), int>(); + Console.WriteLine(tuple2.Item1); + Console.WriteLine(value); + } + public void ForEach_Nested_TupleInner() { foreach (var (value, (value2, value3)) in GetList<(int, (int, int))>()) diff --git a/ICSharpCode.Decompiler/IL/Transforms/DeconstructionTransform.cs b/ICSharpCode.Decompiler/IL/Transforms/DeconstructionTransform.cs index 4e1eeaa0ea..63324af82d 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/DeconstructionTransform.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/DeconstructionTransform.cs @@ -210,22 +210,26 @@ bool MatchDeconstructionSequence(Block block, int startPos, out int endPos, // the forwarding fixup in MatchAssignments. if (!anyAssignments && !(rootCall != null && rootCall.NestedCalls.Any(c => c != null))) return false; - // first tuple element may not be discarded, - // otherwise we would run this transform on a suffix of the actual pattern. - if (deconstructionResults[0] == null) - return false; // A nested tuple designation only holds if the pattern consumed every read of // its temporary; a remaining read means the value escapes the designation. // Retry with the variable as a plain designator leaf, which restores the flat - // deconstruction the escaping read needs. + // deconstruction the escaping read needs. This has to be decided before the + // leaf check below: every leaf of a wrongly nested first element precedes the + // assigned ones, so that check would report the pattern as starting mid-way + // and give up on a designation the retry can still make work. var escaped = EscapedTupleNodes(); - if (escaped == null) + if (escaped != null) { - endPos = pos; - return true; + doNotNest ??= new HashSet(); + doNotNest.UnionWith(escaped); + continue; } - doNotNest ??= new HashSet(); - doNotNest.UnionWith(escaped); + // first tuple element may not be discarded, + // otherwise we would run this transform on a suffix of the actual pattern. + if (deconstructionResults[0] == null) + return false; + endPos = pos; + return true; } List? EscapedTupleNodes()