Skip to content
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.

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

Support assignment to multiple refs in trim analyzer #90287

Merged
merged 4 commits into from
Aug 17, 2023

Conversation

sbomer
Copy link
Member

@sbomer sbomer commented Aug 10, 2023

Fixes dotnet/linker#3046 by adding analyzer support for assignments to multiple refs, for example:

(b ? ref f1 : ref f2) = v;

This also includes support for assignments by reference to array elements:

(b ? ref a1[0] : ref a2[0]) = v;

ILLink and ILCompiler produce a different warning in this case (stdin.ref results in the array values being replaced with UnknownValue). I attempted to quirk this in the analyzer, but this caused more problems in existing tests because it was difficult to make the quirk specific enough.

Also fixes assignment to multiple arrays on the LHS (this appears to be an analysis hole in ILLink and ILCompiler):

(b ? a1 : a2)[0] = v;

@sbomer sbomer requested review from agocke and vitek-karas August 10, 2023 00:43
@sbomer sbomer requested a review from marek-safar as a code owner August 10, 2023 00:43
@ghost ghost added the linkable-framework Issues associated with delivering a linker friendly framework label Aug 10, 2023
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-Tools-ILLink .NET linker development as well as trimming analyzers label Aug 10, 2023
@ghost ghost assigned sbomer Aug 10, 2023
@ghost
Copy link

ghost commented Aug 10, 2023

Tagging subscribers to 'linkable-framework': @eerhardt, @vitek-karas, @LakshanF, @sbomer, @joperezr, @marek-safar
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes dotnet/linker#3046 by adding analyzer support for assignments to multiple refs, for example:

(b ? ref f1 : ref f2) = v;

This also includes support for assignments by reference to array elements:

(b ? ref a1[0] : ref a2[0]) = v;

ILLink and ILCompiler produce a different warning in this case (stdin.ref results in the array values being replaced with UnknownValue). I attempted to quirk this in the analyzer, but this caused more problems in existing tests because it was difficult to make the quirk specific enough.

Also fixes assignment to multiple arrays on the LHS (this appears to be an analysis hole in ILLink and ILCompiler):

(b ? a1 : a2)[0] = v;
Author: sbomer
Assignees: -
Labels:

linkable-framework, area-Tools-ILLink

Milestone: -

Copy link
Member

@agocke agocke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM aside from test suggestion

@sbomer sbomer merged commit 92786fd into dotnet:main Aug 17, 2023
@sbomer
Copy link
Member Author

sbomer commented Aug 22, 2023

/backport to release/8.0

@github-actions
Copy link
Contributor

Started backporting to release/8.0: https://github.com/dotnet/runtime/actions/runs/5941987950

@ghost ghost locked as resolved and limited conversation to collaborators Sep 21, 2023
@sbomer sbomer deleted the refFieldAnalyzerCrash branch November 3, 2023 18:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Tools-ILLink .NET linker development as well as trimming analyzers linkable-framework Issues associated with delivering a linker friendly framework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Analyzer fails with System.InvalidOperationException
2 participants