[RGen] Update the emitters to generate the trampoline static class. #22926
Conversation
This is the initial implementation of the trampoline emitter. It just focuses on the delegate and the static class. A generic class has been added but more tests will come per framework, we simply want to make the diff manageable for the team, so we have not added all the tests in a single PR.
There was a problem hiding this comment.
Pull Request Overview
This PR introduces the initial implementation of the trampoline emitter – generating internal delegate declarations and companion static classes to bridge native block callbacks – and updates the test suite and expected outputs accordingly.
- Implements
TryEmitInternalDelegateandTryEmitStaticClassinTrampolineEmitterto emit delegate types and static bridge classes. - Adds
BlockLiteraltoBindingSyntaxFactory, updates documentation, and adjusts test harness to locate generated trampolines underObjCRuntime. - Extends generator tests with new property-based delegate scenarios and expands expected trampoline code files.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/rgen/Microsoft.Macios.Generator.Tests/Microsoft.Macios.Generator.Tests.csproj | Excludes and includes Trampolines\*.cs patterns for compile and None |
| tests/rgen/Microsoft.Macios.Generator.Tests/Classes/Data/TrampolinePropertyTests.cs | Adds various delegate-backed properties for trampoline testing |
| tests/rgen/Microsoft.Macios.Generator.Tests/Classes/Data/ExpectedTrampolinePropertyTestsTrampolines.cs | Updates expected static classes and delegates for all new trampolines |
| tests/rgen/Microsoft.Macios.Generator.Tests/Classes/Data/ExpectedTrampolinePropertyTests.cs | Updates expected binding code for new trampoline-backed properties |
| tests/rgen/Microsoft.Macios.Generator.Tests/BaseGeneratorTestClass.cs | Adjusts the path filter to ObjCRuntime/Trampolines.g.cs |
| src/rgen/Microsoft.Macios.Generator/Emitters/TrampolineEmitter.cs | Implements the core emitter logic for delegates and static trampoline classes |
| src/rgen/Microsoft.Macios.Generator/Emitters/Documentation.cs | Adds TrampolineStaticClass helper to format doc comments |
| src/rgen/Microsoft.Macios.Generator/Emitters/BindingSyntaxFactory.KnownTypes.cs | Registers ObjCRuntime.BlockLiteral in known types |
Comments suppressed due to low confidence (1)
tests/rgen/Microsoft.Macios.Generator.Tests/Classes/Data/TrampolinePropertyTests.cs:40
- There is an extraneous closing bracket
]at the end of this property declaration. Remove the extra]to fix the syntax error.
public partial CIKernelRoiCallback KernelRoiCallback { get; set; }]
|
|
||
| // Property using AVAssetImageGenerator.AsynchronouslyForTimeCompletionHandler | ||
| [Export<Property> ("imageGeneratorCompletionHandler", ArgumentSemantic.Copy)] | ||
| public partial AVAssetImageGenerateAsynchronouslyForTimeCompletionHandler ImageGeneratorCompletionHandler { get; set; } |
There was a problem hiding this comment.
This property duplicates ImageGeneratorCompletionHandler and uses a mismatched delegate type name. Remove or consolidate the duplicate definition, and ensure the delegate type matches the intended AVAssetImageGenerator.AsynchronouslyForTimeCompletionHandler.
Copilot uses AI. Check for mistakes.
src/rgen/Microsoft.Macios.Generator/Emitters/TrampolineEmitter.cs
Outdated
Show resolved
Hide resolved
| [BindingImpl (BindingImplOptions.GeneratedCode | BindingImplOptions.Optimizable)] | ||
| public partial global::CoreImage.CIKernelRoiCallback KernelRoiCallback | ||
| { | ||
| get | ||
| { | ||
| global::CoreImage.CIKernelRoiCallback ret; | ||
| if (IsDirectBinding) { | ||
| ret = global::ObjCRuntime.Messaging.NativeHandle_objc_msgSend (this.Handle, global::ObjCRuntime.Selector.GetHandle ("kernelRoiCallback")); | ||
| } else { | ||
| ret = global::ObjCRuntime.Messaging.NativeHandle_objc_msgSendSuper (this.Handle, global::ObjCRuntime.Selector.GetHandle ("kernelRoiCallback")); | ||
| } | ||
| GC.KeepAlive (this); | ||
| return ret; | ||
| } | ||
|
|
||
| set | ||
| { | ||
| throw new NotImplementedException(); | ||
| } | ||
| } | ||
|
|
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Co-authored-by: Copilot <[email protected]>
✅ [CI Build #da210a1] Build passed (Build packages) ✅Pipeline on Agent |
✅ [PR Build #da210a1] Build passed (Detect API changes) ✅Pipeline on Agent |
✅ API diff for current PR / commit.NET ( No breaking changes )✅ API diff vs stable.NET ( No breaking changes )ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
✅ [CI Build #da210a1] Build passed (Build macOS tests) ✅Pipeline on Agent |
💻 [CI Build #da210a1] Tests on macOS X64 - Mac Sonoma (14) passed 💻✅ All tests on macOS X64 - Mac Sonoma (14) passed. Pipeline on Agent |
💻 [CI Build #da210a1] Tests on macOS M1 - Mac Monterey (12) passed 💻✅ All tests on macOS M1 - Mac Monterey (12) passed. Pipeline on Agent |
💻 [CI Build #da210a1] Tests on macOS arm64 - Mac Sequoia (15) passed 💻✅ All tests on macOS arm64 - Mac Sequoia (15) passed. Pipeline on Agent |
💻 [CI Build #da210a1] Tests on macOS M1 - Mac Ventura (13) passed 💻✅ All tests on macOS M1 - Mac Ventura (13) passed. Pipeline on Agent |
🔥 [CI Build #da210a1] Test results 🔥Test results❌ Tests failed on VSTS: test results 1 tests crashed, 1 tests failed, 115 tests passed. Failures❌ dotnettests tests (Multiple platforms)
Html Report (VSDrops) Download ❌ windows tests🔥 Failed catastrophically on VSTS: test results - windows (no summary found). Html Report (VSDrops) Download Successes✅ cecil: All 1 tests passed. Html Report (VSDrops) Download Pipeline on Agent |
|
Failing tests are unrelated to the code changes. |
This is the initial implementation of the trampoline emitter. It just focuses on the delegate and the static class. A generic class has been added but more tests will come per framework, we simply want to make the diff manageable for the team, so we have not added all the tests in a single PR.