-
Notifications
You must be signed in to change notification settings - Fork 5.3k
[release/8.0-staging] Fix analyzer crash on C# 14 extensions #122199
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
base: release/8.0-staging
Are you sure you want to change the base?
Conversation
Co-authored-by: sbomer <[email protected]>
src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: sbomer <[email protected]>
This branch will not be updated to support using C# 14 in tests.
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 backports a fix for an analyzer crash that occurs when the ILLink Roslyn analyzer encounters C# 14 extension members in a .NET 8.0 project. The crash was caused by an unsafe cast assumption that a parameter's ContainingSymbol is always an IMethodSymbol, which is false for extension member parameters where the container is an extension type.
Key Changes:
- Refactored parameter handling to require explicit
IMethodSymbolinstead of unsafe casting - Added early-exit logic to skip analysis of extension members (returning
TopValue) - Added test case to validate regular extension method analysis
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/MethodParameterValue.cs |
Removed constructors that performed unsafe casts of parameter.ContainingSymbol to IMethodSymbol |
src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/ParameterProxy.cs |
Modified constructor to require explicit IMethodSymbol parameter instead of casting internally |
src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs |
Added guard to check if parameter's containing symbol is a method before analysis; updated instance reference handling to use new constructor |
src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ExtensionsDataFlow.cs |
New test case validating analyzer behavior with extension methods (not C# 14 extension members) |
src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/DataFlowTests.cs |
Added test method to run the new ExtensionsDataFlow test case |
Backport of #121351 to release/8.0-staging
Customer Impact
Fixes a customer-reported crash in ILLink analyzer in a net8.0 project that uses C# 14 extension members.
Customer issue: #120728
Regression
No - crash only happens for language features introduced after .NET 8 was released.
Testing
Tested locally. We can't add automated testing because this repo doesn't reference a new enough version of Roslyn to build tests with extension members.
Risk
Low, analyzer-only fix, with limited scope.
Fixes #120728
main PR #121351
Description
Backports fix for analyzer crash when encountering C# 14 extension members. The crash occurred because the analyzer assumed
parameter.ContainingSymbolis alwaysIMethodSymbol, which is false for extension member parameters where the container is an extension type.Core analyzer changes:
MethodParameterValueconstructors that castparameter.ContainingSymboltoIMethodSymbolIMethodSymbolparameter inParameterProxyconstructorGetParameterTargetValue(returnTopValuewhenContainingSymbolis notIMethodSymbol)VisitInstanceReferenceto use newParameterProxyconstructor (without unrelated static method checks)Test infrastructure:
Microsoft.Net.Compilers.Toolsetpackage reference for C# preview featuresResultChecker.csto skip validation of compiler-generated extension member types (<>E__prefix)ExtensionsDataFlow.cstest (enabled)ExtensionMembersDataFlow.cstest (disabled until C# 14 available)Note: NativeAOT test changes from original PR excluded as
ILCompiler.Trimming.Testsdoesn't exist in 8.0.Customer Impact
Without this fix, the Roslyn analyzer crashes when analyzing code that uses or references C# 14 extension members, breaking the build or IDE experience.
Regression
No. This is a new feature compatibility fix for C# 14, not a regression from 8.0 functionality.
Testing
ExtensionsDataFlowtest validates fix for regular extension methodsRisk
Low. Changes are surgical and isolated to extension member handling:
Package authoring signed off?
N/A - Changes are internal to analyzer implementation, no public API surface modified.
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.