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

[release/9.0] Fix RunClassConstructor trim warning for Nullable<T> #107039

Merged
merged 3 commits into from
Sep 4, 2024

Conversation

sbomer
Copy link
Member

@sbomer sbomer commented Aug 27, 2024

Backport of #106975 to .NET 9.

Customer Impact

  • Customer reported

The trimming tools (ILLink, native AOT, and the trim analyzer) report warnings when RunClassConstructor is called for nullable value types. This is not problematic for trimming and should not warn.

From #106975 (comment):

The XAML compiler relies on the analyzer and ILC not emitting a warning here to make sure WinUI 3 apps will not produce trim warnings. Right now this will break everyone using WarnAsErrors and trying to build a WinUI 3 app with NativeAOT, and the workaround of suppressing the warning is not viable either as it will also hide real warnings from user code 😅

Regression

  • Yes
  • No

Testing

Added unit test that runs for all of the trimming tools.

Risk

Low. Fix is straightforward and limited in scope - it only adds an extra case in existing handling for the RunClassConstructor intrinsic.

- Fix test name
- Run it in ILC tests
- Check warnings in analyzer tests
@sbomer sbomer added the Servicing-consider Issue for next servicing release review label Aug 27, 2024
@sbomer sbomer added this to the 9.0.0 milestone Aug 27, 2024
@sbomer sbomer requested a review from agocke August 27, 2024 16:16
@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Aug 27, 2024
@agocke agocke removed the Servicing-consider Issue for next servicing release review label Aug 27, 2024
@teo-tsirpanis teo-tsirpanis added area-Tools-ILLink .NET linker development as well as trimming analyzers and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Sep 1, 2024
Copy link
Contributor

Tagging subscribers to this area: @agocke, @sbomer, @vitek-karas
See info in area-owners.md if you want to be subscribed.

Copy link
Member

@jeffschwMSFT jeffschwMSFT left a comment

Choose a reason for hiding this comment

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

approved. please get a code review. we can merge when ready

@jeffschwMSFT jeffschwMSFT added the Servicing-approved Approved for servicing release label Sep 3, 2024
@jeffschwMSFT
Copy link
Member

@sbomer can you take a look at the PR failures?

@sbomer
Copy link
Member Author

sbomer commented Sep 4, 2024

/ba-g "Failures are unrelated timeouts."

@github-actions github-actions bot locked and limited conversation to collaborators Oct 6, 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 Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants