Skip to content

Conversation

sbomer
Copy link
Member

@sbomer sbomer commented Aug 22, 2025

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.

@Copilot Copilot AI review requested due to automatic review settings August 22, 2025 18:17
@sbomer sbomer requested a review from marek-safar as a code owner August 22, 2025 18:17
@github-actions github-actions bot added the area-Tools-ILLink .NET linker development as well as trimming analyzers label Aug 22, 2025
@sbomer sbomer requested a review from a team August 22, 2025 18:17
@dotnet-policy-service dotnet-policy-service bot added the linkable-framework Issues associated with delivering a linker friendly framework label Aug 22, 2025
Copy link
Contributor

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

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 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

Copy link
Member

@jtschuster jtschuster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

Including extension operators, and plain operators.
@tannergooding
Copy link
Member

Will this need backport to .NET 10?

@sbomer
Copy link
Member Author

sbomer commented Aug 27, 2025

@jkoritzinsky does the version update look OK to you?

Copy link
Member

@jkoritzinsky jkoritzinsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Version update looks good!

Copy link
Member

@jtschuster jtschuster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch on the operators!

@sbomer sbomer merged commit 7eacd69 into dotnet:main Aug 28, 2025
164 checks passed
@sbomer
Copy link
Member Author

sbomer commented Aug 28, 2025

/backport to release/10.0

Copy link
Contributor

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

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 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 128

Please backport manually!

sbomer added a commit to sbomer/runtime that referenced this pull request Aug 28, 2025
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.
@pentp
Copy link
Contributor

pentp commented Aug 29, 2025

It seems this commit is causing local builds to fail with error IDE0031: Null check can be simplified (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0031) [C:\dev\runtime\src\tools\illink\src\ILLink.RoslynAnalyzer\ILLink.RoslynAnalyzer.csproj

@sbomer
Copy link
Member Author

sbomer commented Aug 29, 2025

error IDE0031

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):

> git checkout 7eacd692219e1f5eefc143a2dbb3d342f9978c03
> git clean -xfd
> .\build.cmd -s tools.illink

jeffschwMSFT pushed a commit that referenced this pull request Sep 1, 2025
* 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
@sbomer
Copy link
Member Author

sbomer commented Sep 15, 2025

Looks like #119391 (comment) is a potential explanation for the inconsistent repro.

@github-actions github-actions bot locked and limited conversation to collaborators Oct 16, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

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.

System.InvalidCastException in DynamicallyAccessedMembersAnalyzer when using C# 14 extensions

5 participants