[RGen] Fix bug where we converted a bool to a byte when we needed the opposite. #22891
Conversation
… opposite. The argument conversions is from low level to high level, therefore we need to look for a bool parameter and convert the byte to a bool. Not the otherway around.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
✅ [CI Build #be1d187] Build passed (Build packages) ✅Pipeline on Agent |
✅ [PR Build #be1d187] Build passed (Detect API changes) ✅Pipeline on Agent |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a conversion bug by ensuring that boolean values are properly converted from a byte to a bool instead of the incorrect conversion from bool to byte.
- Updated test expectation to compare "boolParameter != 0" instead of a ternary byte cast.
- Refactored the trampoline emitter to invoke CastToBool.
- Introduced a new helper method CastToBool in the ObjCRuntime emitter to perform the byte-to-bool conversion.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/rgen/Microsoft.Macios.Generator.Tests/Emitters/BindingSyntaxFactoryTrampolineTests.cs | Updated test assertion for bool conversion |
| src/rgen/Microsoft.Macios.Generator/Emitters/BindingSyntaxFactory.Trampoline.cs | Fixed conversion by using CastToBool instead of CastToByte |
| src/rgen/Microsoft.Macios.Generator/Emitters/BindingSyntaxFactory.ObjCRuntime.cs | Added new CastToBool helper function with documentation |
| /// <param name="typeInfo">The type information of the variable.</param> | ||
| /// <returns>A binary expression that casts a byte to a bool.</returns> | ||
| internal static BinaryExpressionSyntax CastToBool (string variableName, in TypeInfo typeInfo) |
There was a problem hiding this comment.
The 'typeInfo' parameter is not used in the function body; consider either utilizing it for additional type-based logic or removing it to simplify the method signature.
| /// <param name="typeInfo">The type information of the variable.</param> | |
| /// <returns>A binary expression that casts a byte to a bool.</returns> | |
| internal static BinaryExpressionSyntax CastToBool (string variableName, in TypeInfo typeInfo) | |
| /// <returns>A binary expression that casts a byte to a bool.</returns> | |
| internal static BinaryExpressionSyntax CastToBool (string variableName) |
Copilot uses AI. Check for mistakes.
This comment has been minimized.
This comment has been minimized.
✅ 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.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
💻 [CI Build #942854b] Tests on macOS arm64 - Mac Sequoia (15) passed 💻✅ All tests on macOS arm64 - Mac Sequoia (15) passed. Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
✅ [CI Build #be1d187] Build passed (Build macOS tests) ✅Pipeline on Agent |
💻 [CI Build #be1d187] Tests on macOS X64 - Mac Sonoma (14) passed 💻✅ All tests on macOS X64 - Mac Sonoma (14) passed. Pipeline on Agent |
💻 [CI Build #be1d187] Tests on macOS M1 - Mac Monterey (12) passed 💻✅ All tests on macOS M1 - Mac Monterey (12) passed. Pipeline on Agent |
💻 [CI Build #be1d187] Tests on macOS M1 - Mac Ventura (13) passed 💻✅ All tests on macOS M1 - Mac Ventura (13) passed. Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
🔥 [CI Build #be1d187] Test results 🔥Test results❌ Tests failed on VSTS: test results 0 tests crashed, 11 tests failed, 108 tests passed. Failures❌ monotouch tests (MacCatalyst)
Html Report (VSDrops) Download Successes✅ cecil: All 1 tests passed. Html Report (VSDrops) Download Pipeline on Agent |
|
Failing tests are unrelated to code changes. |
4 participants
The argument conversions is from low level to high level, therefore we need to look for a bool parameter and convert the byte to a bool. Not the otherway around.