Skip to content

Commit 90ac202

Browse files
jonpryorjonpryor
authored
[generator] Don't avoid blittable types for fields (#1353)
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
1 parent 3a8079d commit 90ac202

File tree

7 files changed

+164
-1
lines changed

7 files changed

+164
-1
lines changed

‎tests/generator-Tests/expected.ji/NonStaticFields/NonStaticField.xml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
final="false" name="SomeObject" static="false" visibility="public">
1010
<field deprecated="not deprecated" final="true" name="Value" static="false" transient="false" type="int" type-generic-aware="int" visibility="public" volatile="false">
1111
</field>
12+
<field deprecated="not deprecated" final="false" name="BooleanValue" static="false" transient="false" type="boolean" type-generic-aware="int" visibility="public" volatile="false" />
13+
<field deprecated="not deprecated" final="false" name="CharValue" static="false" transient="false" type="boolean" type-generic-aware="int" visibility="public" volatile="false" />
1214
</class>
1315
</package>
1416
</api>

‎tests/generator-Tests/expected.ji/NonStaticFields/Xamarin.Test.SomeObject.cs

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,44 @@ public int Value {
3636
}
3737
}
3838

39+
40+
// Metadata.xml XPath field reference: path="/api/package[@name='xamarin.test']/class[@name='SomeObject']/field[@name='BooleanValue']"
41+
public bool BooleanValue {
42+
get {
43+
const string __id = "BooleanValue.Z";
44+
45+
var __v = _members.InstanceFields.GetBooleanValue (__id, this);
46+
return __v;
47+
}
48+
set {
49+
const string __id = "BooleanValue.Z";
50+
51+
try {
52+
_members.InstanceFields.SetValue (__id, this, value);
53+
} finally {
54+
}
55+
}
56+
}
57+
58+
59+
// Metadata.xml XPath field reference: path="/api/package[@name='xamarin.test']/class[@name='SomeObject']/field[@name='CharValue']"
60+
public bool CharValue {
61+
get {
62+
const string __id = "CharValue.Z";
63+
64+
var __v = _members.InstanceFields.GetBooleanValue (__id, this);
65+
return __v;
66+
}
67+
set {
68+
const string __id = "CharValue.Z";
69+
70+
try {
71+
_members.InstanceFields.SetValue (__id, this, value);
72+
} finally {
73+
}
74+
}
75+
}
76+
3977
static readonly JniPeerMembers _members = new JniPeerMembers ("xamarin/test/SomeObject", typeof (SomeObject));
4078

4179
[global::System.Diagnostics.DebuggerBrowsable (global::System.Diagnostics.DebuggerBrowsableState.Never)]

‎tests/generator-Tests/expected.ji/StaticFields/StaticField.xml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111
</field>
1212
<field deprecated="not deprecated" final="false" name="Value2" static="true" transient="false" type="int" type-generic-aware="int" visibility="public" volatile="false">
1313
</field>
14+
<field deprecated="not deprecated" final="false" name="BooleanValue" static="true" transient="false" type="boolean" type-generic-aware="int" visibility="public" volatile="false" />
15+
<field deprecated="not deprecated" final="false" name="CharValue" static="true" transient="false" type="boolean" type-generic-aware="int" visibility="public" volatile="false" />
1416
</class>
1517
</package>
1618
</api>

‎tests/generator-Tests/expected.ji/StaticFields/Xamarin.Test.SomeObject.cs

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,44 @@ public static int Value2 {
4747
}
4848
}
4949

50+
51+
// Metadata.xml XPath field reference: path="/api/package[@name='xamarin.test']/class[@name='SomeObject']/field[@name='BooleanValue']"
52+
public static bool BooleanValue {
53+
get {
54+
const string __id = "BooleanValue.Z";
55+
56+
var __v = _members.StaticFields.GetBooleanValue (__id);
57+
return __v;
58+
}
59+
set {
60+
const string __id = "BooleanValue.Z";
61+
62+
try {
63+
_members.StaticFields.SetValue (__id, value);
64+
} finally {
65+
}
66+
}
67+
}
68+
69+
70+
// Metadata.xml XPath field reference: path="/api/package[@name='xamarin.test']/class[@name='SomeObject']/field[@name='CharValue']"
71+
public static bool CharValue {
72+
get {
73+
const string __id = "CharValue.Z";
74+
75+
var __v = _members.StaticFields.GetBooleanValue (__id);
76+
return __v;
77+
}
78+
set {
79+
const string __id = "CharValue.Z";
80+
81+
try {
82+
_members.StaticFields.SetValue (__id, value);
83+
} finally {
84+
}
85+
}
86+
}
87+
5088
static readonly JniPeerMembers _members = new JniPeerMembers ("xamarin/test/SomeObject", typeof (SomeObject));
5189

