Skip to content

Commit

Permalink
[jnimarshalmethod-gen] JavaInterop1 marshal methods (#1164)
Browse files Browse the repository at this point in the history
Fixes: #1159

Context: c93fea0

`jnimarshalmethod-gen` is intended to generate marshal methods for
any type that that:

  * Has `[JavaCallable]` (tested in `Java.Interop.Export-Tests.dll`
    and c93fea0), or
  * Overrides a `virtual` method which has `[JniMethodSignature]`, or
  * Implements an interface method which has `[JniMethodSignature]`.

Thus, the intention was that it should generate marshal methods for
`Java.Base-Tests`:

	% dotnet bin/Debug-net7.0/jnimarshalmethod-gen.dll -v bin/TestDebug-net7.0/Java.Base-Tests.dll
	Unable to read assembly 'bin/TestDebug-net7.0/Java.Base-Tests.dll' with symbols. Retrying to load it without them.
	Preparing marshal method assembly 'Java.Base-Tests-JniMarshalMethods'
	Processing Java.BaseTests.JavaInvoker type
	Processing Java.BaseTests.MyRunnable type
	Processing Java.BaseTests.MyIntConsumer type
	Marshal method assembly 'Java.Base-Tests-JniMarshalMethods' created

Notably missing? No messages stating:

	Adding marshal method for …

Also missing?  `ikdasm bin/TestDebug-net7.0/Java.Base-Tests.dll`
showed that there were no `__RegisterNativeMembers()` methods emitted.

The `jnimarshalmethod-gen` invocation was a no-op!

The problems were twofold:

 1. It was only looking for methods with
    `Android.Runtime.RegisterAttribute`.  This was useful for
    Xamarin.Android (when we were trying to make it work), but
    doesn't work with Java.Base.  We need to *also* look for
    `Java.Interop.JniMethodSignature`.

    Relatedly, the attempt to use
    `registerAttribute.Constructor.Parameters` to determine parameter
    names didn't work; the parameter name was always `""`.

 2. A'la c93fea0, we need to ensure that the Java `native` methods
    we register are consistent with `jcw-gen` output.

Fix these two problems, which allows `jnimarshalmethod-gen` to now
emit marshal methods for types within `Java.Base-Tests.dll`.

Additionally, rework the `jnimarshalmethod-gen -f` logic to *remove*
the existing `__<$>_jni_marshal_methods` nested type when present.
  • Loading branch information
jonpryor authored Nov 20, 2023
1 parent 320636d commit 25850ba
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 30 deletions.
20 changes: 20 additions & 0 deletions build-tools/automation/templates/core-tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,26 @@ steps:
continueOnError: true
retryCountOnTaskFailure: 1

- task: DotNetCoreCLI@2
displayName: 'jnimarshalmethod-gen Java.Base-Tests.dll'
condition: or(eq('${{ parameters.runNativeDotnetTests }}', 'true'), eq('${{ parameters.runNativeTests }}', 'true'))
inputs:
command: custom
custom: bin/$(Build.Configuration)$(NetCoreTargetFrameworkPathSuffix)/jnimarshalmethod-gen.dll
arguments: bin/Test$(Build.Configuration)$(NetCoreTargetFrameworkPathSuffix)/Java.Base-Tests.dll -v -v --keeptemp
continueOnError: true
retryCountOnTaskFailure: 1

- task: DotNetCoreCLI@2
displayName: 'Tests: Java.Base'
condition: or(eq('${{ parameters.runNativeDotnetTests }}', 'true'), eq('${{ parameters.runNativeTests }}', 'true'))
inputs:
command: test
testRunTitle: Java.Base ($(DotNetTargetFramework) - ${{ parameters.platformName }})
arguments: bin/Test$(Build.Configuration)$(NetCoreTargetFrameworkPathSuffix)/Java.Base-Tests.dll
continueOnError: true
retryCountOnTaskFailure: 1

- task: DotNetCoreCLI@2
displayName: 'Tests: java-source-utils'
inputs:
Expand Down
48 changes: 18 additions & 30 deletions tools/jnimarshalmethod-gen/App.cs
Original file line number Diff line number Diff line change
Expand Up @@ -476,10 +476,14 @@ void CreateMarshalMethodAssembly (string path)
continue;

var existingMarshalMethodsType = td.GetNestedType (TypeMover.NestedName);
if (existingMarshalMethodsType != null && !forceRegeneration) {
Warning (Message.WarningMarshalMethodsTypeAlreadyExists, existingMarshalMethodsType.GetAssemblyQualifiedName (cache), assemblyName);
if (existingMarshalMethodsType != null) {

return;
if (forceRegeneration) {
td.NestedTypes.Remove (existingMarshalMethodsType);
} else {
Warning (Message.WarningMarshalMethodsTypeAlreadyExists, existingMarshalMethodsType.GetAssemblyQualifiedName (cache), assemblyName);
return;
}
}

if (Verbose)
Expand Down Expand Up @@ -512,7 +516,7 @@ void CreateMarshalMethodAssembly (string path)

if (exportObj != null) {
dynamic e = exportObj;
name = e.Name;
name = "n_" + methodName;
signature = e.Signature;
}
else {
Expand Down Expand Up @@ -550,15 +554,14 @@ void CreateMarshalMethodAssembly (string path)
Log (TraceLevel.Verbose, $"## Dumping contents of marshal method for `{td.FullName}::{method.Name}({string.Join (", ", method.GetParameters ().Select (p => p.ParameterType))})`:");
Console.WriteLine (lambda.ToCSharpCode ());
#endif // _DUMP_REGISTER_NATIVE_MEMBERS
name = export?.Name ?? method.Name;

var mmDef = assemblyBuilder.Compile (lambda);
mmDef.Name = name;
mmTypeDef.Methods.Add (mmDef);

signature = export?.Signature ?? builder.GetJniMethodSignature (method);

registrations.Add (new ExpressionMethodRegistration ("n_" + method.Name, signature, mmDef));
registrations.Add (new ExpressionMethodRegistration (name, signature, mmDef));

addedMethods.Add (methodName);
}
Expand Down Expand Up @@ -739,35 +742,19 @@ public static MethodDefinition GetMethodDefinition (this TypeDefinition td, Meth

static bool CheckMethod (MethodDefinition m, ref string name, ref string methodName, ref string signature)
{
foreach (var registerAttribute in m.GetCustomAttributes ("Android.Runtime.RegisterAttribute")) {
var registerAttrs = m.GetCustomAttributes ("Android.Runtime.RegisterAttribute")
.Concat (m.GetCustomAttributes ("Java.Interop.JniMethodSignatureAttribute"));
foreach (var registerAttribute in registerAttrs) {
if (registerAttribute == null || !registerAttribute.HasConstructorArguments)
continue;

var constructorParameters = registerAttribute.Constructor.Parameters.ToArray ();
var constructorArguments = registerAttribute.ConstructorArguments.ToArray ();

for (int i = 0; i < constructorArguments.Length; i++) {
switch (constructorParameters [i].Name) {
case "name":
name = constructorArguments [i].Value.ToString ();
break;
case "signature":
signature = constructorArguments [i].Value.ToString ();
break;
}

}

if ((string.IsNullOrEmpty (name) || string.IsNullOrEmpty (signature)) && constructorArguments.Length != 3)
if (registerAttribute.ConstructorArguments.Count < 2)
continue;

if (string.IsNullOrEmpty (name))
name = constructorArguments [0].Value.ToString ();

if (string.IsNullOrEmpty (signature))
signature = constructorArguments [1].Value.ToString ();
name = registerAttribute.ConstructorArguments [0].Value.ToString ();
signature = registerAttribute.ConstructorArguments [1].Value.ToString ();

if (string.IsNullOrEmpty (name) || string.IsNullOrEmpty (signature))
if ((string.IsNullOrEmpty (name) || string.IsNullOrEmpty (signature)))
continue;

methodName = MarshalMemberBuilder.GetMarshalMethodName (name, signature);
Expand Down Expand Up @@ -808,13 +795,14 @@ public static bool NeedsMarshalMethod (this MethodDefinition md, DirectoryAssemb
continue;
}

for (int i = 0; i < ifaceMap.TargetMethods.Length; i++)
for (int i = 0; i < ifaceMap.TargetMethods.Length; i++) {
if (ifaceMap.TargetMethods [i] == method) {
var imd = id.GetMethodDefinition (ifaceMap.InterfaceMethods [i]);

if (CheckMethod (imd, ref name, ref methodName, ref signature))
return true;
}
}
}

return false;
Expand Down

0 comments on commit 25850ba

Please sign in to comment.