Add ILLink coverage for unused virtual method annotations - #130640
Add ILLink coverage for unused virtual method annotations#130640jtschuster with Copilot wants to merge 6 commits into
Conversation
Co-authored-by: jtschuster <36744439+jtschuster@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds a new ILLink test case under Mono.Linker.Tests.Cases intended to ensure virtual/interface annotation mismatch validation only happens for implementations that are actually retained, avoiding IL2046/IL2092 noise from unused implementations.
Changes:
- Introduces
UnusedVirtualMethodAnnotationstest coverage for four scenarios (fully unused impl, type-only reference, used impl must warn, used interface with unused sibling impl). - Uses
ExpectedWarningto pin IL2046/IL2092 to the retained implementations while suppressing IL2026 at the call sites.
Comments suppressed due to low confidence (2)
src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/UnusedVirtualMethodAnnotations.cs:49
- Same as UnusedImplementation: the Roslyn analyzers validate virtual/interface annotation mismatches independent of trimming reachability, so analyzer-only IL2046/IL2092 expectations are needed here to avoid unexpected warnings under the class-level [ExpectedNoWarnings].
class TypeOnlyImplementation : ITypeOnly
{
public void Method(Type type) { }
}
src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/UnusedVirtualMethodAnnotations.cs:82
- Because [ExpectedNoWarnings] is applied at the test-case level, any analyzer-emitted mismatches for unused sibling implementations will be treated as unexpected. Add analyzer-only IL2046/IL2092 expectations for this unused implementation so the test can still validate the trimmer-specific reachability behavior without breaking analyzer runs.
class UnusedImplementationOfPartiallyUsedInterface : IPartiallyUsed
{
public void Method(Type type) { }
}
…unused implementations Co-authored-by: jtschuster <36744439+jtschuster@users.noreply.github.com>
| [UnexpectedWarning("IL2046", Tool.Analyzer, "https://github.com/dotnet/runtime/pull/130640")] | ||
| [UnexpectedWarning("IL2092", Tool.Analyzer, "https://github.com/dotnet/runtime/pull/130640")] |
There was a problem hiding this comment.
@copilot Update the issue/reason string for the UnexpectedWarnings to say "Analyzer does not track reachability"
There was a problem hiding this comment.
Updated all 6 UnexpectedWarning reason strings to "Analyzer does not track reachability" in the latest commit.
…eachability' Co-authored-by: jtschuster <36744439+jtschuster@users.noreply.github.com>
|
|
||
| class UnusedImplementation : IUnused | ||
| { | ||
| [UnexpectedWarning("IL2046", Tool.Analyzer, "Analyzer does not track reachability")] |
There was a problem hiding this comment.
@copilot this should be ExpectedWarning for the analyzer warnings
There was a problem hiding this comment.
Changed all three sites from UnexpectedWarning to ExpectedWarning with Tool.Analyzer.
| public void Method(Type type) { } | ||
| } | ||
|
|
||
| [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] |
There was a problem hiding this comment.
Do the interfaces need DAM to repro this issue? I would leave this out (or at least add a simpler test without DAM on the interface).
|
|
||
| class UnusedImplementationOfPartiallyUsedInterface : IPartiallyUsed | ||
| { | ||
| [UnexpectedWarning("IL2046", Tool.Analyzer, "Analyzer does not track reachability")] |
There was a problem hiding this comment.
The testcase should demonstrate a scenario where ILLink produces unexpected warnings too
…IL2092 warnings Co-authored-by: sbomer <787361+sbomer@users.noreply.github.com>
ILLink should validate virtual annotation mismatches only for retained implementations, avoiding IL2046/IL2092 warnings from unused code.