Uh oh!
There was an error while loading. Please reload this page.
Add trimming intrinsic for EventInfo.EventHandlerType.GetMethod("Invoke") - #116931
Conversation
Tagging subscribers to this area: @dotnet/illink |
There was a problem hiding this comment.
Pull Request Overview
This PR prevents spurious trimming warnings when calling EventInfo.EventHandlerType.GetMethod("Invoke") on delegates by adding special-case handling in the dataflow logic, and adds a test covering both delegate and custom-EventInfo scenarios.
- Added a new test case
EventHanderTypeGetInvokeMethodto cover the delegate and non-delegate paths. - Updated
HandleCallActionto recognize and allowInvokeon delegate types without additional requirements. - Refactored several
MethodReturnValueimplementations to carry both the original method proxy and its definition.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/EventHanderTypeGetInvokeMethod.cs | New test exercising delegate and custom EventInfo Invoke scenarios |
| src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/ReflectionTests.cs | Registered new analyzer test for the above case |
| src/tools/illink/src/linker/Linker.Dataflow/MethodReturnValue.cs | Switched to carrying a MethodProxy and a MethodDefinition |
| src/tools/illink/src/ILLink.Shared/TrimAnalysis/MethodReturnValue.cs | Added Method property to shared trim analysis value |
| src/tools/illink/src/ILLink.Shared/TrimAnalysis/HandleCallAction.cs | Special-case logic for GetMethod("Invoke") on delegates |
| src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/MethodReturnValue.cs | Updated Roslyn analyzer to construct MethodProxy |
| src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/MethodReturnValue.cs | Updated AOT dataflow to construct MethodProxy and separate MethodDesc |
Comments suppressed due to low confidence (5)
src/tools/illink/src/linker/Linker.Dataflow/MethodReturnValue.cs:34
- The constructor assigns to
Methodbut noMethodfield or property is declared in this class. Add apublic readonly MethodProxy Method;(or auto-property) to match the assignment and avoid a compile error.
Method = method;
src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/MethodReturnValue.cs:25
- This line assigns to
Methodbut the class does not declare aMethodfield or property. Introduce apublic MethodProxy Method { get; }to store the proxy and ensure the code compiles.
Method = new MethodProxy (methodSymbol);
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/MethodReturnValue.cs:26
- The constructor sets
Methodbut no such member exists. Add apublic MethodProxy Method { get; }or similar declaration so the code compiles and can supply the proxy where needed.
Method = new MethodProxy(method);
src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/EventHanderTypeGetInvokeMethod.cs:14
- [nitpick] The class name
EventHanderTypeGetInvokeMethodis missing an 'l' inHandler. Consider renaming toEventHandlerTypeGetInvokeMethodfor clarity and consistency.
public class EventHanderTypeGetInvokeMethod
src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/ReflectionTests.cs:38
- [nitpick] The test method name
EventHanderTypeGetInvokeMethodis missing an 'l' inHandler. Update it toEventHandlerTypeGetInvokeMethodto match the corrected class name.
public Task EventHanderTypeGetInvokeMethod ()
Fixes#114113
Because ILLink and ILC keep the
Invokemethod on delegates, there shouldn't be a warning for the common pattern ofEventInfo.EventHandlerType.GetMethod("Invoke").It's possible to break this logic by using a custom
EventInfo- see the testcase and discussion in the original issue.