-
Notifications
You must be signed in to change notification settings - Fork 63
[generator] Don't avoid blittable types for fields #1353
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[generator] Don't avoid blittable types for fields #1353
Conversation
Fixes: dotnet/android#10404 Context: 57f7bc8 Commit 57f7bc8 updated `generator` to "avoid non-blittable types" in native callback methods, for which there were two non-blittables: * System.Boolean, which should be marshaled as a System.SByte, and * System.Char, which should be marshaled as a System.UInt16. The problem is that this hit a codepath which was *not* "for native callback methods": field bindings. Consider [`android.widget.RelativeLayout.LayoutParams.alignWithParent`][0]: package android.widget; public /* partial */ class RelativeLayout { public /* partial */ class LayoutParams { public boolean alignWithParent; } } which is bound as [`RelativeLayout.LayoutParams.AlignWithParent`][1]: namespace Android.Widget; public partial class RelativeLayout { public new partial class LayoutParams { [Register] public bool AlignWithParent { get => _members.InstanceFields.GetBooleanValue ("alignWithParent.Z", this); set { _members.InstanceFields.SetValue("alignWithParent.Z", this, value ? (sbyte)1 : (sbyte)0); } } } } The problem is the property setter, which calls `Java.Interop.JniPeerMembers.JniInstanceFields.SetValue(string, IJavaPeerable, sbyte)`. Before 57f7bc8, it would instead call `Java.Interop.JniPeerMembers.JniInstanceFields.SetValue(string, IJavaPeerable, bool)`, but with 57f7bc8 there are now runtime crashes when boolean fields are accessed, starting in .NET 10 Preview 2. The following code fragment: var p = new RelativeLayout.LayoutParams (1, 2) { AlignWithParent = true, }; crashes with: F droid_boolfiel: java_vm_ext.cc:542] JNI DETECTED ERROR IN APPLICATION: attempt to access field boolean android.widget.RelativeLayout$LayoutParams.alignWithParent of type boolean with the wrong type byte: 0x709385d8 F droid_boolfiel: java_vm_ext.cc:542] in call to SetByteField F droid_boolfiel: java_vm_ext.cc:542] from void crc6463d68d2626be2acb.MainActivity.n_onCreate(android.os.Bundle) Fix this by updating `generator` so that `BoundFieldAsProperty.cs` uses the parameter as-is when `ISymbol.OnlyFormatOnMarshal`=true. The binding for `RelativeLayout.LayoutParams.AlignWithParent` becomes: namespace Android.Widget; public partial class RelativeLayout { public new partial class LayoutParams { [Register] public bool AlignWithParent { get => _members.InstanceFields.GetBooleanValue ("alignWithParent.Z", this); set { _members.InstanceFields.SetValue("alignWithParent.Z", this, value); } } } } i.e. calling `JniPeerMembers.JniInstanceFields.SetValue(string, IJavaPeerable, bool)`. [0]: https://developer.android.com/reference/android/widget/RelativeLayout.LayoutParams#alignWithParent [1]: https://learn.microsoft.com/en-us/dotnet/api/android.widget.relativelayout.layoutparams.alignwithparent?view=net-android-34.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes a runtime crash in .NET 10 Preview 2 where boolean field setters were calling the wrong JNI method due to an unintended side effect of commit 57f7bc8. The issue occurred when the generator's "avoid non-blittable types" logic, intended for native callback methods, was incorrectly applied to field bindings.
- Fixes field property setters to use the correct parameter type instead of converting to native types
- Restores proper boolean field handling by preserving the original parameter when
OnlyFormatOnMarshalis true - Adds test coverage for boolean and char field bindings to prevent regression
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/generator/SourceWriters/BoundFieldAsProperty.cs | Core fix - conditionally preserve parameter type for field setters when OnlyFormatOnMarshal is true |
| tests/generator-Tests/expected.xaji/StaticFields/Xamarin.Test.SomeObject.cs | Test output showing corrected static boolean/char field property generation |
| tests/generator-Tests/expected.xaji/NonStaticFields/Xamarin.Test.SomeObject.cs | Test output showing corrected instance boolean/char field property generation |
| tests/generator-Tests/expected.ji/StaticFields/Xamarin.Test.SomeObject.cs | Test output for Java.Interop target showing corrected static field generation |
| tests/generator-Tests/expected.ji/StaticFields/StaticField.xml | Test metadata defining boolean and char fields for static field tests |
| tests/generator-Tests/expected.ji/NonStaticFields/Xamarin.Test.SomeObject.cs | Test output for Java.Interop target showing corrected instance field generation |
| tests/generator-Tests/expected.ji/NonStaticFields/NonStaticField.xml | Test metadata defining boolean and char fields for instance field tests |
Fixes: dotnet/android#10404
Context: 57f7bc8
Commit 57f7bc8 updated
generatorto "avoid non-blittable types" in native callback methods, for which there were two non-blittables:The problem is that this hit a codepath which was not "for native callback methods": field bindings.
Consider
android.widget.RelativeLayout.LayoutParams.alignWithParent:which is bound as
RelativeLayout.LayoutParams.AlignWithParent:The problem is the property setter, which calls
Java.Interop.JniPeerMembers.JniInstanceFields.SetValue(string, IJavaPeerable, sbyte). Before 57f7bc8, it would instead callJava.Interop.JniPeerMembers.JniInstanceFields.SetValue(string, IJavaPeerable, bool), but with 57f7bc8 there are now runtime crashes when boolean fields are accessed, starting in .NET 10 Preview 2.The following code fragment:
crashes with:
Fix this by updating
generatorso thatBoundFieldAsProperty.csuses the parameter as-is whenISymbol.OnlyFormatOnMarshal=true.The binding for
RelativeLayout.LayoutParams.AlignWithParentbecomes:i.e. calling
JniPeerMembers.JniInstanceFields.SetValue(string, IJavaPeerable, bool).