Skip to content

Conversation

sbomer
Copy link
Member

@sbomer sbomer commented Sep 23, 2025

Fixes #119113 by adding a new warning that is shown by the Roslyn analyzer when attempting to annotate an extension property with DynamicallyAccessedMembersAttribute.

The warning is also produced by ILLink and ILC, but only when the compiler-generated extension metadata type (which contains the property in IL) is referenced, for example via reflection or when the assembly is rooted. So in practice this will be caught just by the analyzer.

I considered reusing IL2041 ("The 'DynamicallyAccessedMembersAttribute' is not allowed on methods. It is allowed on method return value or method parameters") but thought it made sense to introduce a new warning code with a message specific to properties.

See comments in the code that explain some nuances of the test approach (we need a way to assert ExpectedWarnings on the extension properties, but not the compiler-generated methods in the extension metadata type).

Testing this uncovered a test bug which was hiding #120004.

@sbomer sbomer requested a review from a team September 23, 2025 16:55
@Copilot Copilot AI review requested due to automatic review settings September 23, 2025 16:55
@github-actions github-actions bot added the area-Tools-ILLink .NET linker development as well as trimming analyzers label Sep 23, 2025
Copy link
Contributor

@Copilot Copilot AI left a 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 adds a warning for extension properties annotated with DynamicallyAccessedMembersAttribute. The change introduces a new diagnostic (IL2127) that warns users when attempting to use this attribute on extension properties, which are not supported by the trimming tools.

Key changes:

  • Adds a new IL2127 warning for extension properties with DynamicallyAccessedMembersAttribute
  • Updates test infrastructure to support rooting entire assemblies via a new attribute
  • Fixes test issues that were previously hidden by infrastructure bugs

Reviewed Changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/tools/illink/src/ILLink.Shared/DiagnosticId.cs Adds new diagnostic ID IL2127 for extension property warning
src/tools/illink/src/ILLink.Shared/SharedStrings.resx Adds warning message text for the new diagnostic
src/tools/illink/src/ILLink.Shared/DataFlow/CompilerGeneratedNames.cs Adds helper methods to identify extension types
src/tools/illink/src/linker/Linker.Dataflow/FlowAnnotations.cs Implements the warning logic for ILLink
src/tools/illink/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs Implements the warning logic for Roslyn analyzer
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/FlowAnnotations.cs Implements the warning logic for ILC
src/tools/illink/test/Mono.Linker.Tests.Cases.Expectations/Metadata/SetupRootEntireAssemblyAttribute.cs New test attribute for rooting entire assemblies
src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/TestCaseMetadataProvider.cs Implements support for the new test attribute in ILLink
src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCasesRunner/*.cs Implements support for the new test attribute in ILC
src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/ResultChecker.cs Updates test verification logic for extension properties
src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ExtensionMembersDataFlow.cs Updates test with new expected warnings
src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/AttributePropertyDataflow.cs Adds expected warnings for NativeAOT test bug
src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresInRootAllAssembly.cs Updates to use new test attribute
src/tools/illink/test/Mono.Linker.Tests.Cases/Metadata/RootAllAssemblyNamesAreKept.cs Updates to use new test attribute

@dotnet-policy-service dotnet-policy-service bot added the linkable-framework Issues associated with delivering a linker friendly framework label Sep 23, 2025
…adata/SetupRootEntireAssemblyAttribute.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@sbomer sbomer enabled auto-merge (squash) September 24, 2025 17:45
@sbomer
Copy link
Member Author

sbomer commented Sep 25, 2025

/ba-g "unrelated timeout"

@sbomer sbomer merged commit 9eb989c into dotnet:main Sep 25, 2025
115 of 117 checks passed
@sbomer
Copy link
Member Author

sbomer commented Sep 25, 2025

/backport to release/10.0

Copy link
Contributor

Started backporting to release/10.0: https://github.com/dotnet/runtime/actions/runs/18021392652

Copy link
Contributor

@sbomer backporting to "release/10.0" failed, the patch most likely resulted in conflicts:

$ git am --3way --empty=keep --ignore-whitespace --keep-non-patch changes.patch

Applying: Add warning for annotated extension property
Using index info to reconstruct a base tree...
M	src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/FlowAnnotations.cs
M	src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCasesRunner/ResultChecker.cs
M	src/tools/illink/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs
M	src/tools/illink/src/ILLink.Shared/DataFlow/CompilerGeneratedNames.cs
M	src/tools/illink/src/linker/Linker.Dataflow/FlowAnnotations.cs
Falling back to patching base and 3-way merge...
Auto-merging src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/FlowAnnotations.cs
Auto-merging src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCasesRunner/ResultChecker.cs
CONFLICT (content): Merge conflict in src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCasesRunner/ResultChecker.cs
Auto-merging src/tools/illink/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs
Auto-merging src/tools/illink/src/ILLink.Shared/DataFlow/CompilerGeneratedNames.cs
CONFLICT (content): Merge conflict in src/tools/illink/src/ILLink.Shared/DataFlow/CompilerGeneratedNames.cs
Auto-merging src/tools/illink/src/linker/Linker.Dataflow/FlowAnnotations.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0001 Add warning for annotated extension property
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

sbomer added a commit to sbomer/runtime that referenced this pull request Sep 25, 2025
Fixes dotnet#119113 by adding a new
warning that is shown by the Roslyn analyzer when attempting to
annotate an extension property with
DynamicallyAccessedMembersAttribute.

The warning is also produced by ILLink and ILC, but only when the
compiler-generated extension metadata type (which contains the
property in IL) is referenced, for example via reflection or when the
assembly is rooted. So in practice this will be caught just by the
analyzer.

See comments in the code that explain some nuances of the test
approach (we need a way to assert `ExpectedWarning`s on the extension
properties, but not the compiler-generated methods in the extension
metadata type).

Testing this uncovered a test bug which was hiding
dotnet#120004.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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.

DAM on extension properties is silently ignored

3 participants