From 689af4cdba99c3948ba6deae2763ba96a902cd82 Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Fri, 7 Aug 2026 12:08:53 +0200 Subject: [PATCH] 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 95cf793e8..494bf801b 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/DeconstructionTests.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/DeconstructionTests.cs @@ -469,6 +469,60 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty 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 d4d2cc061..4e1eeaa0e 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/DeconstructionTransform.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/DeconstructionTransform.cs @@ -288,35 +288,49 @@ namespace ICSharpCode.Decompiler.IL.Transforms /// 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 @@ namespace ICSharpCode.Decompiler.IL.Transforms } /// - /// 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 @@ namespace ICSharpCode.Decompiler.IL.Transforms 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; } ///