-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Don't warn if we can't find the backing field in trim tools #119820
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
Don't warn if we can't find the backing field in trim tools #119820
Conversation
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/FlowAnnotations.cs
Outdated
Show resolved
Hide resolved
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/FlowAnnotations.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR addresses a warning issue in trim tools where IL2042 warnings were incorrectly generated when backing fields of auto properties couldn't be found, particularly in illink->ilc scenarios when auto properties were stubbed out. Instead of warning, the tools now simply skip propagating the DAM annotation when the backing field cannot be located.
Key Changes
- Remove IL2042 warning generation when backing fields of auto properties cannot be found
- Simplify backing field detection logic to silently handle missing fields
- Update Roslyn analyzer to only check for auto properties when field has an associated property
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| PropertyDataFlow.cs | Remove expected IL2042 warnings from test cases since these warnings are no longer generated |
| FlowAnnotations.cs (illink) | Simplify backing field logic to skip warning when field not found and only propagate annotation when valid backing field exists |
| DiagnosticId.cs | Mark IL2042 diagnostic ID as unused by prefixing with "unused_" |
| FlowAnnotations.cs (analyzer) | Optimize property checking by combining auto property check with property symbol check |
| FlowAnnotations.cs (ilcompiler) | Apply same backing field logic simplification as illink version |
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/FlowAnnotations.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/FlowAnnotations.cs
Outdated
Show resolved
Hide resolved
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/FlowAnnotations.cs
Show resolved
Hide resolved
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/FlowAnnotations.cs
Outdated
Show resolved
Hide resolved
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/FlowAnnotations.cs
Outdated
Show resolved
Hide resolved
…ere is a single valid field
sbomer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you!
agocke
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/PropertyDataFlow.cs
Outdated
Show resolved
Hide resolved
src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/PropertyDataFlow.cs
Outdated
Show resolved
Hide resolved
|
/ba-g unknown failures were crashes in the infra |
Fixes #119780
We added a warning when we cannot find the backing field of an auto property in #119329 which caused some issues in illink->ilc scenarios when an auto property was stubbed out. Instead, we should just not propagate the DAM annotation.