Add trim analyzer support for C# 14 extensions#119017
Conversation
|
Tagging subscribers to this area: @dotnet/illink |
There was a problem hiding this comment.
Pull Request Overview
This PR adds trim analyzer support for C# 14 extensions by implementing comprehensive handling of the new extension syntax introduced in C# 14. The key changes enable proper data flow analysis and warning generation for extension members while addressing compiler-generated artifacts.
- Adds test coverage for both traditional extension methods and new C# 14 extension members
- Updates the trim analysis infrastructure to handle extension parameters on types
- Implements workarounds for compiler-generated types that inherit attributes from extension members
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/ResultChecker.cs |
Adds workaround to skip attribute validation for compiler-generated extension member types |
src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ExtensionsDataFlow.cs |
New test case for traditional extension method data flow analysis |
src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ExtensionMembersDataFlow.cs |
New test case for C# 14 extension members data flow analysis |
src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/DataFlowTests.cs |
Adds test methods for the new extension data flow test cases |
src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs |
Updates parameter handling to support extension parameters on types |
src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/ParameterProxy.cs |
Enhances parameter proxy to handle extension parameters with proper indexing |
src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/MethodParameterValue.cs |
Removes obsolete constructors for method parameter values |
src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/FlowAnnotations.cs |
Updates parameter annotation retrieval for extension methods |
src/tools/illink/src/ILLink.RoslynAnalyzer/IMethodSymbolExtensions.cs |
Adds extension parameter detection and parameter counting logic |
src/tools/illink/src/ILLink.RoslynAnalyzer/ILLink.RoslynAnalyzer.csproj |
Reorders NoWarn directives and removes conditional compilation warning suppression |
src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs |
Updates method and property call handling for extension parameters |
src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCasesRunner/ResultChecker.cs |
Duplicates the compiler-generated type workaround for NativeAOT testing |
eng/Versions.props |
Updates Microsoft.CodeAnalysis version from 4.8.0 to 4.14.0 |
src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/ParameterProxy.cs
Show resolved
Hide resolved
src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs
Show resolved
Hide resolved
src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ExtensionMembersDataFlow.cs
Show resolved
Hide resolved
src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ExtensionMembersDataFlow.cs
Outdated
Show resolved
Hide resolved
src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ExtensionMembersDataFlow.cs
Show resolved
Hide resolved
Including extension operators, and plain operators.
|
Will this need backport to .NET 10? |
|
@jkoritzinsky does the version update look OK to you? |
jkoritzinsky
left a comment
There was a problem hiding this comment.
Version update looks good!
jtschuster
left a comment
There was a problem hiding this comment.
Good catch on the operators!
|
/backport to release/10.0 |
|
Started backporting to release/10.0: https://github.com/dotnet/runtime/actions/runs/17307689788 |
|
@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 ExtensionsDataFlow test
Applying: Add support for extension members
Applying: Add property tests and arguments fix
Applying: Fix ILCompiler.Trimming.Tests infra
Applying: Update MicrosoftCodeAnalysisVersion_LatestVS
Applying: Update MicrosoftCodeAnalysisVersion_LatestVS
Using index info to reconstruct a base tree...
M eng/Versions.props
Falling back to patching base and 3-way merge...
No changes -- Patch already applied.
Applying: Reorganize tests, fix setter arguments
Applying: Don't propagate extension property annotations
Applying: Avoid conflict warning for DAM on property
Applying: Add coverage for operators
Applying: Add comment about property annotations
Applying: Silence warnings
Using index info to reconstruct a base tree...
M src/tools/illink/src/ILLink.RoslynAnalyzer/ILLink.RoslynAnalyzer.csproj
Falling back to patching base and 3-way merge...
Auto-merging src/tools/illink/src/ILLink.RoslynAnalyzer/ILLink.RoslynAnalyzer.csproj
CONFLICT (content): Merge conflict in src/tools/illink/src/ILLink.RoslynAnalyzer/ILLink.RoslynAnalyzer.csproj
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 0012 Silence warnings
Error: The process '/usr/bin/git' failed with exit code 128Please backport manually! |
Fixes dotnet#115949 This adds Roslyn analyzer support for C# 14 extensions. A few notes on the behavior: - DAM is supported on method return/params, including on property accessor return/param. - DAM is not supported on extension properties (see dotnet#119113) - DAM is not supported on extension methods (this produces IL2041) - There's still a bug with the behavior for ILLink/ILC due to the annotations not being preserved into the implementation when lowering: dotnet/roslyn#80017. Includes an update to Microsoft.CodeAnalysis for new analyzer APIs involving extension members. dotnet#119159 tracks fixing new warnings that are suppressed for now. New tests uncovered an analyzer issue with operators in the (extension or not): dotnet#119110. This change includes test coverage for non-extension operators. They also uncovered an issue that looks like a race condition in ILC: dotnet#119155.
|
It seems this commit is causing local builds to fail with |
I haven't been able to reproduce this (tried on a couple machines). If anyone else hits this please let me know. Repro steps, run outside of VS (confirmed with @pentp): |
* Add trim analyzer support for C# 14 extensions (#119017) Fixes #115949 This adds Roslyn analyzer support for C# 14 extensions. A few notes on the behavior: - DAM is supported on method return/params, including on property accessor return/param. - DAM is not supported on extension properties (see #119113) - DAM is not supported on extension methods (this produces IL2041) - There's still a bug with the behavior for ILLink/ILC due to the annotations not being preserved into the implementation when lowering: dotnet/roslyn#80017. Includes an update to Microsoft.CodeAnalysis for new analyzer APIs involving extension members. #119159 tracks fixing new warnings that are suppressed for now. New tests uncovered an analyzer issue with operators in the (extension or not): #119110. This change includes test coverage for non-extension operators. They also uncovered an issue that looks like a race condition in ILC: #119155. * Fix indentation
|
Looks like #119391 (comment) is a potential explanation for the inconsistent repro. |
Fixes #115949
This adds Roslyn analyzer support for C# 14 extensions. A few notes on the behavior:
Includes an update to Microsoft.CodeAnalysis for new analyzer APIs involving extension members. #119159 tracks fixing new warnings that are suppressed for now.
New tests uncovered an analyzer issue with operators in the (extension or not): #119110. This change includes test coverage for non-extension operators.
They also uncovered an issue that looks like a race condition in ILC: #119155.