[generator] Don't avoid blittable types for fields#1353
Merged
jonathanpeppers merged 2 commits intodotnet:mainfrom Aug 12, 2025
Merged
[generator] Don't avoid blittable types for fields#1353jonathanpeppers merged 2 commits intodotnet:mainfrom
jonathanpeppers merged 2 commits intodotnet:mainfrom
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.
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 |
tests/generator-Tests/expected.xaji/StaticFields/Xamarin.Test.SomeObject.cs
Show resolved
Hide resolved
tests/generator-Tests/expected.xaji/NonStaticFields/Xamarin.Test.SomeObject.cs
Show resolved
Hide resolved
tests/generator-Tests/expected.ji/StaticFields/Xamarin.Test.SomeObject.cs
Show resolved
Hide resolved
tests/generator-Tests/expected.ji/NonStaticFields/Xamarin.Test.SomeObject.cs
Show resolved
Hide resolved
jonathanpeppers
approved these changes
Aug 12, 2025
jonathanpeppers
approved these changes
Aug 12, 2025
grendello
approved these changes
Aug 12, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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).