[RGen] Factory method for the method signature of Invoke for the NI* classes. #22950
Conversation
…classes. Create the signature based on the information from the delegate.
There was a problem hiding this comment.
Pull Request Overview
Adds a new factory method to generate native trampoline invoke signatures from delegate metadata and verifies it with a comprehensive set of unit tests.
- Introduce
GetTrampolineNativeInvokeSignatureinBindingSyntaxFactory.Trampoline.cs - Add
TestDataGetTrampolineNativeInvokeSignatureand corresponding theory test inBindingSyntaxFactoryTrampolineTests.cs
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/rgen/Microsoft.Macios.Generator.Tests/Emitters/BindingSyntaxFactoryTrampolineTests.cs | Add TestDataGetTrampolineNativeInvokeSignature data class and test |
| src/rgen/Microsoft.Macios.Generator/Emitters/BindingSyntaxFactory.Trampoline.cs | Implement GetTrampolineNativeInvokeSignature with XML doc comments |
Comments suppressed due to low confidence (2)
src/rgen/Microsoft.Macios.Generator/Emitters/BindingSyntaxFactory.Trampoline.cs:872
- [nitpick] The loop variable
currentParametercould be shortened or made more descriptive (e.g.,paramordelegateParam) for brevity and clarity.
foreach (var currentParameter in delegateTypeInfo.Delegate!.Parameters) {
tests/rgen/Microsoft.Macios.Generator.Tests/Emitters/BindingSyntaxFactoryTrampolineTests.cs:3310
- All existing cases cover single-parameter delegates; consider adding one or more multi-parameter delegate scenarios to ensure the signature generation handles multiple parameters correctly.
class TestDataGetTrampolineNativeInvokeSignature : IEnumerable<object []> {
src/rgen/Microsoft.Macios.Generator/Emitters/BindingSyntaxFactory.Trampoline.cs
Outdated
Show resolved
Hide resolved
| var modifiers = TokenList ( | ||
| Token (SyntaxKind.UnsafeKeyword)); | ||
| // the parameters are the exact same as the delegate parameters, we are not going to low level them | ||
| var parameterBucket = ImmutableArray.CreateBuilder<ParameterSyntax> (delegateTypeInfo.Delegate!.Parameters.Length); |
There was a problem hiding this comment.
[nitpick] The variable name parameterBucket is vague—consider renaming it to something like parametersBuilder or parameterListBuilder to clarify its purpose.
| var parameterBucket = ImmutableArray.CreateBuilder<ParameterSyntax> (delegateTypeInfo.Delegate!.Parameters.Length); | |
| var parametersBuilder = ImmutableArray.CreateBuilder<ParameterSyntax> (delegateTypeInfo.Delegate!.Parameters.Length); |
Copilot uses AI. Check for mistakes.
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 #2c8ca3c] Build passed (Build macOS tests) ✅Pipeline on Agent |
❌ [CI Build #2c8ca3c] Tests on macOS X64 - Mac Sonoma (14) failed ❌Failed tests are:
Pipeline on Agent |
💻 [CI Build #2c8ca3c] Tests on macOS M1 - Mac Monterey (12) passed 💻✅ All tests on macOS M1 - Mac Monterey (12) passed. Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
💻 [CI Build #2c8ca3c] Tests on macOS arm64 - Mac Sequoia (15) passed 💻✅ All tests on macOS arm64 - Mac Sequoia (15) passed. Pipeline on Agent |
💻 [CI Build #2c8ca3c] Tests on macOS M1 - Mac Ventura (13) passed 💻✅ All tests on macOS M1 - Mac Ventura (13) passed. 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 |
This comment has been minimized.
This comment has been minimized.
🔥 [CI Build #2c8ca3c] Test results 🔥Test results❌ Tests failed on VSTS: test results 1 tests crashed, 1 tests failed, 105 tests passed. Failures❌ monotouch tests (macOS)🔥 Failed catastrophically on VSTS: test results - monotouch_macos (no summary found). Html Report (VSDrops) Download ❌ windows testsHtml Report (VSDrops) Download Successes✅ cecil: All 1 tests passed. Html Report (VSDrops) Download Pipeline on Agent |
🔥 [CI Build #36c163b] Build failed (Build packages) 🔥Build failed for the job 'Build packages' (with job status 'Failed') Pipeline on Agent |
🔥 [PR Build #36c163b] Build failed (Detect API changes) 🔥Build failed for the job 'Detect API changes' (with job status 'Failed') Pipeline on Agent |
…ory.Trampoline.cs Co-authored-by: Copilot <[email protected]>
Create the signature based on the information from the delegate.