Skip to content

Commit ae914d7

Browse files
jpobstjpobst
committed
[generator] Avoid non-blittable types in native callback methods
1 parent 6bc87e8 commit ae914d7

File tree

9 files changed

+174
-6
lines changed

9 files changed

+174
-6
lines changed

‎tests/generator-Tests/Unit-Tests/BlittableTypeTests.cs‎

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
using System;
2+
using MonoDroid.Generation;
3+
using NUnit.Framework;
4+
using Xamarin.Android.Binder;
5+
6+
namespace generatortests;
7+
8+
[TestFixture]
9+
class BlittableTypeTests : CodeGeneratorTestBase
10+
{
11+
protected override CodeGenerationTarget Target => CodeGenerationTarget.XAJavaInterop1;
12+
13+
[Test]
14+
public void MethodWithBoolReturnType ()
15+
{
16+
var klass = new TestClass ("Object", "java.code.MyClass");
17+
var method = SupportTypeBuilder.CreateMethod (klass, "IsEmpty", options, "boolean");
18+
19+
klass.Methods.Add (method);
20+
21+
var actual = GetGeneratedTypeOutput (klass);
22+
23+
// Return type should be byte
24+
Assert.That (actual, Contains.Substring ("static byte n_IsEmpty"));
25+
26+
// Return statement should convert to 0 or 1
27+
Assert.That (actual, Contains.Substring ("return __this.IsEmpty () ? 1 : 0"));
28+
29+
// Ensure the marshal delegate is byte
30+
Assert.That (actual, Contains.Substring ("new _JniMarshal_PP_B"));
31+
Assert.That (actual, Does.Not.Contains ("new _JniMarshal_PP_Z"));
32+
}
33+
34+
[Test]
35+
public void MethodWithBoolParameter ()
36+
{
37+
var klass = new TestClass ("Object", "java.code.MyClass");
38+
var method = SupportTypeBuilder.CreateMethod (klass, "SetEmpty", options, "void", parameters: new Parameter ("value", "boolean", "bool", false));
39+
40+
klass.Methods.Add (method);
41+
42+
var actual = GetGeneratedTypeOutput (klass);
43+
44+
// Method parameter should be byte
45+
Assert.That (actual, Contains.Substring ("static void n_SetEmpty_Z (IntPtr jnienv, IntPtr native__this, byte native_value)"));
46+
47+
// Method should convert from 0 or 1
48+
Assert.That (actual, Contains.Substring ("var value = native_value != 0;"));
49+
50+
// Ensure the marshal delegate is byte
51+
Assert.That (actual, Contains.Substring ("new _JniMarshal_PPB_V"));
52+
Assert.That (actual, Does.Not.Contains ("new _JniMarshal_PPZ_V"));
53+
}
54+
55+
[Test]
56+
public void BoolProperty ()
57+
{
58+
var klass = SupportTypeBuilder.CreateClassWithProperty ("MyClass", "com.example.myClass", "IsEmpty", "boolean", options);
59+
var actual = GetGeneratedTypeOutput (klass);
60+
61+
// Getter return type should be byte
62+
Assert.That (actual, Contains.Substring ("static byte n_get_IsEmpty"));
63+
64+
// Getter return statement should convert to 0 or 1
65+
Assert.That (actual, Contains.Substring ("return __this.IsEmpty ? 1 : 0"));
66+
67+
// Setter parameter should be byte
68+
Assert.That (actual, Contains.Substring ("static void n_set_IsEmpty_Z (IntPtr jnienv, IntPtr native__this, byte native_value)"));
69+
70+
// Setter should convert from 0 or 1
71+
Assert.That (actual, Contains.Substring ("var value = native_value != 0;"));
72+
73+
// Ensure the marshal delegate is byte
74+
Assert.That (actual, Contains.Substring ("new _JniMarshal_PP_B"));
75+
Assert.That (actual, Does.Not.Contains ("new _JniMarshal_PP_Z"));
76+
}
77+
78+
[Test]
79+
public void MethodWithCharReturnType ()
80+
{
81+
var klass = new TestClass ("Object", "java.code.MyClass");
82+
var method = SupportTypeBuilder.CreateMethod (klass, "GetFirstLetter", options, "char");
83+
84+
klass.Methods.Add (method);
85+
86+
var actual = GetGeneratedTypeOutput (klass);
87+
88+
// Return type should be ushort
89+
Assert.That (actual, Contains.Substring ("static ushort n_GetFirstLetter"));
90+
91+
// Return statement should convert to ushort
92+
Assert.That (actual, Contains.Substring ("return (ushort)__this.GetFirstLetter ()"));
93+
94+
// Ensure the marshal delegate is ushort
95+
Assert.That (actual, Contains.Substring ("new _JniMarshal_PP_s"));
96+
Assert.That (actual, Does.Not.Contains ("new _JniMarshal_PP_C"));
97+
}
98+
99+
[Test]
100+
public void MethodWithCharParameter ()
101+
{
102+
var klass = new TestClass ("Object", "java.code.MyClass");
103+
var method = SupportTypeBuilder.CreateMethod (klass, "SetFirstLetter", options, "void", parameters: new Parameter ("value", "char", "char", false));
104+
105+
klass.Methods.Add (method);
106+
107+
var actual = GetGeneratedTypeOutput (klass);
108+
109+
// Method parameter should be ushort
110+
Assert.That (actual, Contains.Substring ("static void n_SetFirstLetter_C (IntPtr jnienv, IntPtr native__this, ushort native_value)"));
111+
112+
// Method should convert from ushort to char
113+
Assert.That (actual, Contains.Substring ("var value = (char)native_value;"));
114+
115+
// Ensure the marshal delegate is ushort
116+
Assert.That (actual, Contains.Substring ("new _JniMarshal_PPs_V"));
117+
Assert.That (actual, Does.Not.Contains ("new _JniMarshal_PPC_V"));
118+
}
119+
120+
[Test]
121+
public void CharProperty ()
122+
{
123+
var klass = SupportTypeBuilder.CreateClassWithProperty ("MyClass", "com.example.myClass", "FirstLetter", "char", options);
124+
var actual = GetGeneratedTypeOutput (klass);
125+
126+
// Getter return type should be ushort
127+
Assert.That (actual, Contains.Substring ("static ushort n_get_FirstLetter"));
128+
129+
// Getter return statement should convert to ushort
130+
Assert.That (actual, Contains.Substring ("return (ushort)__this.FirstLetter"));
131+
132+
// Setter parameter should be ushort
133+
Assert.That (actual, Contains.Substring ("static void n_set_FirstLetter_C (IntPtr jnienv, IntPtr native__this, ushort native_value)"));
134+
135+
// Setter should convert from ushort to char
136+
Assert.That (actual, Contains.Substring ("var value = (char)native_value;"));
137+
138+
// Ensure the marshal delegate is ushort
139+
Assert.That (actual, Contains.Substring ("new _JniMarshal_PP_s"));
140+
Assert.That (actual, Does.Not.Contains ("new _JniMarshal_PP_C"));
141+
}
142+
}

