[release/11.0] Revert deconstruction analysis in trim analyzer by jtschuster · Pull Request #134484 · dotnet/runtime

jtschuster · 2026-09-23T00:06:31Z

#131624 implemented support for deconstruction and dataflow for TupleOperations (e.g. (a, b) = (b, a)). However, in some cases it made assumptions that Roslyn would lower a TupleOperation to a ValueTuple creation and ItemN field reference. This led to unexpected warnings like the one found in #133911.

#134377 offers one possible fix, but doesn't seem bulletproof and may introduce more regressions. A more "proper" fix would be an even larger change. The least risky "fix" is to revert the original PR and avoid creating bogus warnings at the cost of missing some valid warnings until publish time. However, a full revert reintroduces some bogus warnings (though they were present in .NET 10). Instead, we add very basic support to clear the dataflow information for locals that we see are assigned to in deconstruction assignments. In other words, the following snippet does not produce a bogus warning.

void Method(Type input)
{
    Type localType1 = input;
    Type localType2 = input;
    (localType, localType2) = GetTypesWithMethods();
    localType1.GetMethods(); // warns in .NET 10, does not warn in 11
    localType2.GetMethods(); // warns in .NET 10, does not warn in 11
}

This PR is broken into three commits:

Revert the initial PR
Re-add the tests from the initial PR, but with assertions changed to reflect the current behavior
Add minimal support for "clearing" dataflow values from local variables that are targets of deconstruction assignments.

Customer Impact

Customer reported
Found internally

Regression

Yes
No

Testing

Tests from the original PR were re-introduced with the assertions changed to reflect the behavior prior to the PR's product changes. Existing regression tests validate other scenarios are unaffected.

Risk

Low to medium. The diff is large, but it reverts behavior to .NET 10. There's a small possibility of regressions, but the likelihood and severity of regressions are less risky than the known regression in #133911.

…31624)" This reverts commit 778a989.

azure-pipelines · 2026-09-23T00:07:13Z

Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run.

dotnet-policy-service · 2026-09-23T00:07:46Z

Tagging subscribers to this area: @agocke, @dotnet/illink
See info in area-owners.md if you want to be subscribed.

…nments

Copilot

Copilot review overview

🟡 Changes recommended

One or more issues must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2 · 1

Open (3)

Avoid throwing from LocalStateAndContext.GetHashCode · New
Include CapturedReferences in local state equality · New
Preserve unwrapping of conversion and parenthesized targets · New

What changed in this PR

Reverts full deconstruction dataflow analysis while retaining conservative local clearing to avoid bogus trim warnings.

Changes:

Restores conservative conversion handling.
Removes complex deconstruction tracking and adds basic local invalidation.
Updates shared test expectations.

File	Description
src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ExtensionMembersDataFlow.cs	Updated as part of this pull request.
src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/DeconstructUserDefinedConversion.cs	Updated as part of this pull request.
src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/DeconstructFieldTarget.cs	Updated as part of this pull request.
src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ConstructedTypesDataFlow.cs	Updated as part of this pull request.
src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs	Updated as part of this pull request.
src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalStateLattice.cs	Updated as part of this pull request.
src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalStateAndContextLattice.cs	Updated as part of this pull request.
src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs	Updated as part of this pull request.

-
-        // Local dataflow states are mutable and should never be used as dictionary keys.
-        public override int GetHashCode() => throw new NotImplementedException();
+        public override int GetHashCode() => HashUtils.Combine(LocalState, Context);


-            Dictionary.Equals(other.Dictionary) &&
-            CapturedReferences.Equals(other.CapturedReferences) &&
-            CapturedTargetValues.Equals(other.CapturedTargetValues);
+        public bool Equals(LocalState<TValue> other) => Dictionary.Equals(other.Dictionary);


+                case IDeclarationExpressionOperation declaration:
+                    SetDeconstructedLocalsToTop(declaration.Expression, state);
+                    break;


Copilot

Copilot review overview

🟡 Changes recommended

Flow-captured locals are not cleared, and captured-reference changes are omitted from fixpoint equality.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 2 · 2 · 3

Open (7)

Include CapturedReferences in local state equality
Avoid throwing from LocalStateAndContext.GetHashCode
Flow-captured targets are not cleared · New
Preserve unwrapping of conversion and parenthesized targets
Use singular form after “a” · New
Test does not exercise flow-capture clearing · New
Comment references removed analyzer methods · New

