Skip to content

Fix IL2080 for conditional tuple deconstruction #133971

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Copilot wants to merge 2 commits into from
+245 −13
Closed

Fix IL2080 for conditional tuple deconstruction #133971

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
928c07d
Initial plan
Copilot Sep 15, 2026
b884090
Preserve tuple flow through conditional deconstruction
Copilot Sep 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
193 changes: 185 additions & 8 deletions src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,16 @@ public abstract class LocalDataFlowVisitor<TValue, TContext, TValueLattice, TCon

private readonly ImmutableHashSet<CaptureId> _deconstructionLValueFlowCaptures;

private readonly ImmutableHashSet<CaptureId> _conditionalDeconstructionFlowCaptures;

private readonly Dictionary<CaptureId, ImmutableArray<ITupleOperation>> _tupleFlowCaptureSources = new();

private CaptureId? _tupleFlowCaptureId;

private ImmutableArray<int> _tupleFlowCapturePath;

private ITupleOperation? _tupleFlowCaptureOperation;

public InterproceduralState<TValue, TValueLattice> InterproceduralState;

private bool IsLValueFlowCapture(CaptureId captureId)
Expand All @@ -80,14 +90,58 @@ public LocalDataFlowVisitor(
_semanticModel = cfg.OriginalOperation.SemanticModel ??
compilation.GetSemanticModel(cfg.OriginalOperation.Syntax.SyntaxTree);
this.lValueFlowCaptures = lValueFlowCaptures;
_deconstructionLValueFlowCaptures = cfg
ImmutableArray<IFlowCaptureReferenceOperation> flowCaptureReferences = cfg
.DescendantOperations<IFlowCaptureReferenceOperation>(OperationKind.FlowCaptureReference)
.ToImmutableArray();
_deconstructionLValueFlowCaptures = flowCaptureReferences
.Where(reference => reference.IsInLeftOfDeconstructionAssignment(out _))
.Select(reference => reference.Id)
.ToImmutableHashSet();
_conditionalDeconstructionFlowCaptures = flowCaptureReferences
.Where(reference =>
reference.Syntax is ConditionalExpressionSyntax &&
reference.Parent is IDeconstructionAssignmentOperation deconstruction &&
UnwrapDeconstructionSource(deconstruction.Value) == reference)
.Select(reference => reference.Id)
.ToImmutableHashSet();
InterproceduralState = interproceduralState;
}

public override TValue DefaultVisit(
IOperation operation,
LocalDataFlowState<TValue, TContext, TValueLattice, TContextLattice> state)
{
if (_tupleFlowCaptureId is not CaptureId captureId ||
operation is not ITupleOperation tuple ||
operation != _tupleFlowCaptureOperation)
{
return base.DefaultVisit(operation, state);
}

for (int i = 0; i < tuple.Elements.Length; i++)
{
ImmutableArray<int> elementPath = _tupleFlowCapturePath.Add(i);
ImmutableArray<int> previousPath = _tupleFlowCapturePath;
ITupleOperation? previousOperation = _tupleFlowCaptureOperation;
_tupleFlowCapturePath = elementPath;
_tupleFlowCaptureOperation = UnwrapDeconstructionSource(tuple.Elements[i]) as ITupleOperation;
TValue elementValue;
try
{
elementValue = Visit(tuple.Elements[i], state);
}
finally
{
_tupleFlowCapturePath = previousPath;
_tupleFlowCaptureOperation = previousOperation;
}

state.Set(new LocalKey(captureId, elementPath), elementValue);
}

return TopValue;
}

public abstract void ApplyCondition(TConditionValue condition, ref LocalStateAndContext<TValue, TContext> localContextState);

