[RGen] Fix the argument generation for a non nullable ref value type. #22894
Conversation
The code was not creating a pointer declaration when the type was not nullable because we were missing a switch option. Updated the way we generate the pointer in a more generic way by calling the GetLowerMethod which will work for all possible types.
There was a problem hiding this comment.
Pull Request Overview
This PR fixes the pointer generation logic for non nullable ref value types by using a more generic approach through GetLowLevelType.
- Updated pointer generation logic in BindingSyntaxFactory.Trampoline to handle both nullable and non-nullable types correctly.
- Added new test cases in BindingSyntaxFactoryTrampolineTests to validate the changes for int and boolean types.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tests/rgen/Microsoft.Macios.Generator.Tests/Emitters/BindingSyntaxFactoryTrampolineTests.cs | Added test cases for pointer generation with nullable and non-nullable int types |
| src/rgen/Microsoft.Macios.Generator/Emitters/BindingSyntaxFactory.Trampoline.cs | Modified pointer generation logic to use GetLowLevelType for improved type handling |
Comments suppressed due to low confidence (1)
tests/rgen/Microsoft.Macios.Generator.Tests/Emitters/BindingSyntaxFactoryTrampolineTests.cs:2932
- [nitpick] The delegate parameter name 'outNullableInt' is misleading given that its type is now non-nullable; consider renaming it to 'outInt' for improved clarity.
public delegate void Callback (out int outNullableInt);
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.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
✅ [CI Build #866598c] Build passed (Build packages) ✅Pipeline on Agent |
✅ [PR Build #866598c] 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 #866598c] Build passed (Build macOS tests) ✅Pipeline on Agent |
💻 [CI Build #866598c] Tests on macOS X64 - Mac Sonoma (14) passed 💻✅ All tests on macOS X64 - Mac Sonoma (14) passed. Pipeline on Agent |
💻 [CI Build #866598c] Tests on macOS M1 - Mac Monterey (12) passed 💻✅ All tests on macOS M1 - Mac Monterey (12) passed. Pipeline on Agent |
💻 [CI Build #866598c] Tests on macOS arm64 - Mac Sequoia (15) passed 💻✅ All tests on macOS arm64 - Mac Sequoia (15) passed. Pipeline on Agent |
💻 [CI Build #866598c] Tests on macOS M1 - Mac Ventura (13) passed 💻✅ All tests on macOS M1 - Mac Ventura (13) passed. Pipeline on Agent |
🔥 [CI Build #866598c] Test results 🔥Test results❌ Tests failed on VSTS: test results 0 tests crashed, 1 tests failed, 114 tests passed. Failures❌ dotnettests tests (MacCatalyst)
Html Report (VSDrops) Download Successes✅ cecil: All 1 tests passed. Html Report (VSDrops) Download Pipeline on Agent |
4 participants
The code was not creating a pointer declaration when the type was not nullable because we were missing a switch option.
Updated the way we generate the pointer in a more generic way by calling the GetLowerMethod which will work for all possible types.