From 928c07dfb364241fe1ab380c8e41c6ed29cd7089 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 15 Sep 2026 17:57:37 +0000 Subject: [PATCH 1/2] Initial plan From b8840906a7d6312836791396c87f2d16a0e3730f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 15 Sep 2026 18:48:14 +0000 Subject: [PATCH 2/2] Preserve tuple flow through conditional deconstruction Co-authored-by: jtschuster <36744439+jtschuster@users.noreply.github.com> --- .../DataFlow/LocalDataFlowVisitor.cs | 193 +++++++++++++++++- .../DataFlow/LocalStateLattice.cs | 29 ++- .../DataFlow/ConstructedTypesDataFlow.cs | 36 ++++ 3 files changed, 245 insertions(+), 13 deletions(-) diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs index d0624a134d28eb..f9a422090c9124 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs @@ -56,6 +56,16 @@ public abstract class LocalDataFlowVisitor _deconstructionLValueFlowCaptures; + private readonly ImmutableHashSet _conditionalDeconstructionFlowCaptures; + + private readonly Dictionary> _tupleFlowCaptureSources = new(); + + private CaptureId? _tupleFlowCaptureId; + + private ImmutableArray _tupleFlowCapturePath; + + private ITupleOperation? _tupleFlowCaptureOperation; + public InterproceduralState InterproceduralState; private bool IsLValueFlowCapture(CaptureId captureId) @@ -80,14 +90,58 @@ public LocalDataFlowVisitor( _semanticModel = cfg.OriginalOperation.SemanticModel ?? compilation.GetSemanticModel(cfg.OriginalOperation.Syntax.SyntaxTree); this.lValueFlowCaptures = lValueFlowCaptures; - _deconstructionLValueFlowCaptures = cfg + ImmutableArray flowCaptureReferences = cfg .DescendantOperations(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 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 elementPath = _tupleFlowCapturePath.Add(i); + ImmutableArray 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 localContextState); public TConditionValue? Transfer( @@ -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 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). @@ -652,7 +714,8 @@ public override TValue VisitDeconstructionAssignment( sourceValueIsKnown, deconstructionInfo, operation, - state); + state, + sourceTupleCapture); if (deconstructionValue.DoesNotReturn) { state.Current = LocalStateAndContextLattice.Top; @@ -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 _path; + + private readonly ImmutableArray _sources; + + public bool HasValue => !_sources.IsDefaultOrEmpty; + + public TupleFlowCapture(CaptureId captureId, ImmutableArray sources) + : this(captureId, ImmutableArray.Empty, sources) + { + } + + private TupleFlowCapture( + CaptureId captureId, + ImmutableArray path, + ImmutableArray 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(_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, @@ -716,7 +831,8 @@ private DeconstructionValue EvaluateDeconstruction( bool sourceValueIsKnown, DeconstructionInfo deconstructionInfo, IDeconstructionAssignmentOperation operation, - LocalDataFlowState state) + LocalDataFlowState state, + TupleFlowCapture sourceTupleCapture) { target = UnwrapDeconstructionTarget(target); @@ -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); @@ -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); @@ -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); @@ -859,6 +983,31 @@ private DeconstructionValue EvaluateDeconstruction( return new DeconstructionValue(tupleValues.MoveToImmutable()); } + private ImmutableArray GetTupleFlowCaptureSources(CaptureId captureId) + { + if (_tupleFlowCaptureSources.TryGetValue(captureId, out ImmutableArray sources)) + return sources; + + var builder = ImmutableArray.CreateBuilder(); + foreach (IFlowCaptureOperation flowCapture in ControlFlowGraph.DescendantOperations(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, @@ -1182,7 +1331,7 @@ public override TValue VisitFlowCapture(IFlowCaptureOperation operation, LocalDa } else { - capturedValue = Visit(operation.Value, state); + capturedValue = VisitFlowCaptureValue(operation, state); } state.Set(new LocalKey(operation.Id), capturedValue); @@ -1190,6 +1339,34 @@ public override TValue VisitFlowCapture(IFlowCaptureOperation operation, LocalDa } } + private TValue VisitFlowCaptureValue( + IFlowCaptureOperation operation, + LocalDataFlowState state) + { + if (!_conditionalDeconstructionFlowCaptures.Contains(operation.Id) || + UnwrapDeconstructionSource(operation.Value) is not ITupleOperation) + { + return Visit(operation.Value, state); + } + + CaptureId? previousCaptureId = _tupleFlowCaptureId; + ImmutableArray previousPath = _tupleFlowCapturePath; + ITupleOperation? previousOperation = _tupleFlowCaptureOperation; + _tupleFlowCaptureId = operation.Id; + _tupleFlowCapturePath = ImmutableArray.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 state) { Visit(operation.Operation, state); diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalStateLattice.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalStateLattice.cs index cafdf95b461850..d4f9f0c29c6c62 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalStateLattice.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalStateLattice.cs @@ -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; @@ -15,23 +17,40 @@ namespace ILLink.RoslynAnalyzer.DataFlow private readonly CaptureId? CaptureId; - public LocalKey(ILocalSymbol symbol) => (Local, CaptureId) = (symbol, null); + private readonly ImmutableArray 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 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()}"; } } diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ConstructedTypesDataFlow.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ConstructedTypesDataFlow.cs index e7a86b1350620c..316e5c0e8b1584 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ConstructedTypesDataFlow.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ConstructedTypesDataFlow.cs @@ -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( @@ -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));