public TConditionValue? Transfer(
Expand Down Expand Up @@ -641,6 +695,14 @@ public override TValue VisitDeconstructionAssignment(
IOperation source = UnwrapDeconstructionSource(operation.Value);
bool sourceValueIsKnown = source is not ITupleOperation;
TValue sourceValue = sourceValueIsKnown ? Visit(source, state) : TopValue;
TupleFlowCapture sourceTupleCapture = default;
if (source is IFlowCaptureReferenceOperation { Syntax: ConditionalExpressionSyntax } flowCaptureReference &&
_conditionalDeconstructionFlowCaptures.Contains(flowCaptureReference.Id))
{
ImmutableArray<ITupleOperation> tupleSources = GetTupleFlowCaptureSources(flowCaptureReference.Id);
if (!tupleSources.IsDefaultOrEmpty)
sourceTupleCapture = new TupleFlowCapture(flowCaptureReference.Id, tupleSources);
}

// Deconstruction evaluates all source values before assigning any target. Keeping these
// phases separate is required for assignments such as (first, second) = (second, first).
Expand All @@ -652,7 +714,8 @@ public override TValue VisitDeconstructionAssignment(
sourceValueIsKnown,
deconstructionInfo,
operation,
state);
state,
sourceTupleCapture);
if (deconstructionValue.DoesNotReturn)
{
state.Current = LocalStateAndContextLattice.Top;
Expand Down Expand Up @@ -708,6 +771,58 @@ private DeconstructionValue(bool isInvalid, bool doesNotReturn)
public static DeconstructionValue NonReturning => new(isInvalid: false, doesNotReturn: true);
}

private readonly struct TupleFlowCapture
{
private readonly CaptureId _captureId;

private readonly ImmutableArray<int> _path;

private readonly ImmutableArray<ITupleOperation> _sources;

public bool HasValue => !_sources.IsDefaultOrEmpty;

public TupleFlowCapture(CaptureId captureId, ImmutableArray<ITupleOperation> sources)
: this(captureId, ImmutableArray<int>.Empty, sources)
{
}

private TupleFlowCapture(
CaptureId captureId,
ImmutableArray<int> path,
ImmutableArray<ITupleOperation> sources)
{
_captureId = captureId;
_path = path;
_sources = sources;
}

public LocalKey GetElementKey(int index)
{
Debug.Assert(HasValue);
return new LocalKey(_captureId, _path.Add(index));
}

public TupleFlowCapture GetNested(int index)
{
if (!HasValue)
return default;

var nestedSources = ImmutableArray.CreateBuilder<ITupleOperation>(_sources.Length);
foreach (ITupleOperation source in _sources)
{
if ((uint)index >= (uint)source.Elements.Length ||
UnwrapDeconstructionSource(source.Elements[index]) is not ITupleOperation nestedSource)
{
return default;
}

nestedSources.Add(nestedSource);
}

return new TupleFlowCapture(_captureId, _path.Add(index), nestedSources.MoveToImmutable());
}
}

private DeconstructionValue EvaluateDeconstruction(
IOperation target,
IOperation? source,
Expand All @@ -716,7 +831,8 @@ private DeconstructionValue EvaluateDeconstruction(
bool sourceValueIsKnown,
DeconstructionInfo deconstructionInfo,
IDeconstructionAssignmentOperation operation,
LocalDataFlowState<TValue, TContext, TValueLattice, TContextLattice> state)
LocalDataFlowState<TValue, TContext, TValueLattice, TContextLattice> state,
TupleFlowCapture sourceTupleCapture)
{
target = UnwrapDeconstructionTarget(target);

Expand Down Expand Up @@ -798,7 +914,8 @@ private DeconstructionValue EvaluateDeconstruction(
sourceValueIsKnown: true,
deconstructionInfo.Nested[i],
operation,
state);
state,
sourceTupleCapture: default);
if (nestedValue.DoesNotReturn)
return DeconstructionValue.NonReturning;
nestedValues.Add(nestedValue);
Expand All @@ -822,7 +939,8 @@ private DeconstructionValue EvaluateDeconstruction(
sourceValueIsKnown: false,
deconstructionInfo.Nested[i],
operation,
state);
state,
sourceTupleCapture: default);
if (nestedValue.DoesNotReturn)
return DeconstructionValue.NonReturning;
nestedValues.Add(nestedValue);
Expand All @@ -842,15 +960,21 @@ private DeconstructionValue EvaluateDeconstruction(
for (int i = 0; i < targetTuple.Elements.Length; i++)
{
IFieldSymbol tupleElement = tupleType.TupleElements[i];
// Roslyn distributes a top-level conditional deconstruction into its tuple branches.
// Use the values captured for each element instead of synthesizing tuple field reads.
TValue tupleElementValue = sourceTupleCapture.HasValue
? state.Get(sourceTupleCapture.GetElementKey(i))
: GetTupleElementValue(tupleElement);
DeconstructionValue tupleValue = EvaluateDeconstruction(
targetTuple.Elements[i],
source: null,
tupleElement.Type,
GetTupleElementValue(tupleElement),
tupleElementValue,
sourceValueIsKnown: true,
deconstructionInfo.Nested[i],
operation,
state);
state,
sourceTupleCapture.GetNested(i));
if (tupleValue.DoesNotReturn)
return DeconstructionValue.NonReturning;
tupleValues.Add(tupleValue);
Expand All @@ -859,6 +983,31 @@ private DeconstructionValue EvaluateDeconstruction(
return new DeconstructionValue(tupleValues.MoveToImmutable());
}

private ImmutableArray<ITupleOperation> GetTupleFlowCaptureSources(CaptureId captureId)
{
if (_tupleFlowCaptureSources.TryGetValue(captureId, out ImmutableArray<ITupleOperation> sources))
return sources;

var builder = ImmutableArray.CreateBuilder<ITupleOperation>();
foreach (IFlowCaptureOperation flowCapture in ControlFlowGraph.DescendantOperations<IFlowCaptureOperation>(OperationKind.FlowCapture))
{
if (!flowCapture.Id.Equals(captureId))
continue;

if (UnwrapDeconstructionSource(flowCapture.Value) is not ITupleOperation tupleSource)
{
_tupleFlowCaptureSources.Add(captureId, default);
return default;
}

builder.Add(tupleSource);
}

sources = builder.Count == 0 ? default : builder.ToImmutable();
_tupleFlowCaptureSources.Add(captureId, sources);
return sources;
}

private void AssignDeconstruction(
IOperation target,
DeconstructionValue value,
Expand Down Expand Up @@ -1182,14 +1331,42 @@ public override TValue VisitFlowCapture(IFlowCaptureOperation operation, LocalDa
}
else
{
capturedValue = Visit(operation.Value, state);
capturedValue = VisitFlowCaptureValue(operation, state);
}

state.Set(new LocalKey(operation.Id), capturedValue);
return capturedValue;
}
}