+        private void SetDeconstructedLocalsToTop(IOperation target, LocalDataFlowState<TValue, TContext, TValueLattice, TContextLattice> state)
+        {
+            switch (target)
+            {
+                case ITupleOperation tuple:
+                    foreach (IOperation element in tuple.Elements)
+                        SetDeconstructedLocalsToTop(element, state);
+                    break;
+                case IDeclarationExpressionOperation declaration:
+                    SetDeconstructedLocalsToTop(declaration.Expression, state);
+                    break;
+                case ILocalReferenceOperation local:
+                    SetLocal(local.Local, TopValue, state);
+                    break;
+            }
+        }


-                // (IIncrementOrDecrementOperation) and coalescing assignment (ICoalesceAssignmentOperation)
-                // are not handled here and fall back to the base visitor, which visits the write target
-                // directly. Enabling the following assert requires handling those first.
+                // This can also happen for a deconstruction assignments, where the write is not to a byref.


            static (Type type, object instance) GetInput(Type type, int unused) => (type, null);

-            [ExpectedWarning("IL2077")]
+            [ExpectedWarning("IL2077", Tool.Trimmer | Tool.NativeAot, "https://github.com/dotnet/runtime/issues/123767")]


+            [ExpectedWarning("IL2026", nameof(GetIndexerHolder), Tool.Trimmer | Tool.NativeAot, "https://github.com/dotnet/runtime/issues/123767")]
+            [ExpectedWarning("IL2026", nameof(GetIndex), Tool.Trimmer | Tool.NativeAot, "https://github.com/dotnet/runtime/issues/123767")]


agocke

LGTM

The base branch was changed.

Revert "Add dataflow support for deconstruction assignments (dotnet#1…

24c1e0b

…31624)" This reverts commit 778a989.

jtschuster self-assigned this Sep 23, 2026

github-project-automation Bot added this to AppModel Sep 23, 2026

github-actions Bot added the area-Tools-ILLink .NET linker development as well as trimming analyzers label Sep 23, 2026

dotnet-policy-service Bot added the linkable-framework Issues associated with delivering a linker friendly framework label Sep 23, 2026

jtschuster added 2 commits September 22, 2026 22:20

Re-add test cases from reverted commit with updated assertions.

702b3f8

Add basic support for assigning Top to Locals in deconstruction assig…

5206ea7

…nments

jtschuster force-pushed the RevertDeconstructionAnalysis branch from ea6366a to 5206ea7 Compare September 23, 2026 05:20

jtschuster marked this pull request as ready for review September 23, 2026 16:38

jtschuster requested a review from sbomer as a code owner September 23, 2026 16:38

Copilot AI lite review requested due to automatic review settings September 23, 2026 16:38

Copilot started reviewing on behalf of jtschuster September 23, 2026 16:39 View session

jtschuster changed the title ~~Revert deconstruction analysis in trim analyzer~~ [release/11.0-rc2] Revert deconstruction analysis in trim analyzer Sep 23, 2026

Copilot AI reviewed Sep 23, 2026

View reviewed changes

agocke added the Servicing-approved Approved for servicing release label Sep 23, 2026

agocke requested a balanced review from Copilot September 23, 2026 17:11

Copilot started reviewing on behalf of agocke September 23, 2026 17:12 View session

Copilot AI reviewed Sep 23, 2026

View reviewed changes

agocke previously approved these changes Sep 23, 2026

View reviewed changes

jtschuster changed the base branch from release/11.0-rc2 to release/11.0 September 24, 2026 17:23

jtschuster changed the title ~~[release/11.0-rc2] Revert deconstruction analysis in trim analyzer~~ [release/11.0] Revert deconstruction analysis in trim analyzer Sep 24, 2026

Navigation Menu

[release/11.0] Revert deconstruction analysis in trim analyzer - #134484

[release/11.0] Revert deconstruction analysis in trim analyzer#134484
jtschuster wants to merge 3 commits into
dotnet:release/11.0from
jtschuster:RevertDeconstructionAnalysis

jtschuster commented Sep 23, 2026 •

edited

Loading

Uh oh!

azure-pipelines Bot commented Sep 23, 2026

Uh oh!

dotnet-policy-service Bot commented Sep 23, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

agocke left a comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		[ExpectedWarning("IL2026", nameof(GetIndexerHolder), Tool.Trimmer \| Tool.NativeAot, "https://github.com/dotnet/runtime/issues/123767")]
		[ExpectedWarning("IL2026", nameof(GetIndex), Tool.Trimmer \| Tool.NativeAot, "https://github.com/dotnet/runtime/issues/123767")]

Conversation

jtschuster commented Sep 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Customer Impact

Regression

Testing

Risk

Uh oh!

azure-pipelines Bot commented Sep 23, 2026

Uh oh!

dotnet-policy-service Bot commented Sep 23, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

agocke left a comment

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jtschuster commented Sep 23, 2026 •

edited

Loading