icsharpcode/ILSpy
1
0
Fork 0
mirror of https://github.com/icsharpcode/ILSpy.git synced 2026-10-12 02:15:58 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix #4059: treat a Deconstruct out argument as a definition

Browse Source 
The reaching-definitions analysis does not track address loads, so a
variable that is only ever written through its address stays potentially
uninitialized for its whole life. SplitVariables merges every such load
into one live range, which is why csc lowering two deconstructions onto
the same pair of temporaries left neither of them recognizable.

IL cannot express that a method assigns through an address without
reading it first - 'out' is a C# convention over 'ref' - so the address
loads that qualify have to be named. A Deconstruct method is the one
case that matters here, and C#'s definite assignment rules rule out a
read; a Deconstruct method written in IL could read the argument, and
that possibility is knowingly ignored.

Assisted-by: Claude:claude-opus-5:Claude Code
This commit is contained in:
Siegfried Pammer
2026-09-06 08:42:34 +02:00
parent 712ad1aed1
commit fd623ccf4f
2 changed files with 42 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files
ICSharpCode.Decompiler.Tests/TestCases/Pretty/DeconstructionTests.cs
+9 -11
View File
@@ -1240,17 +1240,15 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
Console.WriteLine(value2);
}
// #4059: csc reuses the same out-slot temporaries for both calls, and hands them to the
// second one in the opposite order, so neither deconstruction is recognized.
//public void TwoBackToBackDeconstructs_Custom()
//{
// var (value, value2) = GetSource<string, string>();
// Console.WriteLine(value);
// Console.WriteLine(value2);
// var (value3, value4) = GetSource<string, string>();
// Console.WriteLine(value3);
// Console.WriteLine(value4);
//}
public void TwoBackToBackDeconstructs_Custom()
{
var (value, value2) = GetSource<string, string>();
Console.WriteLine(value);
Console.WriteLine(value2);
var (value3, value4) = GetSource<string, string>();
Console.WriteLine(value3);
Console.WriteLine(value4);
}
}
}
ICSharpCode.Decompiler/FlowAnalysis/ReachingDefinitionsVisitor.cs
+33 -2
View File
@@ -39,7 +39,9 @@ namespace ICSharpCode.Decompiler.FlowAnalysis
/// * <c>StLoc</c>
/// * <c>TryCatchHandler</c> (for the exception variable)
/// * <c>ReachingDefinitionsVisitor.UninitializedVariable</c> for uninitialized variables.
/// Note that we do not keep track of <c>LdLoca</c>/references/pointers.
/// * <c>LdLoca</c>, but only where the address is known to be assigned without being read
/// first, see <c>IsDefiniteAssignment</c>.
/// Note that we do not otherwise keep track of <c>LdLoca</c>/references/pointers.
/// The analysis will likely be wrong/incomplete for variables with <c>AddressCount != 0</c>.
///
/// Note: this class does not store the computed information, because doing so
@@ -304,6 +306,8 @@ namespace ICSharpCode.Decompiler.FlowAnalysis
int expectedStoreCount = scope.Variables[vi].StoreInstructions.Count;
// Extra store for the uninitialized state.
expectedStoreCount += 1;
// Extra store for every address load that is a definite assignment.
expectedStoreCount += scope.Variables[vi].AddressInstructions.Count(IsDefiniteAssignment);
Debug.Assert(stores.Count == expectedStoreCount);
stores.CopyTo(allStores, si);
// Add all stores except for the first (representing the uninitialized state)
@@ -335,7 +339,7 @@ namespace ICSharpCode.Decompiler.FlowAnalysis
}
foreach (var inst in scope.Descendants)
{
if (inst.HasDirectFlag(InstructionFlags.MayWriteLocals))
if (inst.HasDirectFlag(InstructionFlags.MayWriteLocals) || IsDefiniteAssignment(inst))
{
cancellationToken.ThrowIfCancellationRequested();
ILVariable v = ((IInstructionWithVariableOperand)inst).Variable;
@@ -395,6 +399,33 @@ namespace ICSharpCode.Decompiler.FlowAnalysis
HandleStore(inst, inst.Variable);
}
protected internal override void VisitLdLoca(LdLoca inst)
{
if (IsDefiniteAssignment(inst))
HandleStore(inst, inst.Variable);
else
base.VisitLdLoca(inst);
}
/// <summary>
/// Gets whether an address load definitely assigns the variable without reading it first,
/// which makes it a definition, just like a store.
/// <para>
/// IL has no such instruction - 'out' is a C# convention over 'ref' - so this holds only
/// for the out arguments of a Deconstruct method, where C#'s definite assignment rules
/// rule out a read. A Deconstruct method written in IL could read the argument, and the
/// possibility is ignored here: without it, two deconstructions that the compiler lowered
/// onto the same temporaries stay in one live range and neither can be recognized.
/// </para>
/// </summary>
static bool IsDefiniteAssignment(ILInstruction inst)
{
// Argument 0 is the receiver of an instance method or the 'this' argument of an
// extension method; IsDeconstructMethod requires every parameter after it to be out.
return inst is LdLoca { ChildIndex: > 0, Parent: CallInstruction call }
&& MatchInstruction.IsDeconstructMethod(call.Method);
}
protected override void HandleMatchStore(MatchInstruction inst)
{
HandleStore(inst, inst.Variable);