private TValue VisitFlowCaptureValue(
IFlowCaptureOperation operation,
LocalDataFlowState<TValue, TContext, TValueLattice, TContextLattice> state)
{
if (!_conditionalDeconstructionFlowCaptures.Contains(operation.Id) ||
UnwrapDeconstructionSource(operation.Value) is not ITupleOperation)
{
return Visit(operation.Value, state);
}

CaptureId? previousCaptureId = _tupleFlowCaptureId;
ImmutableArray<int> previousPath = _tupleFlowCapturePath;
ITupleOperation? previousOperation = _tupleFlowCaptureOperation;
_tupleFlowCaptureId = operation.Id;
_tupleFlowCapturePath = ImmutableArray<int>.Empty;
_tupleFlowCaptureOperation = (ITupleOperation)UnwrapDeconstructionSource(operation.Value);
try
{
return Visit(operation.Value, state);
}
finally
{
_tupleFlowCaptureId = previousCaptureId;
_tupleFlowCapturePath = previousPath;
_tupleFlowCaptureOperation = previousOperation;
}
}

public override TValue VisitExpressionStatement(IExpressionStatementOperation operation, LocalDataFlowState<TValue, TContext, TValueLattice, TContextLattice> state)
{
Visit(operation.Operation, state);
Expand Down
29 changes: 24 additions & 5 deletions src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalStateLattice.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using ILLink.Shared;
using ILLink.Shared.DataFlow;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.FlowAnalysis;
Expand All @@ -15,23 +17,40 @@ namespace ILLink.RoslynAnalyzer.DataFlow

private readonly CaptureId? CaptureId;

public LocalKey(ILocalSymbol symbol) => (Local, CaptureId) = (symbol, null);
private readonly ImmutableArray<int> TupleElementPath;

public LocalKey(CaptureId captureId) => (Local, CaptureId) = (null, captureId);
public LocalKey(ILocalSymbol symbol) => (Local, CaptureId, TupleElementPath) = (symbol, null, default);

public bool Equals(LocalKey other) => SymbolEqualityComparer.Default.Equals(Local, other.Local) &&
(CaptureId?.Equals(other.CaptureId) ?? other.CaptureId == null);
public LocalKey(CaptureId captureId) => (Local, CaptureId, TupleElementPath) = (null, captureId, default);

internal LocalKey(CaptureId captureId, ImmutableArray<int> tupleElementPath) =>
(Local, CaptureId, TupleElementPath) = (null, captureId, tupleElementPath);

public bool Equals(LocalKey other) =>
SymbolEqualityComparer.Default.Equals(Local, other.Local) &&
(CaptureId?.Equals(other.CaptureId) ?? other.CaptureId == null) &&
TupleElementPath.AsSpan().SequenceEqual(other.TupleElementPath.AsSpan());

public override bool Equals(object obj)
=> obj is LocalKey inst && Equals(inst);

public override int GetHashCode()
=> CaptureId is null ? SymbolEqualityComparer.Default.GetHashCode(Local) : CaptureId.GetHashCode();
{
int hashCode = CaptureId is null ? SymbolEqualityComparer.Default.GetHashCode(Local) : CaptureId.GetHashCode();
if (!TupleElementPath.IsDefault)
{
foreach (int index in TupleElementPath)
hashCode = HashUtils.Combine(hashCode, index);
}

return hashCode;
}

public override string ToString()
{
if (Local != null)
return Local.ToString();

return $"capture {CaptureId.GetHashCode()}";
}
}
Expand Down
36 changes: 36 additions & 0 deletions src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ConstructedTypesDataFlow.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,39 @@ static void DeconstructTupleLiteral(
type.RequiresPublicMethods();
}

