Fix the remaining deconstruction issues

[siegfriedpammer]

Fixes #3208, fixes #3453, fixes #3962. Closes #2322, closes #3037, closes #3275: those three no longer reproduce on master (#3275 was fixed by #3949), and this PR adds the missing regression fixtures for them, so they can be closed with it.

Triage first: each issue's repro was compiled and decompiled against current master. The three crashes/wrong-code reports (#2322, #3037, #3275) are already fixed - #2322 is covered by the existing DeconstructDictionaryForEach fixture, and this PR adds fixtures for #3037 (deconstruction assignment to locals captured by a lambda in an async method) and #3275 (nested deconstruction of readonly record structs). What remained collapses into two defects:

Value-semantics copy left behind (#3453, #3208). Deconstruction assignment copies the right-hand side into a temporary before calling Deconstruct. When the RHS is a call, inlining folds the temporary; for a local or parameter it survived into the output as a separate statement (CalibrationPoint calibrationPoint = point; var (num, num2) = calibrationPoint;). DeconstructionTransform now consumes the copy into the pattern - rendering the copied value as the RHS recompiles to the identical temporary. Because blocks are matched back to front, the call-position match defers to the attempt starting at the copy, mirroring the existing nested-deconstruction defer guard.

Rest-chained inner tuple designation (#3962). Element 8+ of a long tuple is read through the Rest field, which Roslyn loads by value; ILSpy rendered that as an addressof(ldobj(...)) sandwich that hid the tuple field chain from every downstream matcher (including the existing Rest flattening in MatchTupleFieldAccess). Two-part fix at the failure origin: EarlyExpressionTransforms elides the copy when the enclosing expression only reads through it, and the deconstruction transform's use-shape guard walks the resulting ldflda chain instead of requiring exactly one field access. var (x, (a, b, c, d, e, f, g, h)) = ... now reconstructs fully; as a side effect, plain reads render as item.Item8 instead of item.Rest.Item1.

Each fix was developed fixture-first (red on master, green after); the full decompiler suite passes on every commit.

---

This PR description was written by the Claude Code agent session that authored the change.

🤖 Generated with Claude Code

[christophwille]

Automated high-effort review (Claude Code, multi-agent find + independent verify pass). Three correctness findings and four cleanups survived verification; three further candidates were investigated and refuted. Details are in the inline comments; the short version:

Correctness

1. EarlyExpressionTransforms.AddressOfLdLocToLdLoca - the new addressof(ldobj) elision has no type-compatibility guard between the inner ldobj and the enclosing read, unlike its siblings LdObjToLdLoc/StObjToStLoc. Type-punning IL gets silently re-rooted onto memory the original never read.
2. DeconstructionTransform defer guard - when the deferred-to attempt at pos - 1 is itself aborted by IsConsumableByEnclosingDeconstruction, the deconstruction is lost at both positions (reproduced end-to-end); master sugared this shape.
3. IsRootDeconstructionCopy also matches the defensive copy of a first-position nested struct element of an enclosing Deconstruct call, so the inner call gets committed as a standalone root deconstruction and the enclosing nested designation is permanently broken. Existing fixtures only cover struct elements in non-first positions, so the suite stays green.

Cleanups

• MatchDeconstruction re-matches the copy+call pair IsRootDeconstructionCopy just matched (up to three times per position); outing the copied value / matched call from the helper removes the redundancy and the drift hazard.
• IsRootDeconstructionCopy discards the copied value (out _), so a byref-typed temp also matches - an in-place Deconstruct through a reference renders as a value-semantics deconstruction.
• The two AddressOfLdLocToLdLoca branches duplicate MatchAddressOf plus the whole replacement/ILRange/ReplaceWith tail.

Findings 2 and 3 each come with a concrete source shape that regresses versus master - both would make good fixture additions.

[christophwille]

Correctness (verified): this new ldobj(...(addressof(ldobj(addr)))) => ldobj(...(addr)) elision only checks the inner LdObj's volatile/unaligned prefixes, but never checks type compatibility between the type the addressof temporary holds and the enclosing read. The sibling transforms (LdObjToLdLoc, StObjToStLoc) guard exactly this with TypeUtils.IsCompatibleTypeForMemoryAccess.

For type-punning IL (hand-written or obfuscated, not Roslyn-emitted) - e.g. ldobj T2(addressof(ldobj T1(addr))) with sizeof(T2) > sizeof(T1), or an ldflda chain whose declaring type does not match the inner ldobj's type - the rewrite re-roots the read directly onto addr: the original program read bytes out of a temporary copy of T1, while the transformed expression reads adjacent live memory behind addr that the original never touched. A type-compatibility check between the inner ldobj type and the addressof type (mirroring the sibling transforms) would restrict the elision to the Roslyn shape it targets.

[christophwille]

Correctness (verified, reproduced end-to-end): this guard unconditionally defers the call-only match at pos whenever the copy+call shape holds at pos - 1, but the deferred-to attempt at pos - 1 can itself be aborted before MatchDeconstruction runs: when the copy's value is a tuple-element read of an earlier designation temporary, IsConsumableByEnclosingDeconstruction (line 130) returns true for pos - 1 via HasEnclosingTupleDesignation, and that attempt bails. The back-to-front walk never returns to pos, so the deconstruction is lost at both positions.

Repro shape: var inner = big.Item1; var (x, y) = inner.Item1; (element type has a Deconstruct method). On master the call position matched and emitted var (x, y) = ...;; with this guard the output regresses to an explicit temporary plus a raw .Deconstruct(out ...) call. Deferring only when the pos - 1 attempt will actually consume the pair (i.e. not itself deferred/aborted) would fix it.

Minor, related: the guard here and the consuming check inside MatchDeconstruction encode the same adjacency test at shifted offsets ((pos-1, pos) vs (pos, pos+1)), coupled only by comments. If the consuming side is ever tightened without mirroring this guard, positions defer to an attempt that no longer consumes them and the sugar is silently lost. Keeping both sides on the identical shared predicate is what holds this together - worth preserving deliberately.

[christophwille]

Correctness (verified): IsRootDeconstructionCopy also matches the defensive copy of a nested struct element of an enclosing Deconstruct call, so the attempt starting at that copy commits the inner call as a standalone root deconstruction and permanently breaks the enclosing nested designation. The back-to-front walk visits the copy before the enclosing call, and nothing defers at the copy position: IsConsumableByEnclosingDeconstruction needs a CallInstruction at pos, and the pos - 1 guard sees the enclosing call, not a stloc.

Repro shape: var ((a, b), c) = GetSource<StructDeconstructionSource<int, string>, int>(); - a struct element with its own Deconstruct in first position, conversion-free leaves. At the copy position, MatchDeconstruction consumes copy+inner call and commits (a, b) = s;; the enclosing call then has no assignments left and stays explicit. Output regresses from master's var ((a, b), c) = src; to src.Deconstruct(out var s, out var num); (a, b) = s; int c = num;.

The existing fixtures only place struct elements in non-first positions (BothElementsNested has the class element first), so the suite stays green while this shape regresses - a first-position fixture would catch it.

[christophwille]

Cleanup: this branch re-matches instructions IsRootDeconstructionCopy just matched - a MatchStLoc whose bool result is discarded, plus a second MatchDeconstructionCall on the same call instruction (a third counting the TransformDeconstruction guard). Having IsRootDeconstructionCopy return the copied value and the matched DeconstructionCall via out parameters removes the redundant work and the lockstep hazard: today, if the helper is ever loosened (say, to accept a stobj-shaped copy), the ignored-result MatchStLoc(out _, out var copiedValue) silently leaves copiedValue wrong instead of failing the match.

The || testedOperand == null check below is also dead: MatchDeconstructionCall sets testedOperand from call.Arguments[0] after checking Arguments.Count >= 3.

[christophwille]

Strictness: the copied value is discarded (out _), so the helper never checks that this is actually a by-value copy. A byref-typed temp - stloc r(ldflda f(x)); call Deconstruct(ldloc r, ...) - satisfies every condition here (MatchLdLocOrLdLoca accepts the plain ldloc r receiver), and an in-place Deconstruct call through a reference gets rendered as a value-semantics deconstruction of x.f. If Deconstruct mutates its receiver struct, the decompiled (a, b) = x.f; recompiles to different behavior.

The XML doc describes the target shape as the value-semantics temporary for a local or parameter; constraining the copied value (non-byref copy variable type, or value matching ldloc/ldobj) keeps the matcher at that guaranteed shape.

[christophwille]

Cleanup: the two branches are near-duplicates - temp.MatchAddressOf is evaluated twice (every visited LdObj pays the second match when the first branch fails), and the replacement/ILRange-transfer/ReplaceWith/EndStep boilerplate is duplicated. A single if (temp.MatchAddressOf(out var addressOfTarget, out var type)) that dispatches on addressOfTarget (MatchLdLoc vs LdObj) to compute the replacement, sharing one range-transfer loop and one ReplaceWith/EndStep tail, does the same job with half the surface - and gives the missing type check (see the comment on the LdObj branch) a single place to live.