-
Notifications
You must be signed in to change notification settings - Fork 5.2k
[release/10.0] Add trim analyzer support for C# 14 extensions #119195
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/10.0] Add trim analyzer support for C# 14 extensions #119195
Conversation
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.
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 adds support for C# 14 extension members to the IL linker's trim analyzer to fix analyzer crashes when encountering these new language features. The implementation includes handling for extension parameters on types, operator overloads, and extension properties while maintaining compatibility with existing features.
Key changes:
- Enhanced parameter handling for extension members with parameters on types
- Added test coverage for extension member data flow analysis scenarios
- Updated Microsoft.CodeAnalysis dependency to support C# 14 features
Reviewed Changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| eng/Versions.props | Updates Microsoft.CodeAnalysis version to 4.14.0 for C# 14 support |
| src/tools/illink/src/ILLink.RoslynAnalyzer/IMethodSymbolExtensions.cs | Adds HasExtensionParameterOnType() method and updates parameter count calculations |
| src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/ParameterProxy.cs | Refactors constructor and property handling for extension parameters |
| src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/MethodParameterValue.cs | Removes obsolete constructors for cleaner parameter value creation |
| src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs | Updates parameter reference handling for extension members |
| src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/FlowAnnotations.cs | Prevents property-level DAM propagation for extension properties |
| src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs | Handles extension property access and method calls with extension parameters |
| src/tools/illink/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs | Skips DAM conflict checks for extension properties |
| src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/ResultChecker.cs | Adds workaround to skip inherited attributes on compiler-generated extension types |
| src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCasesRunner/ResultChecker.cs | Same workaround as above for NativeAOT tests |
| src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ExtensionsDataFlow.cs | New test file for basic extension method data flow scenarios |
| src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ExtensionMembersDataFlow.cs | New comprehensive test file for C# 14 extension member features |
| src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/MethodReturnParameterDataFlow.cs | Adds operator return value test cases |
| src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/MethodParametersDataFlow.cs | Adds operator parameter test cases |
| src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/TestChecker.cs | Adds operator declaration visitor support |
| src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/DataFlowTests.cs | Registers new test cases for execution |
| src/tools/illink/src/ILLink.RoslynAnalyzer/ILLink.RoslynAnalyzer.csproj | Removes conditional suppression of CS0436 warnings |
| Directory.Build.props | Adds IDE warning suppressions for redundant equality and unused parameters |
|
Tagging subscribers to this area: @dotnet/illink |
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.
approved. please get a code review. we can merge when ready
src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/MethodParametersDataFlow.cs
Outdated
Show resolved
Hide resolved
Porting feedback from #119195 to main.
Backport of #119017 to release/10.0
Customer Impact
Reported in #115949 and #118025 as an analyzer crash when analyzing code with C# 14 extension members.
A temporary fix was put in place to avoid the crash, but it did not include support for analyzing extension members. The impact is missing or unexpected warnings when adding trim annotations for extension members.
Regression
Testing
Added testcases to cover annotations for extension members.
Risk
Low. Analyzer-only fix. Note that it does include an update to Microsoft.CodeAnalysis, which needs to remain compatible with the version that ships with VS. Confirmed with @jkoritzinsky that this version is OK for .NET 10.