static void DeconstructConditionalTupleLiteral(bool condition)
{
(var methodName, var type) = condition
? (nameof(string.IsNullOrEmpty), typeof(string))
: (nameof(object.ReferenceEquals), typeof(object));
_ = type.GetMethod(methodName);

((var nestedMethodName, var nestedType), _) = condition
? ((nameof(string.IsNullOrEmpty), typeof(string)), 0)
: ((nameof(object.ReferenceEquals), typeof(object)), 1);
_ = nestedType.GetMethod(nestedMethodName);
}

[ExpectedWarning("IL2067", nameof(DataFlowTypeExtensions.RequiresPublicMethods))]
static void DeconstructConditionalTupleLiteralMismatch(bool condition, Type typeWithoutMethods)
{
(var type, _) = condition
? (typeof(string), 0)
: (typeWithoutMethods, 1);
type.RequiresPublicMethods();
}

[ExpectedWarning("IL2077", nameof(DataFlowTypeExtensions.RequiresPublicMethods))]
static void DeconstructSwitchTupleLiteral(bool condition)
{
(var type, _) = condition switch
{
true => (typeof(string), 0),
false => (typeof(object), 1)
};
type.RequiresPublicMethods();
}

// The swap correctly propagates the annotation from typeWithMethods to first (via second),
// so no warning is produced here.
static void DeconstructTupleSwapSuccess(
Expand Down Expand Up @@ -392,6 +425,9 @@ public static void Test()
DeconstructExtensionWithMismatchAnnotation(new());
DeconstructNestedTuple(((typeof(string), null), null));
DeconstructTupleLiteral(typeof(string));
DeconstructConditionalTupleLiteral(true);
DeconstructConditionalTupleLiteralMismatch(true, typeof(string));
DeconstructSwitchTupleLiteral(true);
DeconstructTupleSwapSuccess(typeof(string), typeof(string));
DeconstructTupleSwap(typeof(string), typeof(string));
DeconstructPropertyTargetSideEffect(typeof(string), typeof(string));
Expand Down