Skip to content

[ILLink analyzer] Move generic parameter analysis to dataflow#95482

Merged
sbomer merged 5 commits intodotnet:mainfrom
sbomer:genericAnalyzerFix
Dec 21, 2023
Merged

[ILLink analyzer] Move generic parameter analysis to dataflow#95482
sbomer merged 5 commits intodotnet:mainfrom
sbomer:genericAnalyzerFix

Conversation

@sbomer
Copy link
Member

@sbomer sbomer commented Nov 30, 2023

And align behavior more closely with NativeAot. I tried to roughly follow the implementation approach used by NativeAot in GenericArgumentDataFlow.cs to make it easier to share the logic (without actually sharing the code for now).

Fixes #95121.

Some things to note:

  • Gets rid of some analyzer warnings in cases where NativeAot doesn't warn either
  • Doesn't warn for generics in typeof
  • Doesn't warn for generics in signatures of reflectable members (ILC does this to work around an incorrect suppression in DI, as discussed in NativeAOT Data flow annotations are not applied to DI created generic types #81358).
  • Generics in base types or interface types are still analyzed outside of dataflow

And align behavior more closely with NativeAot
@sbomer sbomer requested a review from vitek-karas November 30, 2023 20:33
@sbomer sbomer requested a review from marek-safar as a code owner November 30, 2023 20:33
@ghost ghost added linkable-framework Issues associated with delivering a linker friendly framework area-Tools-ILLink .NET linker development as well as trimming analyzers labels Nov 30, 2023
@ghost ghost assigned sbomer Nov 30, 2023
@ghost
Copy link

ghost commented Nov 30, 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

And align behavior more closely with NativeAot.

Fixes #95121.

Some things to note:

  • Gets rid of some analyzer warnings in cases where NativeAot doesn't warn either
  • Doesn't warn for generics in typeof
  • Doesn't warn for generics in signatures of reflectable members (ILC does this to work around an incorrect suppression in DI, as discussed in NativeAOT Data flow annotations are not applied to DI created generic types #81358).
  • Generics in base types or interface types are still analyzed outside of dataflow
Author: sbomer
Assignees: -
Labels:

linkable-framework

Milestone: -

Copy link
Member

@vitek-karas vitek-karas left a comment

Choose a reason for hiding this comment

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

Nice!

@sbomer sbomer merged commit 0cf461b into dotnet:main Dec 21, 2023
@ShreyasJejurkar
Copy link
Contributor

This is great. I just had one question can't we share the trimming logic between the trimmer, NativeAOT, and the rest of the app models where we do trimming? This is hard to do or is it technically impossible to do so?

@sbomer
Copy link
Member Author

sbomer commented Dec 22, 2023

We've been making progress on sharing some of the code - see https://github.com/dotnet/runtime/tree/main/src/tools/illink/src/ILLink.Shared. The three tools use three different type systems, so a big part of the sharing effort is using wrapper types that hide some of the type system differences.

@vitek-karas
Copy link
Member

The additional complication is that trimmer (illink) and NativeAOT see IL, while the analyzer sees the Roslyn's operations/CFG. We could probably share more if(when) we implement CFG reconstruction from IL in trimmer and NativeAOT. But currently the two approaches have to do some things differently.

That said, we already share quite a lot of the core data flow logic, and almost all of the tests.

@github-actions github-actions bot locked and limited conversation to collaborators Feb 3, 2024
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.

Trim analyzer: Doesn't produce warnings due to annotated generic parameter in various situations

3 participants