5290
[global::System.Diagnostics.DebuggerBrowsable (global::System.Diagnostics.DebuggerBrowsableState.Never)]

‎tests/generator-Tests/expected.xaji/NonStaticFields/Xamarin.Test.SomeObject.cs

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,46 @@ public int Value {
3838
}
3939
}
4040

41+
42+
// Metadata.xml XPath field reference: path="/api/package[@name='xamarin.test']/class[@name='SomeObject']/field[@name='BooleanValue']"
43+
[Register ("BooleanValue")]
44+
public bool BooleanValue {
45+
get {
46+
const string __id = "BooleanValue.Z";
47+
48+
var __v = _members.InstanceFields.GetBooleanValue (__id, this);
49+
return __v;
50+
}
51+
set {
52+
const string __id = "BooleanValue.Z";
53+
54+
try {
55+
_members.InstanceFields.SetValue (__id, this, value);
56+
} finally {
57+
}
58+
}
59+
}
60+
61+
62+
// Metadata.xml XPath field reference: path="/api/package[@name='xamarin.test']/class[@name='SomeObject']/field[@name='CharValue']"
63+
[Register ("CharValue")]
64+
public bool CharValue {
65+
get {
66+
const string __id = "CharValue.Z";
67+
68+
var __v = _members.InstanceFields.GetBooleanValue (__id, this);
69+
return __v;
70+
}
71+
set {
72+
const string __id = "CharValue.Z";
73+
74+
try {
75+
_members.InstanceFields.SetValue (__id, this, value);
76+
} finally {
77+
}
78+
}
79+
}
80+
4181
static readonly JniPeerMembers _members = new XAPeerMembers ("xamarin/test/SomeObject", typeof (SomeObject));
4282

4383
internal static new IntPtr class_ref {

‎tests/generator-Tests/expected.xaji/StaticFields/Xamarin.Test.SomeObject.cs

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,46 @@ public static int Value2 {
5050
}
5151
}
5252

53+
54+
// Metadata.xml XPath field reference: path="/api/package[@name='xamarin.test']/class[@name='SomeObject']/field[@name='BooleanValue']"
55+
[Register ("BooleanValue")]
56+
public static bool BooleanValue {
57+
get {
58+
const string __id = "BooleanValue.Z";
59+
60+
var __v = _members.StaticFields.GetBooleanValue (__id);
61+
return __v;
62+
}
63+
set {
64+
const string __id = "BooleanValue.Z";
65+
66+
try {
67+
_members.StaticFields.SetValue (__id, value);
68+
} finally {
69+
}
70+
}
71+
}
72+
73+
74+
// Metadata.xml XPath field reference: path="/api/package[@name='xamarin.test']/class[@name='SomeObject']/field[@name='CharValue']"
75+
[Register ("CharValue")]
76+
public static bool CharValue {
77+
get {
78+
const string __id = "CharValue.Z";
79+
80+
var __v = _members.StaticFields.GetBooleanValue (__id);
81+
return __v;
82+
}
83+
set {
84+
const string __id = "CharValue.Z";
85+
86+
try {
87+
_members.StaticFields.SetValue (__id, value);
88+
} finally {
89+
}
90+
}
91+
}
92+
5393
static readonly JniPeerMembers _members = new XAPeerMembers ("xamarin/test/SomeObject", typeof (SomeObject));
5494

5595
internal static new IntPtr class_ref {

‎tools/generator/SourceWriters/BoundFieldAsProperty.cs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,10 @@ protected override void WriteSetterBody (CodeWriter writer)
139139
writer.WriteLine (prep);
140140
}
141141

142-
arg = field.SetParameters [0].ToNative (opt);
142+
var param = field.SetParameters [0];
143+
arg = param.Symbol.OnlyFormatOnMarshal
144+
? opt.GetSafeIdentifier (param.Name)
145+
: param.ToNative (opt);
143146

144147
if (opt.CodeGenerationTarget != CodeGenerationTarget.JavaInterop1 &&
145148
field.SetParameters.HasCleanup &&

0 commit comments

Comments
 (0)