‎tests/generator-Tests/Unit-Tests/CodeGeneratorTestBase.cs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,5 +131,14 @@ protected static string GetAssertionMessage (string header, string expected, str
131131
$"Expected:\n```\n{expected}\n```\n" +
132132
$"Actual:\n```\n{actual}\n```";
133133
}
134+
135+
protected string GetGeneratedTypeOutput (GenBase gen)
136+
{
137+
generator.Context.ContextTypes.Push (gen);
138+
generator.WriteType (gen, string.Empty, new GenerationInfo ("", "", "MyAssembly"));
139+
generator.Context.ContextTypes.Pop ();
140+
141+
return writer.ToString ();
142+
}
134143
}
135144
}

‎tools/generator/CodeGenerationOptions.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,8 @@ string GetJniTypeCode (ISymbol symbol)
217217
case "uint": return "i";
218218
case "ulong": return "j";
219219
case "ushort": return "s";
220+
case "boolean": return "B"; // We marshal boolean (Z) as byte (B)
221+
case "char": return "s"; // We marshal char (C) as ushort (s)
220222
}
221223

222224
var jni_name = symbol.JniName;

‎tools/generator/Java.Interop.Tools.Generator.ObjectModel/ReturnValue.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ public string RawJavaType {
9797

9898
public ISymbol Symbol => sym;
9999

100-
public string FromNative (CodeGenerationOptions opt, string var_name, bool owned)
100+
public string FromNative (CodeGenerationOptions opt, string var_name, bool owned, bool isMarshal = false)
101101
{
102102
if (!string.IsNullOrEmpty (managed_type) && (sym is ClassGen || sym is InterfaceGen)) {
103103
if (opt.CodeGenerationTarget == Xamarin.Android.Binder.CodeGenerationTarget.JavaInterop1) {
@@ -108,7 +108,7 @@ public string FromNative (CodeGenerationOptions opt, string var_name, bool owned
108108
return string.Format ("global::Java.Lang.Object.GetObject<{0}> ({1}, {2})",
109109
opt.GetOutputName (managed_type), var_name, owned ? "JniHandleOwnership.TransferLocalRef" : "JniHandleOwnership.DoNotTransfer");
110110
}
111-
return sym.FromNative (opt, var_name, owned);
111+
return sym.FromNative (opt, var_name, owned, isMarshal);
112112
}
113113

114114
public string ToNative (CodeGenerationOptions opt, string var_name)

‎tools/generator/Java.Interop.Tools.Generator.ObjectModel/Symbols/FormatSymbol.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,8 @@ public string[] PostCall (CodeGenerationOptions opt, string var_name)
116116
}
117117

118118
public bool NeedsPrep { get { return false; } }
119+
120+
public bool OnlyFormatOnMarshal { get; set; }
119121
}
120122
}
121123

‎tools/generator/Java.Interop.Tools.Generator.ObjectModel/Symbols/ISymbol.cs‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,23 @@ public interface ISymbol {
1515
string ElementType { get; }
1616
string ReturnCast { get; }
1717

18+
// Only apply ToNative/FromNative for marshal methods. This is used when
19+
// we want to change the marshal type to be blittable, like bool -> byte.
20+
// But it will not be used for normal invocations, like InvokeVirtualBooleanMethod, etc.
21+
bool OnlyFormatOnMarshal { get => false; }
22+
1823
string GetObjectHandleProperty (CodeGenerationOptions opt, string variable);
1924

2025
string GetGenericType (Dictionary<string, string> mappings);
2126

27+
string FromNative (CodeGenerationOptions opt, string var_name, bool owned, bool isMarshal = true)
28+
{
29+
if (OnlyFormatOnMarshal && !isMarshal)
30+
return var_name;
31+
32+
return FromNative (opt, var_name, owned);
33+
}
34+
2235
string FromNative (CodeGenerationOptions opt, string var_name, bool owned);
2336
string ToNative (CodeGenerationOptions opt, string var_name, Dictionary<string, string> mappings = null);
2437

‎tools/generator/Java.Interop.Tools.Generator.ObjectModel/Symbols/SymbolTable.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,9 +57,9 @@ public SymbolTable (CodeGenerationTarget target)
5757
this.target = target;
5858

5959
AddType (new SimpleSymbol ("IntPtr.Zero", "void", "void", "V"));
60-
AddType (new SimpleSymbol ("false", "boolean", "bool", "Z"));
60+
AddType (new SimpleSymbol ("false", "boolean", "bool", "Z", "sbyte", from_fmt: "{0} != 0", to_fmt: "{0} ? (sbyte)1 : (sbyte)0") { OnlyFormatOnMarshal = true });
6161
AddType (new SimpleSymbol ("0", "byte", "sbyte", "B"));
62-
AddType (new SimpleSymbol ("(char)0", "char", "char", "C"));
62+
AddType (new SimpleSymbol ("(char)0", "char", "char", "C", "ushort", from_fmt: "(char){0}", to_fmt: "(ushort){0}") { OnlyFormatOnMarshal = true });
6363
AddType (new SimpleSymbol ("0.0", "double", "double", "D"));
6464
AddType (new SimpleSymbol ("0.0F", "float", "float", "F"));
6565
AddType (new SimpleSymbol ("0", "int", "int", "I"));

‎tools/generator/SourceWriters/BoundFieldAsProperty.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ protected override void WriteGetterBody (CodeWriter writer)
108108
if (field.Symbol.IsArray) {
109109
writer.WriteLine ($"return global::Android.Runtime.JavaArray<{opt.GetOutputName (field.Symbol.ElementType)}>.FromJniHandle (__v.Handle, JniHandleOwnership.TransferLocalRef);");
110110
} else if (field.Symbol.NativeType != field.Symbol.FullName) {
111-
writer.WriteLine ($"return {field.Symbol.ReturnCast}{(field.Symbol.FromNative (opt, invokeType != "Object" ? "__v" : "__v.Handle", true) + opt.GetNullForgiveness (field))};");
111+
writer.WriteLine ($"return {field.Symbol.ReturnCast}{(field.Symbol.FromNative (opt, invokeType != "Object" ? "__v" : "__v.Handle", true, false) + opt.GetNullForgiveness (field))};");
112112
} else {
113113
writer.WriteLine ("return __v;");
114114
}

‎tools/generator/SourceWriters/Extensions/SourceWriterExtensions.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,7 @@ public static void AddMethodBodyTryBlock (List<string> body, Method method, Code
284284
if (opt.CodeGenerationTarget != CodeGenerationTarget.JavaInterop1 && invokeType == "Object") {
285285
r += ".Handle";
286286
}
287-
body.Add ($"\treturn {method.RetVal.ReturnCast}{method.RetVal.FromNative (opt, r, true) + opt.GetNullForgiveness (method.RetVal)};");
287+
body.Add ($"\treturn {method.RetVal.ReturnCast}{method.RetVal.FromNative (opt, r, true, false) + opt.GetNullForgiveness (method.RetVal)};");
288288
}
289289
}
290290

0 commit comments

Comments
 (0)