Skip to content

Commit

Permalink
[jcw-gen] Use + for nested types, not / (#1304)
Browse files Browse the repository at this point in the history
Context: dotnet/android#9747
Context: https://discord.com/channels/732297728826277939/732297837953679412/1336353039031734352
Context: https://discord.com/channels/732297728826277939/732297837953679412/1336358257769316372
Context: #1302
Context: dotnet/android#9750

The `[Register]` attribute provides "connector method" names, and for
interface methods this will also include the name of the type which
declares the method, which itself may be in a nested type:

	namespace Android.App {
	  public partial class Application {
	    public partial interface IActivityLifecycleCallbacks : IJavaObject, IJavaPeerable {
	      [Register (
	          name: "onActivityCreated",
	          signature: "(Landroid/app/Activity;Landroid/os/Bundle;)V",
	          connector: "GetOnActivityCreated_Landroid_app_Activity_Landroid_os_Bundle_Handler:Android.App.Application/IActivityLifecycleCallbacksInvoker, Mono.Android, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null")]
	      void OnActivityCreated (Android.App.Activity activity, Android.OS.Bundle? savedInstanceState);

	      // …
	    }
	  }
	}

The `connector` value is copied as-is into Java Callable Wrappers,
as part of the `__md_methods` value and `Runtime.register()` call.
Given the C# type:

	partial class MauiApplication : Application {
	  partial class ActivityLifecycleCallbacks : Java.Lang.Object, Application.IActivityLifecycleCallbacks {
	    public void OnActivityCreated (Activity activity, Bundle? savedInstanceState) => …
	  }
	}

then `Java.Interop.Tools.JavaCallableWrappers` will produce:

	// Java Callable Wrapper
	/* partial */ class MauiApplication_ActivityLifecycleCallbacks
	{
	  public static final String __md_methods;
	  static {
	    __md_methods =
	      // …
	      "n_onActivityCreated:(Landroid/app/Activity;Landroid/os/Bundle;)V:GetOnActivityCreated_Landroid_app_Activity_Landroid_os_Bundle_Handler:Android.App.Application/IActivityLifecycleCallbacksInvoker, Mono.Android, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null\n" +
	      // …
	      "";
	    mono.android.Runtime.register ("Microsoft.Maui.MauiApplication+ActivityLifecycleCallbacks, Microsoft.Maui", MauiApplication_ActivityLifecycleCallbacks.class, __md_methods);
	  }
	}

The `signature` and `connector` values from the `[Register(…)]` on
the method declaration are copied as-is into `__md_methods`.
As the `connector` value contains a `/`, `__md_methods` does as well.

This has worked fine for nearly 15+ years…on Mono/MonoVM.

This *fails* on NativeAOT and CoreCLR, as:

	Type.GetType ("Android.App.Application/IActivityLifecycleCallbacksInvoker, Mono.Android, …", throwOnError:true)

fails with:

	TypeLoadException: Could not resolve type 'Android.App.Application/IActivityLifecycleCallbacksInvoker' in assembly 'Mono.Android, …'.

The reason for the failure is that when using Reflection APIs such as
`Type.GetType()`, the [`Type.AssemblyQualifiedName` grammar][0] must
be followed, and that grammar uses `+` to "Precede a nested class",
*not* `/`.  (`/` isn't even in the Reflection grammar!)

(Aside: where does `/` come from then?  It's the *IL* separator for
nested types!)

For eventual CoreCLR and NativeAOT support, then, we need to replace
`/` with `+` *somewhere* before it hits `Type.GetType()`.

There are (at least?) three places to do so:

 1. Within `JniRuntime.JniTypeManager.RegisterNativeMembers()` or
    equivalent override.

 2. Within `generator`, updating the contents of `[Register]`.

 3. Within `Java.Interop.Tools.JavaCallableWrappers`.

(1) is rejected out of hand as it would be additional work done on-
device at runtime.  Why do that if we don't need to?

(2) was attempted in #1302 & dotnet/android#9750.
It turned into a bit of a boondoggle, because there are lots of
linker steps which interpret the `connector` value on `[Register]`,
and it "just worked" that the `connector` value contained IL names,
as the linker steps deal in IL!  Trying to update `connector` to
instead contain Reflection syntax required finding all the places in
the linker that used `connector` values, which was tedious & brittle.

"Just" (2) is also inadequate, as it would require new binding
assemblies to take advantage of, so (2) *also* needed (3).

Which brings us to (3), the current approach: *don't* alter the
semantics of the `connect` value within `[Register]`, and instead
require that `Java.Interop.Tools.JavaCallableWrappers` replace all
instances of `/` with `+` within `__md_methods`.  This is needed
*anyway*, for compatibility with existing bindings, and also avoids
lots of pain that (2) encountered.

With this approach, `generator` output is unchanged, and
`jcw-gen` output for `MauiApplication.ActivityLifecycleCallbacks`
becomes:

	// Java Callable Wrapper
	/* partial */ class MauiApplication_ActivityLifecycleCallbacks
	{
	  public static final String __md_methods;
	  static {
	    __md_methods =
	      // …
	      "n_onActivityCreated:(Landroid/app/Activity;Landroid/os/Bundle;)V:GetOnActivityCreated_Landroid_app_Activity_Landroid_os_Bundle_Handler:Android.App.Application+IActivityLifecycleCallbacksInvoker, Mono.Android, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null\n" +
	      // …
	      "";
	    mono.android.Runtime.register ("Microsoft.Maui.MauiApplication+ActivityLifecycleCallbacks, Microsoft.Maui", MauiApplication_ActivityLifecycleCallbacks.class, __md_methods);
	  }
	}

[0]: https://learn.microsoft.com/en-us/dotnet/api/system.type.assemblyqualifiedname?view=net-9.0#remarks
  • Loading branch information
jonpryor authored Feb 6, 2025
1 parent dd3c1d0 commit 6bc87e8
Show file tree
Hide file tree
Showing 7 changed files with 96 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ static CallableWrapperMethod CreateMethod (MethodDefinition methodDefinition, Ca
static CallableWrapperMethod CreateMethod (string name, CallableWrapperType declaringType, string? signature, string? connector, string? managedParameters, string? outerType, string? superCall)
{
signature = signature ?? throw new ArgumentNullException ("`connector` cannot be null.", nameof (connector));
var method_name = "n_" + name + ":" + signature + ":" + connector;
var method_name = "n_" + name + ":" + signature + ":" + connector?.Replace ('/', '+');

var method = new CallableWrapperMethod (declaringType, name, method_name, signature);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ static string ToCliTypePart (string part)
for (int i = 0; i < parts.Length; ++i) {
parts [i] = ToPascalCase (parts [i], 1);
}
return string.Join ("/", parts);
return string.Join ("+", parts);
}

static string ToPascalCase (string value, int minLength)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@
<ProjectReference Include="..\..\src\Java.Interop.Export\Java.Interop.Export.csproj" />
<ProjectReference Include="..\..\src\Java.Runtime.Environment\Java.Runtime.Environment.csproj" />
<ProjectReference Include="..\TestJVM\TestJVM.csproj" />
<ProjectReference
Include="..\..\tools\jcw-gen\jcw-gen.csproj"
ReferenceOutputAssembly="false"
/>
</ItemGroup>

<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,55 @@ public void monodroidClearReferences ()
Assert.AreEqual (expected, actual);
}

[Test]
public void GenerateTypeMentioningNestedInvoker ()
{
var actual = Generate (typeof (ApplicationName.ActivityLifecycleCallbacks));
var expected = """
package application;
public class Name_ActivityLifecycleCallbacks
extends java.lang.Object
implements
mono.android.IGCUserPeer,
java.lang.Object
{
/** @hide */
public static final String __md_methods;
static {
__md_methods =
"n_onActivityCreated:(Landroid/app/Activity;Landroid/os/Bundle;)V:GetOnActivityCreated_Landroid_app_Activity_Landroid_os_Bundle_Handler:Android.App.Application+IActivityLifecycleCallbacksInvoker, Mono.Android\n" +
"";
mono.android.Runtime.register ("Xamarin.Android.ToolsTests.ApplicationName+ActivityLifecycleCallbacks, Java.Interop.Tools.JavaCallableWrappers-Tests", Name_ActivityLifecycleCallbacks.class, __md_methods);
}
public void onActivityCreated (android.app.Activity p0, android.os.Bundle p1)
{
n_onActivityCreated (p0, p1);
}
private native void n_onActivityCreated (android.app.Activity p0, android.os.Bundle p1);
private java.util.ArrayList refList;
public void monodroidAddReference (java.lang.Object obj)
{
if (refList == null)
refList = new java.util.ArrayList ();
refList.add (obj);
}
public void monodroidClearReferences ()
{
if (refList != null)
refList.clear ();
}
}
""";
Assert.AreEqual (expected, actual);
}

static string Generate (Type type, string applicationJavaClass = null, string monoRuntimeInit = null, JavaPeerStyle style = JavaPeerStyle.XAJavaInterop1)
{
var reader_options = new CallableWrapperReaderOptions {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,26 @@ class Application : Java.Lang.Object
protected virtual void OnCreate ()
{
}

[Register ("android/app/Application$ActivityLifecycleCallbacks", DoNotGenerateAcw = true)]
public partial interface IActivityLifecycleCallbacks {
[Register (
name: "onActivityCreated",
signature: "(Landroid/app/Activity;Landroid/os/Bundle;)V",
connector: "GetOnActivityCreated_Landroid_app_Activity_Landroid_os_Bundle_Handler:Android.App.Application/IActivityLifecycleCallbacksInvoker, Mono.Android")]
void OnActivityCreated (Android.App.Activity activity, global::Android.OS.Bundle savedInstanceState);
}

internal class IActivityLifecycleCallbacksInvoker : Java.Lang.Object, IActivityLifecycleCallbacks {
static Delegate GetOnActivityCreated_Landroid_app_Activity_Landroid_os_Bundle_Handler ()
{
return null;
}

public void OnActivityCreated (Android.App.Activity activity, global::Android.OS.Bundle savedInstanceState)
{
}
}
}

[Register ("android/app/Activity", DoNotGenerateAcw = true)]
Expand All @@ -43,6 +63,14 @@ public virtual void OnCreate (Java.Lang.Object arguments)
}
}

namespace Android.OS {

[Register ("android/os/Bundle", DoNotGenerateAcw = true)]
class Bundle : Java.Lang.Object
{
}
}

namespace Android.Runtime {

interface IJavaObject
Expand Down Expand Up @@ -72,6 +100,7 @@ static class SupportDeclarations
typeof (AbstractClass),
typeof (ActivityName),
typeof (ApplicationName),
typeof (ApplicationName.ActivityLifecycleCallbacks),
typeof (DefaultName),
typeof (DefaultName.A),
typeof (DefaultName.A.B),
Expand Down Expand Up @@ -139,6 +168,12 @@ class ActivityName : Java.Lang.Object
[Application (Name = "application.Name")]
class ApplicationName : Application
{
public class ActivityLifecycleCallbacks : Java.Lang.Object, Application.IActivityLifecycleCallbacks
{
public void OnActivityCreated (Activity activity, global::Android.OS.Bundle savedInstanceState)
{
}
}
}

class IndirectApplication : ApplicationName
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public void WriteJavaToManaged ()
v.WriteJavaToManaged (o);
var a = ToArray (o);
Save (a, "__j2m");
var length = 190;
var length = 193;
var offset = 76;
var e =
"version=1\u0000" +
Expand All @@ -59,6 +59,7 @@ public void WriteJavaToManaged ()
"value-offset=" + offset + "\u0000" +
GetJ2MEntryLine (typeof (ActivityName), "activity/Name", offset, length) +
GetJ2MEntryLine (typeof (ApplicationName), "application/Name", offset, length) +
GetJ2MEntryLine (typeof (ApplicationName.ActivityLifecycleCallbacks), "application/Name_ActivityLifecycleCallbacks", offset, length) +
GetJ2MEntryLine (typeof (DefaultName), "crc64197ae30a36756915/DefaultName", offset, length) +
GetJ2MEntryLine (typeof (DefaultName.A), "crc64197ae30a36756915/DefaultName_A", offset, length) +
GetJ2MEntryLine (typeof (DefaultName.A.B), "crc64197ae30a36756915/DefaultName_A_B", offset, length) +
Expand Down Expand Up @@ -128,8 +129,8 @@ public void WriteManagedToJava ()
v.WriteManagedToJava (o);
var a = ToArray (o);
Save (a, "__m2j");
var length = 190;
var offset = 114;
var length = 193;
var offset = 117;
var e =
"version=1\u0000" +
$"entry-count={types.Count}\u0000" +
Expand All @@ -138,6 +139,7 @@ public void WriteManagedToJava ()
GetM2JEntryLine (typeof (AbstractClass), "my/AbstractClass", offset, length) +
GetM2JEntryLine (typeof (AbstractClassInvoker), "my/AbstractClass", offset, length) +
GetM2JEntryLine (typeof (ActivityName), "activity/Name", offset, length) +
GetM2JEntryLine (typeof (ApplicationName.ActivityLifecycleCallbacks), "application/Name_ActivityLifecycleCallbacks", offset, length) +
GetM2JEntryLine (typeof (ApplicationName), "application/Name", offset, length) +
GetM2JEntryLine (typeof (DefaultName.A.B), "crc64197ae30a36756915/DefaultName_A_B", offset, length) +
GetM2JEntryLine (typeof (DefaultName.A), "crc64197ae30a36756915/DefaultName_A", offset, length) +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ public static void GenerateTypeRegistrations (CodeGenerationOptions opt, Generat
int ls = reg.Key.LastIndexOf ('/');
string package = ls >= 0 ? reg.Key.Substring (0, ls) : "";

if (JavaNativeTypeManager.ToCliType (reg.Key) == reg.Value)
if (JavaNativeTypeManager.ToCliType (reg.Key) == reg.Value.Replace ('/', '+'))
continue;
if (!mapping.TryGetValue (package, out var v))
mapping.Add (package, v = new List<KeyValuePair<string, string>> ());
Expand Down

0 comments on commit 6bc87e8

Please sign in to comment.