[tools] Mark the Invoke methods for the type to create block proxies when using the static registrar.#24344
Conversation
…when using the static registrar.
Add a custom linker step to mark the Invoke method for the type pointed to by
the DelegateProxy attribute, because this method may only be referenced from
the native code the static registrar generates.
Fixes the following test failure for the "Release (all optimizations)" test variation:
MonoTouchFixtures.ObjCRuntime.RegistrarTest
[FAIL] BlockReturnTest : ObjCRuntime.RuntimeException : Invalid DelegateProxyAttribute for the return value for the method MonoTouchFixtures.ObjCRuntime.RegistrarTest+BlockReturnTestClass.MethodReturningBlock: DelegateType (ObjCRuntime.Trampolines+SDRegistrarTestBlock) specifies a type without a 'Handler' field. Please file a bug report at https://github.com/dotnet/macios/issues/new.
at ObjCRuntime.BlockLiteral.GetBlockForDelegate(MethodInfo , Object , UInt32 , String )
at ObjCRuntime.Runtime.CreateDelegateProxy(IntPtr , IntPtr , IntPtr , UInt32 )
at ObjCRuntime.Runtime.create_delegate_proxy(IntPtr method, IntPtr block, IntPtr signature, UInt32 token_ref, IntPtr* exception_gchandle)
--- End of stack trace from previous location ---
at ObjCRuntime.Runtime.RethrowManagedException(IntPtr )
at ObjCRuntime.Runtime.rethrow_managed_exception(IntPtr original_exception_gchandle, IntPtr* exception_gchandle)
at Bindings.Test.ObjCRegistrarTest.TestBlocks()
at MonoTouchFixtures.ObjCRuntime.RegistrarTest.BlockReturnTest()
at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object , BindingFlags )
There was a problem hiding this comment.
Pull request overview
This PR adds a custom linker step to preserve Invoke methods in delegate proxy types when using the static registrar with block literal optimizations. This prevents the linker from removing methods that are only referenced from native code generated by the static registrar, fixing a test failure in the "Release (all optimizations)" test variation where the Invoke method was being trimmed away.
Key changes:
- Added a new
MarkForStaticRegistrarlinker step to mark delegate proxyInvokemethods for preservation - Refactored
GetDelegateProxyTypeto work without anObjCMethodinstance for use from the linker - Added error handling and a new error code (MX8060) for missing
Invokemethods
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/dotnet-linker/MarkForStaticRegistrar.cs | New linker step to mark delegate proxy Invoke methods for preservation when using the static registrar |
| tools/dotnet-linker/Steps/MarkDispatcher.cs | Integrates the new MarkForStaticRegistrar step into the linker pipeline |
| tools/common/StaticRegistrar.cs | Refactors GetDelegateProxyType to support calling without an ObjCMethod instance; adds CollectAllProtocolsInHierarchy helper; makes GetExportAttribute public for linker access |
| src/ObjCRuntime/Registrar.cs | Changes GetExportAttribute(TMethod) visibility from protected to public to allow access from the linker step |
| src/ObjCRuntime/Blocks.cs | Adds explicit error handling for missing Invoke methods instead of allowing NullReferenceException |
| tools/mtouch/Errors.resx | Adds new error message MX8060 for missing Invoke method in delegate proxy types |
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
tools/common/StaticRegistrar.cs
Outdated
| List<TypeDefinition> CollectAllProtocolsInHierarchy (TypeDefinition type, List<TypeDefinition>? allProtocols = null) | ||
| { | ||
| var rv = new List<TypeDefinition> (); | ||
| CollectInterfaces (ref rv, type); | ||
| return rv; | ||
| } |
There was a problem hiding this comment.
The allProtocols parameter in the method signature is unused. The method always creates a new list and doesn't use the parameter at all. Either remove the parameter or use it as the initial list if provided.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
✅ [CI Build #88e08d7] Build passed (Build packages) ✅Pipeline on Agent |
✅ [PR Build #88e08d7] Build passed (Detect API changes) ✅Pipeline on Agent |
✅ API diff for current PR / commitNET (empty diffs)✅ API diff vs stableNET (empty diffs)ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
✅ [CI Build #88e08d7] Build passed (Build macOS tests) ✅Pipeline on Agent |
💻 [CI Build #88e08d7] Tests on macOS X64 - Mac Sonoma (14) passed 💻✅ All tests on macOS X64 - Mac Sonoma (14) passed. Pipeline on Agent |
💻 [CI Build #88e08d7] Tests on macOS M1 - Mac Monterey (12) passed 💻✅ All tests on macOS M1 - Mac Monterey (12) passed. Pipeline on Agent |
💻 [CI Build #88e08d7] Tests on macOS arm64 - Mac Sequoia (15) passed 💻✅ All tests on macOS arm64 - Mac Sequoia (15) passed. Pipeline on Agent |
💻 [CI Build #88e08d7] Tests on macOS M1 - Mac Ventura (13) passed 💻✅ All tests on macOS M1 - Mac Ventura (13) passed. Pipeline on Agent |
💻 [CI Build #88e08d7] Tests on macOS arm64 - Mac Tahoe (26) passed 💻✅ All tests on macOS arm64 - Mac Tahoe (26) passed. Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
🚀 [CI Build #88e08d7] Test results 🚀Test results✅ All tests passed on VSTS: test results. 🎉 All 122 tests passed 🎉 Tests counts✅ cecil: All 1 tests passed. Html Report (VSDrops) Download Pipeline on Agent |
Add a custom linker step to mark the Invoke method for the type pointed to by
the DelegateProxy attribute, because this method may only be referenced from
the native code the static registrar generates.
Fixes the following test failure for the "Release (all optimizations)" test variation: