Skip to content

Commit 354bd57

Browse files
jonpryorjonpryor
committed
[generator] Fix Formatted() overloads & parameter aliases (#185)
Fixes: https://bugzilla.xamarin.com/show_bug.cgi?id=59472 Assume you have the following Java method: ```java class Example { public static CharSequence identity(CharSequence value) { return value; } } ``` When binding `Example.identity()`, we will *rename* it to have a `Formatted` suffix, and "overload" it with a `string` overload: ```csharp partial class Example { public static ICharSequence IdentityFormatted (ICharSequence value) { // ... } public static string Identity (string value) { // ... } } ``` The purpose of this overloading is to make it easier to call the method from C# code. It's not currently possible for interfaces to contain static methods, which in turn means it's not possible for interfaces to contain conversion operators. Thus, if a C# developer wants to do: Example.Identity ("value"); there would be no way to have this Just Work™ while having only `ICharSequence` running around. (Additionally, we rename the method so that we don't need to worry about return types. In the above example, the return type is converted, so if we didn't rename to `IdentityFormatted()`, we couldn't overload for many interesting use cases.) This feature has existed for quite some time, but there's a bug: `Example.Identity()` would do: ```csharp partial class Example { public static string Identity (string value) { var jls_value = value == null ? null : new Java.Lang.String (value) var __result = IdentityFormatted (jls_value); jls_value?.Dispose (); return __result?.ToString (); } } ``` This seems acceptable, but this will break when `Example.IdentityFormatted()` -- `Example.identity()` -- returns the parameter. When that happens, `jls_value` and `__result` are *the same instance*, and thus the `jls_value?.Dispose()` *also* disposes of `__result`. In this case, `__result.ToString()` will be `null`, which is *not* what is desired. The fix is to grab `__result.ToString()` *before* disposing of the temporary parameters: ```csharp partial class Example { public static string Identity (string value) { var jls_value = value == null ? null : new Java.Lang.String (value) var __result = IdentityFormatted (jls_value); var __rsval = __result?.ToString (); jls_value?.Dispose (); return __rsval; } } ``` This way we ensure that we get a copy before the source is disposed.
1 parent 6c1053c commit 354bd57

File tree

10 files changed

+495
-9
lines changed

10 files changed

+495
-9
lines changed

‎tools/generator/Method.cs‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -687,25 +687,28 @@ void GenerateStringOverloadBody (StreamWriter sw, string indent, CodeGenerationO
687687
call.Append (pname);
688688
}
689689
sw.WriteLine ("{0}{1}{2}{3} ({4});", indent, RetVal.IsVoid ? String.Empty : opt.GetOutputName (RetVal.FullName) + " __result = ", haveSelf ? "self." : "", AdjustedName, call.ToString ());
690-
foreach (Parameter p in Parameters) {
691-
if (p.Type == "Java.Lang.ICharSequence")
692-
sw.WriteLine ("{0}if ({1} != null) {1}.Dispose ();", indent, p.GetName ("jls_"));
693-
else if (p.Type == "Java.Lang.ICharSequence[]")
694-
sw.WriteLine ("{0}if ({1} != null) foreach (global::Java.Lang.String s in {1}) if (s != null) s.Dispose ();", indent, p.GetName ("jlca_"));
695-
}
696690
switch (RetVal.FullName) {
697691
case "void":
698692
break;
699693
case "Java.Lang.ICharSequence[]":
700-
sw.WriteLine ("{0}return CharSequence.ArrayToStringArray (__result);", indent);
694+
sw.WriteLine ("{0}var __rsval = CharSequence.ArrayToStringArray (__result);", indent);
701695
break;
702696
case "Java.Lang.ICharSequence":
703-
sw.WriteLine ("{0}return __result == null ? null : __result.ToString ();", indent);
697+
sw.WriteLine ("{0}var __rsval = __result?.ToString ();", indent);
704698
break;
705699
default:
706-
sw.WriteLine ("{0}return __result;", indent);
700+
sw.WriteLine ("{0}var __rsval = __result;", indent);
707701
break;
708702
}
703+
foreach (Parameter p in Parameters) {
704+
if (p.Type == "Java.Lang.ICharSequence")
705+
sw.WriteLine ("{0}{1}?.Dispose ();", indent, p.GetName ("jls_"));
706+
else if (p.Type == "Java.Lang.ICharSequence[]")
707+
sw.WriteLine ("{0}if ({1} != null) foreach (global::Java.Lang.String s in {1}) s?.Dispose ();", indent, p.GetName ("jlca_"));
708+
}
709+
if (!RetVal.IsVoid) {
710+
sw.WriteLine ($"{indent}return __rsval;");
711+
}
709712
}
710713

711714
void GenerateStringOverload (StreamWriter sw, string indent, CodeGenerationOptions opt)

‎tools/generator/Tests/SupportFiles/Android_Runtime_CharSequence.cs‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
using System;
2+
using System.Collections.Generic;
3+
4+
namespace Android.Runtime {
5+
6+
public static class CharSequence
7+
{
8+
public static Java.Lang.ICharSequence [] ArrayFromStringArray (string [] val)
9+
{
10+
throw new NotImplementedException ();
11+
}
12+
13+
public static string [] ArrayToStringArray (Java.Lang.ICharSequence [] val)
14+
{
15+
throw new NotImplementedException ();
16+
}
17+
18+
public static IntPtr ToLocalJniHandle (string value)
19+
{
20+
throw new NotImplementedException ();
21+
}
22+
23+
public static IntPtr ToLocalJniHandle (Java.Lang.ICharSequence value)
24+
{
25+
throw new NotImplementedException ();
26+
}
27+
28+
public static IntPtr ToLocalJniHandle (IEnumerable<char> value)
29+
{
30+
throw new NotImplementedException ();
31+
}
32+
}
33+
}

‎tools/generator/Tests/SupportFiles/Java_Lang_ICharSequence.cs‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
using System;
2+
using Android.Runtime;
3+
4+
namespace Java.Lang {
5+
6+
public partial interface ICharSequence : IJavaObject
7+
{
8+
char CharAt (int index);
9+
int Length ();
10+
Java.Lang.ICharSequence SubSequenceFormatted (int start, int end);
11+
string ToString ();
12+
}
13+
}

‎tools/generator/Tests/SupportFiles/Java_Lang_String.cs‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
using System;
2+
using System.Collections;
3+
using System.Collections.Generic;
4+
5+
namespace Java.Lang {
6+
7+
public sealed partial class String : global::Java.Lang.Object, Java.Lang.ICharSequence
8+
{
9+
public String (string value)
10+
{
11+
}
12+
13+
public char CharAt (int index)
14+
{
15+
throw new NotImplementedException ();
16+
}
17+
18+
public int Length ()
19+
{
20+
throw new NotImplementedException ();
21+
}
22+
23+
public Java.Lang.ICharSequence SubSequenceFormatted (int start, int end)
24+
{
25+
throw new NotImplementedException ();
26+
}
27+
28+
public override string ToString ()
29+
{
30+
throw new NotImplementedException ();
31+
}
32+
33+
public IEnumerator<char> GetEnumerator ()
34+
{
35+
throw new NotImplementedException ();
36+
}
37+
38+
IEnumerator IEnumerable.GetEnumerator ()
39+
{
40+
throw new NotImplementedException ();
41+
}
42+
}
43+
}

‎tools/generator/Tests/expected.ji/TestInterface/Test.ME.ITestInterface.cs‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,33 @@ public partial interface ITestInterface : IJavaObject {
4848
[Register ("getSpanFlags", "(Ljava/lang/Object;)I", "GetGetSpanFlags_Ljava_lang_Object_Handler:Test.ME.ITestInterfaceInvoker, Mono.Android, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null")]
4949
int GetSpanFlags (global::Java.Lang.Object tag);
5050

51+
// Metadata.xml XPath method reference: path="/api/package[@name='test.me']/interface[@name='TestInterface']/method[@name='append' and count(parameter)=1 and parameter[1][@type='java.lang.CharSequence']]"
52+
[Register ("append", "(Ljava/lang/CharSequence;)V", "GetAppend_Ljava_lang_CharSequence_Handler:Test.ME.ITestInterfaceInvoker, Mono.Android, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null")]
53+
void Append (global::Java.Lang.ICharSequence value);
54+
55+
// Metadata.xml XPath method reference: path="/api/package[@name='test.me']/interface[@name='TestInterface']/method[@name='identity' and count(parameter)=1 and parameter[1][@type='java.lang.CharSequence']]"
56+
[Register ("identity", "(Ljava/lang/CharSequence;)Ljava/lang/CharSequence;", "GetIdentity_Ljava_lang_CharSequence_Handler:Test.ME.ITestInterfaceInvoker, Mono.Android, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null")]
57+
global::Java.Lang.ICharSequence IdentityFormatted (global::Java.Lang.ICharSequence value);
58+
59+
}
60+
61+
public static partial class ITestInterfaceExtensions {
62+
63+
public static void Append (this Test.ME.ITestInterface self, string value)
64+
{
65+
global::Java.Lang.String jls_value = value == null ? null : new global::Java.Lang.String (value);
66+
self.Append (jls_value);
67+
jls_value?.Dispose ();
68+
}
69+
70+
public static string Identity (this Test.ME.ITestInterface self, string value)
71+
{
72+
global::Java.Lang.String jls_value = value == null ? null : new global::Java.Lang.String (value);
73+
global::Java.Lang.ICharSequence __result = self.IdentityFormatted (jls_value);
74+
var __rsval = __result?.ToString ();
75+
jls_value?.Dispose ();
76+
return __rsval;
77+
}
5178
}
5279

5380
[global::Android.Runtime.Register ("test/me/TestInterface", DoNotGenerateAcw=true)]
@@ -130,6 +157,66 @@ public unsafe int GetSpanFlags (global::Java.Lang.Object tag)
130157
return __ret;
131158
}
132159

160+
static Delegate cb_append_Ljava_lang_CharSequence_;
161+
#pragma warning disable 0169
162+
static Delegate GetAppend_Ljava_lang_CharSequence_Handler ()
163+
{
164+
if (cb_append_Ljava_lang_CharSequence_ == null)
165+
cb_append_Ljava_lang_CharSequence_ = JNINativeWrapper.CreateDelegate ((Action<IntPtr, IntPtr, IntPtr>) n_Append_Ljava_lang_CharSequence_);
166+
return cb_append_Ljava_lang_CharSequence_;
167+
}
168+
169+
static void n_Append_Ljava_lang_CharSequence_ (IntPtr jnienv, IntPtr native__this, IntPtr native_value)
170+
{
171+
global::Test.ME.ITestInterface __this = global::Java.Lang.Object.GetObject<global::Test.ME.ITestInterface> (jnienv, native__this, JniHandleOwnership.DoNotTransfer);
172+
global::Java.Lang.ICharSequence value = global::Java.Lang.Object.GetObject<global::Java.Lang.ICharSequence> (native_value, JniHandleOwnership.DoNotTransfer);
173+
__this.Append (value);
174+
}
175+
#pragma warning restore 0169
176+
177+
IntPtr id_append_Ljava_lang_CharSequence_;
178+
public unsafe void Append (global::Java.Lang.ICharSequence value)
179+
{
180+
if (id_append_Ljava_lang_CharSequence_ == IntPtr.Zero)
181+
id_append_Ljava_lang_CharSequence_ = JNIEnv.GetMethodID (class_ref, "append", "(Ljava/lang/CharSequence;)V");
182+
IntPtr native_value = CharSequence.ToLocalJniHandle (value);
183+
JValue* __args = stackalloc JValue [1];
184+
__args [0] = new JValue (native_value);
185+
JNIEnv.CallVoidMethod (((global::Java.Lang.Object) this).Handle, id_append_Ljava_lang_CharSequence_, __args);
186+
JNIEnv.DeleteLocalRef (native_value);
187+
}
188+
189+
static Delegate cb_identity_Ljava_lang_CharSequence_;
190+
#pragma warning disable 0169
191+
static Delegate GetIdentity_Ljava_lang_CharSequence_Handler ()
192+
{
193+
if (cb_identity_Ljava_lang_CharSequence_ == null)
194+
cb_identity_Ljava_lang_CharSequence_ = JNINativeWrapper.CreateDelegate ((Func<IntPtr, IntPtr, IntPtr, IntPtr>) n_Identity_Ljava_lang_CharSequence_);
195+
return cb_identity_Ljava_lang_CharSequence_;
196+
}
197+
198+
static IntPtr n_Identity_Ljava_lang_CharSequence_ (IntPtr jnienv, IntPtr native__this, IntPtr native_value)
199+
{
200+
global::Test.ME.ITestInterface __this = global::Java.Lang.Object.GetObject<global::Test.ME.ITestInterface> (jnienv, native__this, JniHandleOwnership.DoNotTransfer);
201+
global::Java.Lang.ICharSequence value = global::Java.Lang.Object.GetObject<global::Java.Lang.ICharSequence> (native_value, JniHandleOwnership.DoNotTransfer);
202+
IntPtr __ret = CharSequence.ToLocalJniHandle (__this.IdentityFormatted (value));
203+
return __ret;
204+
}
205+
#pragma warning restore 0169
206+
207+
IntPtr id_identity_Ljava_lang_CharSequence_;
208+
public unsafe global::Java.Lang.ICharSequence IdentityFormatted (global::Java.Lang.ICharSequence value)
209+
{
210+
if (id_identity_Ljava_lang_CharSequence_ == IntPtr.Zero)
211+
id_identity_Ljava_lang_CharSequence_ = JNIEnv.GetMethodID (class_ref, "identity", "(Ljava/lang/CharSequence;)Ljava/lang/CharSequence;");
212+
IntPtr native_value = CharSequence.ToLocalJniHandle (value);
213+
JValue* __args = stackalloc JValue [1];
214+
__args [0] = new JValue (native_value);
215+
global::Java.Lang.ICharSequence __ret = global::Java.Lang.Object.GetObject<Java.Lang.ICharSequence> (JNIEnv.CallObjectMethod (((global::Java.Lang.Object) this).Handle, id_identity_Ljava_lang_CharSequence_, __args), JniHandleOwnership.TransferLocalRef);
216+
JNIEnv.DeleteLocalRef (native_value);
217+
return __ret;
218+
}
219+
133220
}
134221

135222
}

‎tools/generator/Tests/expected.ji/TestInterface/Test.ME.TestInterfaceImplementation.cs‎

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,65 @@ static int n_GetSpanFlags_Ljava_lang_Object_ (IntPtr jnienv, IntPtr native__this
9292
[Register ("getSpanFlags", "(Ljava/lang/Object;)I", "GetGetSpanFlags_Ljava_lang_Object_Handler")]
9393
public abstract int GetSpanFlags (global::Java.Lang.Object tag);
9494

95+
static Delegate cb_append_Ljava_lang_CharSequence_;
96+
#pragma warning disable 0169
97+
static Delegate GetAppend_Ljava_lang_CharSequence_Handler ()
98+
{
99+
if (cb_append_Ljava_lang_CharSequence_ == null)
100+
cb_append_Ljava_lang_CharSequence_ = JNINativeWrapper.CreateDelegate ((Action<IntPtr, IntPtr, IntPtr>) n_Append_Ljava_lang_CharSequence_);
101+
return cb_append_Ljava_lang_CharSequence_;
102+
}
103+
104+
static void n_Append_Ljava_lang_CharSequence_ (IntPtr jnienv, IntPtr native__this, IntPtr native_value)
105+
{
106+
global::Test.ME.TestInterfaceImplementation __this = global::Java.Lang.Object.GetObject<global::Test.ME.TestInterfaceImplementation> (jnienv, native__this, JniHandleOwnership.DoNotTransfer);
107+
global::Java.Lang.ICharSequence value = global::Java.Lang.Object.GetObject<global::Java.Lang.ICharSequence> (native_value, JniHandleOwnership.DoNotTransfer);
108+
__this.Append (value);
109+
}
110+
#pragma warning restore 0169
111+
112+
// Metadata.xml XPath method reference: path="/api/package[@name='test.me']/interface[@name='TestInterface']/method[@name='append' and count(parameter)=1 and parameter[1][@type='java.lang.CharSequence']]"
113+
[Register ("append", "(Ljava/lang/CharSequence;)V", "GetAppend_Ljava_lang_CharSequence_Handler")]
114+
public abstract void Append (global::Java.Lang.ICharSequence value);
115+
116+
public void Append (string value)
117+
{
118+
global::Java.Lang.String jls_value = value == null ? null : new global::Java.Lang.String (value);
119+
Append (jls_value);
120+
jls_value?.Dispose ();
121+
}
122+
123+
static Delegate cb_identity_Ljava_lang_CharSequence_;
124+
#pragma warning disable 0169
125+
static Delegate GetIdentity_Ljava_lang_CharSequence_Handler ()
126+
{
127+
if (cb_identity_Ljava_lang_CharSequence_ == null)
128+
cb_identity_Ljava_lang_CharSequence_ = JNINativeWrapper.CreateDelegate ((Func<IntPtr, IntPtr, IntPtr, IntPtr>) n_Identity_Ljava_lang_CharSequence_);
129+
return cb_identity_Ljava_lang_CharSequence_;
130+
}
131+
132+
static IntPtr n_Identity_Ljava_lang_CharSequence_ (IntPtr jnienv, IntPtr native__this, IntPtr native_value)
133+
{
134+
global::Test.ME.TestInterfaceImplementation __this = global::Java.Lang.Object.GetObject<global::Test.ME.TestInterfaceImplementation> (jnienv, native__this, JniHandleOwnership.DoNotTransfer);
135+
global::Java.Lang.ICharSequence value = global::Java.Lang.Object.GetObject<global::Java.Lang.ICharSequence> (native_value, JniHandleOwnership.DoNotTransfer);
136+
IntPtr __ret = CharSequence.ToLocalJniHandle (__this.IdentityFormatted (value));
137+
return __ret;
138+
}
139+
#pragma warning restore 0169
140+
141+
// Metadata.xml XPath method reference: path="/api/package[@name='test.me']/interface[@name='TestInterface']/method[@name='identity' and count(parameter)=1 and parameter[1][@type='java.lang.CharSequence']]"
142+
[Register ("identity", "(Ljava/lang/CharSequence;)Ljava/lang/CharSequence;", "GetIdentity_Ljava_lang_CharSequence_Handler")]
143+
public abstract global::Java.Lang.ICharSequence IdentityFormatted (global::Java.Lang.ICharSequence value);
144+
145+
public string Identity (string value)
146+
{
147+
global::Java.Lang.String jls_value = value == null ? null : new global::Java.Lang.String (value);
148+
global::Java.Lang.ICharSequence __result = IdentityFormatted (jls_value);
149+
var __rsval = __result?.ToString ();
150+
jls_value?.Dispose ();
151+
return __rsval;
152+
}
153+
95154
}
96155

97156
[global::Android.Runtime.Register ("test/me/TestInterfaceImplementation", DoNotGenerateAcw=true)]
@@ -123,6 +182,46 @@ public override unsafe int GetSpanFlags (global::Java.Lang.Object tag)
123182
}
124183
}
125184

185+
// Metadata.xml XPath method reference: path="/api/package[@name='test.me']/interface[@name='TestInterface']/method[@name='append' and count(parameter)=1 and parameter[1][@type='java.lang.CharSequence']]"
186+
[Register ("append", "(Ljava/lang/CharSequence;)V", "GetAppend_Ljava_lang_CharSequence_Handler")]
187+
public override unsafe void Append (global::Java.Lang.ICharSequence value)
188+
{
189+
const string __id = "append.(Ljava/lang/CharSequence;)V";
190+
IntPtr native_value = CharSequence.ToLocalJniHandle (value);
191+
try {
192+
JniArgumentValue* __args = stackalloc JniArgumentValue [1];
193+
__args [0] = new JniArgumentValue (native_value);
194+
_members.InstanceMethods.InvokeAbstractVoidMethod (__id, this, __args);
195+
} finally {
196+
JNIEnv.DeleteLocalRef (native_value);
197+
}
198+
}
199+
200+
// Metadata.xml XPath method reference: path="/api/package[@name='test.me']/interface[@name='TestInterface']/method[@name='identity' and count(parameter)=1 and parameter[1][@type='java.lang.CharSequence']]"
201+
[Register ("identity", "(Ljava/lang/CharSequence;)Ljava/lang/CharSequence;", "GetIdentity_Ljava_lang_CharSequence_Handler")]
202+
public override unsafe global::Java.Lang.ICharSequence IdentityFormatted (global::Java.Lang.ICharSequence value)
203+
{
204+
const string __id = "identity.(Ljava/lang/CharSequence;)Ljava/lang/CharSequence;";
205+
IntPtr native_value = CharSequence.ToLocalJniHandle (value);
206+
try {
207+
JniArgumentValue* __args = stackalloc JniArgumentValue [1];
208+
__args [0] = new JniArgumentValue (native_value);
209+
var __rm = _members.InstanceMethods.InvokeAbstractObjectMethod (__id, this, __args);
210+
return global::Java.Lang.Object.GetObject<Java.Lang.ICharSequence> (__rm.Handle, JniHandleOwnership.TransferLocalRef);
211+
} finally {
212+
JNIEnv.DeleteLocalRef (native_value);
213+
}
214+
}
215+
216+
public string Identity (string value)
217+
{
218+
global::Java.Lang.String jls_value = value == null ? null : new global::Java.Lang.String (value);
219+
global::Java.Lang.ICharSequence __result = IdentityFormatted (jls_value);
220+
var __rsval = __result?.ToString ();
221+
jls_value?.Dispose ();
222+
return __rsval;
223+
}
224+
126225
}
127226

128227
}

0 commit comments

Comments
 (0)