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] Add missing [RequiresDynamicCode] attributes to System.Linq.Expressions API in ref assembly #107638

Merged
merged 8 commits into from
Sep 12, 2024

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Sep 10, 2024

Backport of #107580 to release/9.0

/cc @eerhardt @cston

Customer Impact

  • Customer reported
  • Found internally

When customers use the native AOT Roslyn Analyzer to find AOT incompatible code, they are not getting the warnings correctly for System.Linq.Expressions. This degrades the value of the Roslyn Analyzer.

By adding these attributes to the ref assemblies, this allows the Roslyn Analyzers to work correctly and raise warnings where customer code is calling AOT-incompatible APIs.

Regression

  • Yes
  • No

[If yes, specify when the regression was introduced. Provide the PR or commit if known.]

Testing

The change to Microsoft.Extensions.DependencyInjection in our repo was because the Roslyn Analyzer found a place where we were calling an incompatible API. This verifies that the ref assembly is correctly attributed.

All existing automated tests pass.

Risk

Low. The change to Linq.Expressions is only to put the matching attributes in the ref assembly from the implementation assembly. This only affects compilation and won't affect runtime behavior. The change to Microsoft.Extensions.DependencyInjection was verified by existing tests and is in code that is only used in netstandard2.0 builds, not in .NET Framework or .NET Core builds.

Copy link

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

Copy link

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

Copy link
Contributor

Tagging subscribers to this area: @cston
See info in area-owners.md if you want to be subscribed.

@eerhardt eerhardt added the Servicing-consider Issue for next servicing release review label Sep 10, 2024
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. we can merge when ready

@jeffschwMSFT jeffschwMSFT added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Sep 12, 2024
@jeffschwMSFT jeffschwMSFT merged commit 0db8219 into release/9.0 Sep 12, 2024
10 checks passed
@eerhardt eerhardt deleted the backport/pr-107580-to-release/9.0 branch September 12, 2024 17:54
@github-actions github-actions bot locked and limited conversation to collaborators Oct 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants