Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable IsAotCompatible=true, to identify trimmer warnings in Java.Interop.dll #1157

Closed
jonathanpeppers opened this issue Nov 7, 2023 · 10 comments · Fixed by #1190
Closed
Labels
enhancement Proposed change to current functionality java-interop Runtime bridge between .NET and Java
Milestone

Comments

@jonathanpeppers
Copy link
Member

As one of the prerequisites for supporting NativeAOT, we should solve all the trim warnings in this repo.

For details, see:

As an initial step, we could try enabling IsAotCompatible in this repo, this implicitly sets:

  • IsTrimmable
  • EnableTrimAnalyzer
  • EnableAotAnalyzer
  • EnableSingleFileAnalyzer

Then we'd want to address all the warnings and potentially make trimmer warnings build errors? FYI @jonpryor

@jonathanpeppers jonathanpeppers added this to the 9.0.0 Planning milestone Nov 7, 2023
@jonathanpeppers
Copy link
Member Author

Related: dotnet/maui#5041

@jonathanpeppers
Copy link
Member Author

Ok, one other comment, is you can further test trimming by doing something like:

  • dotnet new console
  • Add project reference to Java.Interop.dll
  • Make Java.Interop.dll a @(TrimmerRootAssembly)
  • Publish and verify there are 0 trimmer warnings

This is more comprehensive than just setting IsAotCompatible=true.

@jpobst jpobst added enhancement Proposed change to current functionality java-interop Runtime bridge between .NET and Java labels Nov 7, 2023
@jpobst
Copy link
Contributor

jpobst commented Nov 7, 2023

Results of running <IsAotCompatible> on Java.Interop today:

ManagedPeer.cs(93,19,93,112): warning IL2057: Unrecognized value passed to the parameter 'typeName' of method 'System.Type.GetType(String, Boolean)'. It's not possible to guarantee the availability of the target type.
ManagedPeer.cs(156,18,156,65): warning IL2057: Unrecognized value passed to the parameter 'typeName' of method 'System.Type.GetType(String, Boolean)'. It's not possible to guarantee the availability of the target type.
ManagedPeer.cs(198,35,198,92): warning IL2057: Unrecognized value passed to the parameter 'typeName' of method 'System.Type.GetType(String, Boolean)'. It's not possible to guarantee the availability of the target type.
JniValueMarshaler.cs(169,66,169,107): warning IL2026: Using member 'System.Linq.Expressions.Expression.Property(Expression, String)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Creating Expressions requires unreferenced code because the members being referenced by the Expression may be trimmed.
JniValueMarshaler.cs(200,7,202,17): warning IL2026: Using member 'System.Linq.Expressions.Expression.Property(Expression, String)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Creating Expressions requires unreferenced code because the members being referenced by the Expression may be trimmed.
JniValueMarshaler.cs(201,8,201,53): warning IL2026: Using member 'System.Linq.Expressions.Expression.Property(Expression, String)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Creating Expressions requires unreferenced code because the members being referenced by the Expression may be trimmed.
JniRuntime.JniMarshalMemberBuilder.cs(57,40,57,68): warning IL2072: 'type' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicParameterlessConstructor' in call to 'System.Activator.CreateInstance(Type)'. The return value of method 'System.Reflection.Assembly.GetType(String)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.
JniRuntime.cs(249,29,249,81): warning IL3050: Using member 'System.Runtime.InteropServices.Marshal.PtrToStructure(nint, Type)' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. Marshalling code for the object might not be available.
JniRuntime.JniValueManager.cs(672,58,672,91): warning IL2026: Using member 'System.Linq.Expressions.Expression.Property(Expression, String)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Creating Expressions requires unreferenced code because the members being referenced by the Expression may be trimmed.
JniStringValueMarshaler.cs(44,60,44,95): warning IL2026: Using member 'System.Linq.Expressions.Expression.Property(Expression, String)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Creating Expressions requires unreferenced code because the members being referenced by the Expression may be trimmed.
JniRuntime.JniValueManager.cs(685,41,685,136): warning IL2026: Using member 'System.Linq.Expressions.Expression.Property(Expression, String)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Creating Expressions requires unreferenced code because the members being referenced by the Expression may be trimmed.
JniRuntime.JniValueManager.cs(703,7,707,20): warning IL2026: Using member 'System.Linq.Expressions.Expression.Call(Expression, String, Type[], params Expression[])' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Creating Expressions requires unreferenced code because the members being referenced by the Expression may be trimmed.
JniRuntime.JniValueManager.cs(704,32,704,85): warning IL2026: Using member 'System.Linq.Expressions.Expression.Property(Expression, String)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Creating Expressions requires unreferenced code because the members being referenced by the Expression may be trimmed.
JniRuntime.JniTypeManager.cs(300,22,300,79): warning IL3050: Using member 'System.Type.MakeGenericType(params Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime.
JniRuntime.JniTypeManager.cs(304,22,304,76): warning IL3050: Using member 'System.Type.MakeGenericType(params Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime.
JniRuntime.JniTypeManager.cs(313,22,313,48): warning IL3050: Using member 'System.Type.MakeArrayType()' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The code for an array of the specified type might not be available.
JniRuntime.JniMarshalMemberBuilder.cs(164,33,164,78): warning IL2072: 'type' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicParameterlessConstructor' in call to 'System.Activator.CreateInstance(Type)'. The return value of method 'Java.Interop.JniValueMarshalerAttribute.MarshalerType.get' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.
JniRuntime.JniValueManager.cs(363,12,363,56): warning IL3050: Using member 'System.Type.MakeGenericType(params Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime.
JniRuntime.JniValueManager.cs(354,13,354,50): warning IL2026: Using member 'System.Reflection.Assembly.GetType(String)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Types might be removed.
JniRuntime.JniValueManager.cs(359,30,360,91): warning IL2026: Using member 'System.Reflection.Assembly.GetType(String)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Types might be removed.
JniRuntime.JniValueManager.cs(363,12,363,56): warning IL2055: Call to 'System.Type.MakeGenericType(params Type[])' can not be statically analyzed. It's not possible to guarantee the availability of requirements of the generic type.
JniRuntime.JniValueManager.cs(341,23,341,114): warning IL2070: 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicConstructors', 'DynamicallyAccessedMemberTypes.NonPublicConstructors' in call to 'System.Type.GetConstructors(BindingFlags)'. The parameter 'type' of method 'Java.Interop.JniRuntime.JniValueManager.GetActivationConstructor(Type)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.
JniRuntime.JniTypeManager.cs(414,28,414,96): warning IL2070: 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.NonPublicNestedTypes' in call to 'System.Type.GetNestedType(String, BindingFlags)'. The parameter 'type' of method 'Java.Interop.JniRuntime.JniTypeManager.TryLoadJniMarshalMethods(JniType, Type, String)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.
JniRuntime.JniTypeManager.cs(418,26,418,108): warning IL2072: 'type' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicMethods' in call to 'System.Reflection.RuntimeReflectionExtensions.GetRuntimeMethod(Type, String, Type[])'. The return value of method 'System.Type.GetNestedType(String, BindingFlags)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.
JniRuntime.JniTypeManager.cs(464,32,464,64): warning IL2067: 'type' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicMethods', 'DynamicallyAccessedMemberTypes.NonPublicMethods' in call to 'System.Reflection.RuntimeReflectionExtensions.GetRuntimeMethods(Type)'. The parameter 'marshalType' of method 'Java.Interop.JniRuntime.JniTypeManager.FindAndCallRegisterMethod(Type, JniNativeMethodRegistrationArguments)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.
JniRuntime.JniValueManager.cs(596,29,597,38): warning IL3050: Using member 'System.Reflection.MethodInfo.MakeGenericMethod(params Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime.
JniRuntime.JniValueManager.cs(596,29,597,38): warning IL2060: Call to 'System.Reflection.MethodInfo.MakeGenericMethod(params Type[])' can not be statically analyzed. It's not possible to guarantee the availability of requirements of the generic method.
JniRuntime.JniValueManager.cs(542,33,542,87): warning IL2072: 'type' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicParameterlessConstructor' in call to 'System.Activator.CreateInstance(Type)'. The return value of method 'Java.Interop.JniValueMarshalerAttribute.MarshalerType.get' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.
JniRuntime.JniValueManager.cs(557,21,557,42): warning IL2070: 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.Interfaces' in call to 'System.Type.GetInterfaces()'. The parameter 'type' of method 'Java.Interop.JniRuntime.JniValueManager.GetValueMarshaler(Type)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.
JniRuntime.JniValueManager.cs(578,27,578,48): warning IL2070: 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.Interfaces' in call to 'System.Type.GetInterfaces()'. The parameter 'type' of method 'Java.Interop.JniRuntime.JniValueManager.GetValueMarshaler(Type)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.
JniRuntime.JniValueManager.cs(588,33,588,88): warning IL2072: 'type' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicParameterlessConstructor' in call to 'System.Activator.CreateInstance(Type)'. The return value of method 'Java.Interop.JniValueMarshalerAttribute.MarshalerType.get' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.

@jonpryor
Copy link
Member

jonpryor commented Nov 8, 2023

Ran into many of these as part of PR #1153, which brings me to:

The ManagedPeer.cs warnings are because it uses Type.GetType() for a string value provided by Java, specifically from the Java Callable Wrappers. PR #1153 "makes this work" by adding a RegisterNativeMembers() overload which takes a ReadOnlySpan<char> instead of a Type -- overridden in NativeAotTypeManager -- and while allows registration to proceed, it doesn't fix the ManagedPeer.Construct() side of things, which means. you can't (currently) have Java Callable Wrapper constructors which take arguments.

I do wonder if the 'glib one-off' comment is actually the most viable solution:

Possible "quick hack": replace Type.GetType() use with calls to something on JniRuntime.JniTypeManager, allowing a subclass to provide its own mapping?

The Expressions code is never used in .NET Android, and in a NativeAOT context if (when?) used will require the use of jnimarshalmethod-gen in order to AOT the Expressions, so this is mostly a manner of adding the appropriate custom attributes to silence the linker.

Marshal.PtrToStructure() & related code should probably just migrate to using C#9 function pointers et al.

Assembly.GetType() is in the "required" camp -- not for .NET Android per-se, but just in general -- as it and equivalent code in xamarin-android is used to look up *Invoker types for interface wrapping.

@jonpryor
Copy link
Member

jonpryor commented Nov 8, 2023

Part of me wonders if "fuller NativeAOT support" will require that jcw-gen/etc. not only emit .java code, but also emit C# code or IL for use by NativeAOT compilation. For example, given:

class Example : Java.Lang.Object {
    [Export(SuperArgumentString="")]
    public Example(int a, String b) {}
}

the Java Callable Wrapper will currently use strings to allow looking up the constructor:

ManagedPeer.construct (
    /* self */      this,
    /* aqn */       "Namespace.Example, Assembly",
    /* ctor sig */  "System.Int32, System.Runtime:System.String, System.Runtime",
    /* arguments */ new java.lang.Object[] { p0, p1 }
);

As we can't use Type.GetType() on the "ctor sig" argument value components within NativeAOT, maybe the "workaround" is to have jcw-gen emit "something" in C#/IL which would allow mapping the "ctor sig" string to new[]{typeof(int), typeof(string)}, which would allow Type.GetConstructor() to work? Or we rethink how constructors work by having them invoke a native method, plus associated "magic" to make that work? (I can see that working with jnimarshalmethod-gen, as it's updating the assembly anyway, so it could certainly just add more methods to register, but I'm less certain how to make this work in a .NET Android context where marshal methods come from generator output…)

@jonathanpeppers
Copy link
Member Author

The Expressions code is never used in .NET Android, and in a NativeAOT context

So, some of these warnings can go away by adding additional attributes that pass warning along to the caller. If xamarin-android never calls some of these, we're good.

That might get us by to at least address the warnings here and be able to move on to xamarin-android.

@jonpryor
Copy link
Member

@jonathanpeppers: are there any HOWTOs on how to "fix" or otherwise silence these warnings? Use [SuppressMessage]?

I guess maybe [UnconditionalSuppressMessage]? https://github.com/dotnet/dotNext/blob/4c7d7f4ae3bbdcc5ef63ecfdb8b937d2032d1eea/src/DotNext/Text/Json/OptionalConverter.cs#L14

@jonpryor
Copy link
Member

Issue #1165 has some thoughts on how to remove Type.GetType() from ManagedPeer.cs. I'll need to think about the other issues separately.

@jonathanpeppers
Copy link
Member Author

There are some reasonable docs in:

[UnconditionalSuppressMessage] should be the last resort, so if the problem is related to [RequiresUnreferencedCode] sometimes you just decorate the method to forward the warning to any callers (until you get to a caller you can't).

jonpryor added a commit that referenced this issue Nov 24, 2023
Fixes: #1165
Context: #1157

If we more strongly rely on JNI signatures, we can remove the need
for Java Callable Wrappers to contain assembly-qualified type names,
thus removing the need for `ManagedPeer` to use `Type.GetType()`
entirely, removing the [IL2057][0] warnings.

Furthermore, if we add `[DynamicallyAccessedMembers]` to
`JniRuntime.JniTypeManager.GetType()`, we can fix some [IL2075][1]
warnings which appeared after fixing the IL2057 warnings.

Aside: Excising assembly-qualified type names from Java Callable
Wrappers had some "interesting" knock-on effects in the unit tests,
requiring that more typemap information be explicitly provided.
(This same information was *implicitly* provided before, via the
provision of assembly-qualified type names everywhere…)

[0]: https://learn.microsoft.com/en-us/dotnet/core/deploying/trimming/trim-warnings/IL2057
[1]: https://learn.microsoft.com/en-us/dotnet/core/deploying/trimming/trim-warnings/il2075
jonpryor added a commit that referenced this issue Nov 25, 2023
Fixes: #1165

Context: #1153
Context: #1157

When building for NativeAOT (#1153) or when building .NET Android
apps with `-p:IsAotcompatible=true` (#1157), we get [IL2057][0]
warnings from `ManagedPeer.cs`:

        ManagedPeer.cs(93,19,93,112): warning IL2057: Unrecognized value passed to the parameter 'typeName' of method 'System.Type.GetType(String, Boolean)'. It's not possible to guarantee the availability of the target type.
        ManagedPeer.cs(156,18,156,65): warning IL2057: Unrecognized value passed to the parameter 'typeName' of method 'System.Type.GetType(String, Boolean)'. It's not possible to guarantee the availability of the target type.
        ManagedPeer.cs(198,35,198,92): warning IL2057: Unrecognized value passed to the parameter 'typeName' of method 'System.Type.GetType(String, Boolean)'. It's not possible to guarantee the availability of the target type.

These warnings are because `ManagedPeer.Construct()` and
`ManagedPeer.RegisterNativeMembers()` use `Type.GetType()` on string
values provided *from Java code*, and thus the IL trimmer does not
have visibility into those strings, and thus cannot reliably
determine which types need to be preserved:

	// Java Callable Wrapper
	/* partial */ class ManagedType
	{
	  public static final String __md_methods;
	  static {
	    __md_methods =
	      "n_GetString:()Ljava/lang/String;:__export__\n" +
	      "";
	    net.dot.jni.ManagedPeer.registerNativeMembers (
	        /* nativeClass */             ManagedType.class,
	        /* assemblyQualifiedName */   "Example.ManagedType, Hello-NativeAOTFromJNI",
	        /* methods */                 __md_methods);
	  }

	  public ManagedType (int p0)
	  {
	    super ();
	    if (getClass () == ManagedType.class) {
	      net.dot.jni.ManagedPeer.construct (
	          /* self */                  this,
	          /* assemblyQualifiedName */ "Example.ManagedType, Hello-NativeAOTFromJNI",
	          /* constructorSignature */  "System.Int32, System.Runtime",
	          /* arguments */             new java.lang.Object[] { p0 });
	    }
	  }
	}

`ManagedPeer.construct()` passes *two* sets of assembly-qualified
type names: `assemblyQualifiedName` contains the type to construct,
while `constructorSignature` contains a `:`-separated list of
assembly-qualified type names for the constructor parameters.
Each of these are passed to `Type.GetType()`.

`ManagedPeer.registerNativeMembers()` passes an assembly-qualified
type name to `ManagedPeer.RegisterNativeMembers()`, which passes the
assembly-qualified type name to `Type.GetType()` to find the type
to register native methods for.

If we more strongly rely on JNI signatures, we can remove the need
for Java Callable Wrappers to contain assembly-qualified type names
entirely, thus removing the need for `ManagedPeer` to use
`Type.GetType()`, removing the IL2057 warnings.

For `ManagedPeer.construct()`, `assemblyQualifiedName` can be
replaced with getting the JNI type signature from `self.getClass()`,
and `constructorSignature` can be replaced with a
*JNI method signature* of the calling constructor.

For `ManagedPeer.registerNativeMembers()`, `assemblyQualifiedName`
can be replaced with getting the JNI type signature from `nativeClass`.

	// Java Callable Wrapper
	/* partial */ class ManagedType
	{
	  public static final String __md_methods;
	  static {
	    __md_methods =
	      "n_GetString:()Ljava/lang/String;:__export__\n" +
	      "";
	    net.dot.jni.ManagedPeer.registerNativeMembers (
	        /* nativeClass */             ManagedType.class,
	        /* methods */                 __md_methods);
	  }

	  public ManagedType (int p0)
	  {
	    super ();
	    if (getClass () == ManagedType.class) {
	      net.dot.jni.ManagedPeer.construct (
	          /* self */                  this,
	          /* constructorSignature */  "(I)V",
	          /* arguments */             new java.lang.Object[] { p0 });
	    }
	  }
	}

Furthermore, if we add `[DynamicallyAccessedMembers]` to
`JniRuntime.JniTypeManager.GetType()`, we can fix some [IL2075][1]
warnings which appeared after fixing the IL2057 warnings.

Aside: Excising assembly-qualified type names from Java Callable
Wrappers had some "interesting" knock-on effects in the unit tests,
requiring that more typemap information be explicitly provided.
(This same information was *implicitly* provided before, via the
provision of assembly-qualified type names everywhere…)

[0]: https://learn.microsoft.com/en-us/dotnet/core/deploying/trimming/trim-warnings/IL2057
[1]: https://learn.microsoft.com/en-us/dotnet/core/deploying/trimming/trim-warnings/il2075
jonpryor added a commit that referenced this issue Nov 25, 2023
Fixes: #1165

Context: #1153
Context: #1157

When building for NativeAOT (#1153) or when building .NET Android
apps with `-p:IsAotcompatible=true` (#1157), we get [IL2057][0]
warnings from `ManagedPeer.cs`:

        ManagedPeer.cs(93,19,93,112): warning IL2057: Unrecognized value passed to the parameter 'typeName' of method 'System.Type.GetType(String, Boolean)'. It's not possible to guarantee the availability of the target type.
        ManagedPeer.cs(156,18,156,65): warning IL2057: Unrecognized value passed to the parameter 'typeName' of method 'System.Type.GetType(String, Boolean)'. It's not possible to guarantee the availability of the target type.
        ManagedPeer.cs(198,35,198,92): warning IL2057: Unrecognized value passed to the parameter 'typeName' of method 'System.Type.GetType(String, Boolean)'. It's not possible to guarantee the availability of the target type.

These warnings are because `ManagedPeer.Construct()` and
`ManagedPeer.RegisterNativeMembers()` use `Type.GetType()` on string
values provided *from Java code*, and thus the IL trimmer does not
have visibility into those strings, and thus cannot reliably
determine which types need to be preserved:

	// Java Callable Wrapper
	/* partial */ class ManagedType
	{
	  public static final String __md_methods;
	  static {
	    __md_methods =
	      "n_GetString:()Ljava/lang/String;:__export__\n" +
	      "";
	    net.dot.jni.ManagedPeer.registerNativeMembers (
	        /* nativeClass */             ManagedType.class,
	        /* assemblyQualifiedName */   "Example.ManagedType, Hello-NativeAOTFromJNI",
	        /* methods */                 __md_methods);
	  }

	  public ManagedType (int p0)
	  {
	    super ();
	    if (getClass () == ManagedType.class) {
	      net.dot.jni.ManagedPeer.construct (
	          /* self */                  this,
	          /* assemblyQualifiedName */ "Example.ManagedType, Hello-NativeAOTFromJNI",
	          /* constructorSignature */  "System.Int32, System.Runtime",
	          /* arguments */             new java.lang.Object[] { p0 });
	    }
	  }
	}

`ManagedPeer.construct()` passes *two* sets of assembly-qualified
type names: `assemblyQualifiedName` contains the type to construct,
while `constructorSignature` contains a `:`-separated list of
assembly-qualified type names for the constructor parameters.
Each of these are passed to `Type.GetType()`.

`ManagedPeer.registerNativeMembers()` passes an assembly-qualified
type name to `ManagedPeer.RegisterNativeMembers()`, which passes the
assembly-qualified type name to `Type.GetType()` to find the type
to register native methods for.

If we more strongly rely on JNI signatures, we can remove the need
for Java Callable Wrappers to contain assembly-qualified type names
entirely, thus removing the need for `ManagedPeer` to use
`Type.GetType()`, removing the IL2057 warnings.

For `ManagedPeer.construct()`, `assemblyQualifiedName` can be
replaced with getting the JNI type signature from `self.getClass()`,
and `constructorSignature` can be replaced with a
*JNI method signature* of the calling constructor.

For `ManagedPeer.registerNativeMembers()`, `assemblyQualifiedName`
can be replaced with getting the JNI type signature from `nativeClass`.

	// Java Callable Wrapper
	/* partial */ class ManagedType
	{
	  public static final String __md_methods;
	  static {
	    __md_methods =
	      "n_GetString:()Ljava/lang/String;:__export__\n" +
	      "";
	    net.dot.jni.ManagedPeer.registerNativeMembers (
	        /* nativeClass */             ManagedType.class,
	        /* methods */                 __md_methods);
	  }

	  public ManagedType (int p0)
	  {
	    super ();
	    if (getClass () == ManagedType.class) {
	      net.dot.jni.ManagedPeer.construct (
	          /* self */                  this,
	          /* constructorSignature */  "(I)V",
	          /* arguments */             new java.lang.Object[] { p0 });
	    }
	  }
	}

Furthermore, if we add `[DynamicallyAccessedMembers]` to
`JniRuntime.JniTypeManager.GetType()`, we can fix some [IL2075][1]
warnings which appeared after fixing the IL2057 warnings.

Aside: Excising assembly-qualified type names from Java Callable
Wrappers had some "interesting" knock-on effects in the unit tests,
requiring that more typemap information be explicitly provided.
(This same information was *implicitly* provided before, via the
provision of assembly-qualified type names everywhere…)

[0]: https://learn.microsoft.com/en-us/dotnet/core/deploying/trimming/trim-warnings/IL2057
[1]: https://learn.microsoft.com/en-us/dotnet/core/deploying/trimming/trim-warnings/il2075
@jonpryor
Copy link
Member

An idea occurs to me around the IL2026 warnings around System.Linq.Expressions.Expression.Property(): I should look into moving that code into Java.Interop.Export.dll, and see what breaks (and if it can be fixed without breaking API).

With Research Project jnimarshalmethod-gen™, the NativeAOT sample should (lol) be able to use expression tree output in the post-build step, removing the need for further expression tree usage, such that the final output doesn't need Java.Interop.Export at all. (I hope.)

(Whether that would actually impact the linker warnings in a NativeAOT app/#1168 is a different question entirely; what target produces them? If it's after build and after jnimarshalmethod-gen runs, then this is a plausible way to "totally" fix the warnings, and even in a "current .NET Android world order" simply moving that code into Java.Interop.Export will also remove those warnings…)

jonpryor added a commit that referenced this issue Dec 2, 2023
Fixes: #1165

Context: #1153
Context: #1157
Context: f60906c

When building for NativeAOT (#1153) or when building .NET Android
apps with `-p:IsAotcompatible=true` (#1157), we get [IL2057][0]
warnings from `ManagedPeer.cs`:

	ManagedPeer.cs(93,19,93,112): warning IL2057: Unrecognized value passed to the parameter 'typeName' of method 'System.Type.GetType(String, Boolean)'. It's not possible to guarantee the availability of the target type.
	ManagedPeer.cs(156,18,156,65): warning IL2057: Unrecognized value passed to the parameter 'typeName' of method 'System.Type.GetType(String, Boolean)'. It's not possible to guarantee the availability of the target type.
	ManagedPeer.cs(198,35,198,92): warning IL2057: Unrecognized value passed to the parameter 'typeName' of method 'System.Type.GetType(String, Boolean)'. It's not possible to guarantee the availability of the target type.

These warnings are because `ManagedPeer.Construct()` and
`ManagedPeer.RegisterNativeMembers()` use `Type.GetType()` on string
values provided *from Java code*, and thus the IL trimmer does not
have visibility into those strings, and thus cannot reliably
determine which types need to be preserved:

	// Java Callable Wrapper
	/* partial */ class ManagedType
	{
	  public static final String __md_methods;
	  static {
	    __md_methods =
	      "n_GetString:()Ljava/lang/String;:__export__\n" +
	      "";
	    net.dot.jni.ManagedPeer.registerNativeMembers (
	        /* nativeClass */             ManagedType.class,
	        /* assemblyQualifiedName */   "Example.ManagedType, Hello-NativeAOTFromJNI",
	        /* methods */                 __md_methods);
	  }

	  public ManagedType (int p0)
	  {
	    super ();
	    if (getClass () == ManagedType.class) {
	      net.dot.jni.ManagedPeer.construct (
	          /* self */                  this,
	          /* assemblyQualifiedName */ "Example.ManagedType, Hello-NativeAOTFromJNI",
	          /* constructorSignature */  "System.Int32, System.Runtime",
	          /* arguments */             new java.lang.Object[] { p0 });
	    }
	  }
	}

`ManagedPeer.construct()` passes *two* sets of assembly-qualified
type names: `assemblyQualifiedName` contains the type to construct,
while `constructorSignature` contains a `:`-separated list of
assembly-qualified type names for the constructor parameters.
Each of these are passed to `Type.GetType()`.

`ManagedPeer.registerNativeMembers()` passes an assembly-qualified
type name to `ManagedPeer.RegisterNativeMembers()`, which passes the
assembly-qualified type name to `Type.GetType()` to find the type
to register native methods for.

If we more strongly rely on JNI signatures, we can remove the need
for Java Callable Wrappers to contain assembly-qualified type names
entirely, thus removing the need for `ManagedPeer` to use
`Type.GetType()`, removing the IL2057 warnings.

For `ManagedPeer.construct()`, `assemblyQualifiedName` can be
replaced with getting the JNI type signature from `self.getClass()`,
and `constructorSignature` can be replaced with a
*JNI method signature* of the calling constructor.

For `ManagedPeer.registerNativeMembers()`, `assemblyQualifiedName`
can be replaced with getting the JNI type signature from `nativeClass`.
`jcw-gen --codegen-target=JavaInterop1` output becomes:

	// New JavaInterop1 Java Callable Wrapper
	/* partial */ class ManagedType
	{
	  public static final String __md_methods;
	  static {
	    __md_methods =
	      "n_GetString:()Ljava/lang/String;:__export__\n" +
	      "";
	    net.dot.jni.ManagedPeer.registerNativeMembers (
	        /* nativeClass */             ManagedType.class,
	        /* methods */                 __md_methods);
	  }

	  public ManagedType (int p0)
	  {
	    super ();
	    if (getClass () == ManagedType.class) {
	      net.dot.jni.ManagedPeer.construct (
	          /* self */                  this,
	          /* constructorSignature */  "(I)V",
	          /* arguments */             new java.lang.Object[] { p0 });
	    }
	  }
	}

This does not alter `jcw-gen --codegen-target=XAJavaInterop1` output;
.NET Android will continue to require `Type.GetType()` calls within
xamarin/xamarin-android, e.g.
[`AndroidTypeManager.RegisterNativeMembers()`][2].

Furthermore, if we add `[DynamicallyAccessedMembers]` to
`JniRuntime.JniTypeManager.GetType()`, we can fix some [IL2075][1]
warnings which appeared after fixing the IL2057 warnings.

Aside: Excising assembly-qualified type names from Java Callable
Wrappers had some "interesting" knock-on effects in the unit tests,
requiring that more typemap information be explicitly provided.
(This same information was *implicitly* provided before, via the
provision of assembly-qualified type names everywhere…)

One problem with the approach of using JNI signatures instead of
using assembly-qualified names is *ambiguity*: there can be multiple
managed types which correspond to a given JNI signature.  Consider
the JNI signature `[I`, which is a Java `int[]`.  This is bound as:

  * C# `int[]`
  * `JavaArray<int>`
  * `JavaPrimitiveArray<int>`
  * `JavaInt32Array`

How do we know which to use?  Using assembly-qualified type names
for constructor parameters nicely solved this issue, but if we're not
using them anymore…

Update `JavaCallableExample` to demonstrate this:

	partial class JavaCallableExample {
	    [JavaCallableConstructor(SuperConstructorExpression="")]
	    public JavaCallableExample (int[] a, JavaInt32Array b);
	}

The intention is twofold:

 1. This should result in a Java Callable Wrapper constructor with
    signature `JavaCallableExample(int[] p0, int[] p1)`, and

 2. Java code should be able to invoke this constructor.

Turns out, neither of these worked when `Type.GetType()` is not used
for constructor argument lookup: `JavaCallableWrapperGenerator`
didn't fully support e.g. `[JniTypeSignature("I", ArrayRank=1)]`
(present on `JavaInt32Array`), so it didn't know what to do with
the `JavaInt32Array` parameter.

Once (1) was fixed, (2) would fail because
`JniRuntime.JniTypeManager.GetType(JniTypeSignature.Parse("[I"))`
would return `JavaPrimitiveArray<int>`, which wasn't used in
`JavaCallableExample`, resulting in:

	System.NotSupportedException : Unable to find constructor
	  Java.InteropTests.JavaCallableExample(Java.Interop.JavaPrimitiveArray`1[[System.Int32, System.Private.CoreLib, Version=7.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]], Java.Interop.JavaPrimitiveArray`1[[System.Int32, System.Private.CoreLib, Version=7.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]]).
	  Please provide the missing constructor.
	  ----> Java.Interop.JniLocationException : Exception of type 'Java.Interop.JniLocationException' was thrown.
	  Stack Trace:
	     at Java.Interop.ManagedPeer.GetConstructor(JniTypeManager typeManager, Type type, String signature, Type[]& parameterTypes)
	   at Java.Interop.ManagedPeer.Construct(IntPtr jnienv, IntPtr klass, IntPtr n_self, IntPtr n_constructorSignature, IntPtr n_constructorArguments)
	…
	  --- End of managed Java.Interop.JavaException stack trace ---
	java.lang.Throwable
		at net.dot.jni.ManagedPeer.construct(Native Method)
		at net.dot.jni.test.JavaCallableExample.<init>(JavaCallableExample.java:32)
		at net.dot.jni.test.UseJavaCallableExample.test(UseJavaCallableExample.java:8)

The constructor couldn't be found because
`JniRuntime.JniTypeManager.GetTypes()` was incomplete, which is
a longstanding limitation from f60906c: for `[I`, it would only
return `JavaPrimitiveArray<int>` and `int[]`, in that order.

Fix both of these.
`JniRuntime.JniTypeManager.GetTypes(JniTypeSignature.Parse("[I"))`
will now include:

  * `JavaArray<int>`
  * `JavaPrimitiveArray<int>`
  * `JavaInt32Array`
  * `int[]`

This now allows the `JavaCallableExample` constructor to be invoked
from Java.

Because `ManagedPeer.Construct()` is now doing so much extra work
in order to find the `ConstructorInfo` to invoke, cache the lookups.
(Technically this is a "memory leak," as cache entries are never
removed.)

Finally, update `CecilCompilerExpressionVisitor` to emit `newobj`
in certain `VisitNew()` invocations.  This was needed while trying:

	partial class JavaCallableExample {
	    [JavaCallable ("getA")]
	    public int[] GetA() => this.a;
	}

in order to fix the IL error:

	% $HOME/.dotnet/tools/ilverify bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll \
	    --tokens --system-module System.Private.CoreLib \
	    -r 'bin/TestDebug-net7.0/*.dll' \
	    -r '/usr/local/share/dotnet/shared/Microsoft.NETCore.App/7.0.10/*.dll'
	[IL]: Error [StackUnderflow]: […/bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll : .__<$>_jni_marshal_methods::n_GetA(native int, native int)][offset 0x0000002F] Stack underflow.

Unfortunately, even after the above fix invalid IL was generated during
`jnimarshalmethod-gen` processing, which will be investigated later.

[0]: https://learn.microsoft.com/en-us/dotnet/core/deploying/trimming/trim-warnings/IL2057
[1]: https://learn.microsoft.com/en-us/dotnet/core/deploying/trimming/trim-warnings/il2075
[2]: https://github.com/xamarin/xamarin-android/blob/main/src/Mono.Android/Android.Runtime/AndroidRuntime.cs#L481-L577
jonathanpeppers added a commit to jonathanpeppers/java.interop that referenced this issue Feb 5, 2024
Context: dotnet#1157

On the path to enabling `IsAotCompatible`, we can enable some settings
to get started:

    <IsTrimmable>true</IsTrimmable>
    <EnableSingleFileAnalyzer>true</EnableSingleFileAnalyzer>
    <EnableAotAnalyzer>true</EnableAotAnalyzer>

This opts into the analyzers without declaring the assembly is fully
AOT-compatible.

We can also *remove* where we manually specify:

    [assembly: AssemblyMetadata ("IsTrimmable", "True")]

The `$(IsTrimmable)` property does this for us, as well as enabling
analyzers.

Starting out, I got 33 warnings: this is an attempt to address the
ones that don't require too much thinking.

This results in 22 warnings remaining. I was also able to solve all
instances of `IL2067;IL2070`, so I turned these two into build errors
to ensure they don't come back.

~~ Example Warnings ~~

`System.Linq.Expression` usage:

    src\Java.Interop\Java.Interop\JniRuntime.JniValueManager.cs(672,58):
    warning IL2026: Using member 'System.Linq.Expressions.Expression.Property(Expression, String)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Creating Expressions requires unreferenced code because the members being referenced by the Expression may be trimmed.

I decorated this one with:

    [RequiresUnreferencedCode (ExpressionRequiresUnreferencedCode)]

`Type.GetNestedType()` usage:

    src\Java.Interop\Java.Interop\JniRuntime.JniTypeManager.cs(447,28):
    warning IL2070: 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.NonPublicNestedTypes' in call to 'System.Type.GetNestedType(String, BindingFlags)'. The parameter 'type' of method 'Java.Interop.JniRuntime.JniTypeManager.TryLoadJniMarshalMethods(JniType, Type, String)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.

Added:

    [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.NonPublicNestedTypes)]

`Activator.CreateInstance()` usage:

    src\Java.Interop\Java.Interop\JniRuntime.JniValueManager.cs(542,33): warning IL2072: 'type' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicParameterlessConstructor' in call to 'System.Activator.CreateInstance(Type)'. The return value of method 'Java.Interop.JniValueMarshalerAttribute.MarshalerType.get' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.

Added:

    [return: DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)]

~~ Code that Actually Changed ~~

As I enabled more attributes, these snowball into more and more
warnings! I eventually had to make `GetPeerType()` look like:

    [return: DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)]
    static Type GetPeerType ([DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] Type type)

The analyzer was not able to understand code like:

    static  readonly    KeyValuePair<Type, Type>[]      PeerTypeMappings = new []{
        new KeyValuePair<Type, Type>(typeof (object),           typeof (JavaObject)),
        new KeyValuePair<Type, Type>(typeof (IJavaPeerable),    typeof (JavaObject)),
        new KeyValuePair<Type, Type>(typeof (Exception),        typeof (JavaException)),
    };
    //...
    foreach (var m in PeerTypeMappings) {
        if (m.Key == type)
            return m.Value;
    }

Simply removing the `PeerTypeMappings` array and using `if` statements
solved the warnings. This may be the only real code change if any.
jonathanpeppers added a commit to jonathanpeppers/java.interop that referenced this issue Feb 8, 2024
Context: dotnet#1157

On the path to enabling `IsAotCompatible`, we can enable some settings
to get started:

    <IsTrimmable>true</IsTrimmable>
    <EnableSingleFileAnalyzer>true</EnableSingleFileAnalyzer>
    <EnableAotAnalyzer>true</EnableAotAnalyzer>

This opts into the analyzers without declaring the assembly is fully
AOT-compatible.

We can also *remove* where we manually specify:

    [assembly: AssemblyMetadata ("IsTrimmable", "True")]

The `$(IsTrimmable)` property does this for us, as well as enabling
analyzers.

Starting out, I got 33 warnings: this is an attempt to address the
ones that don't require too much thinking.

This results in 22 warnings remaining. I was also able to solve all
instances of `IL2067;IL2070`, so I turned these two into build errors
to ensure they don't come back.

~~ Example Warnings ~~

`System.Linq.Expression` usage:

    src\Java.Interop\Java.Interop\JniRuntime.JniValueManager.cs(672,58):
    warning IL2026: Using member 'System.Linq.Expressions.Expression.Property(Expression, String)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Creating Expressions requires unreferenced code because the members being referenced by the Expression may be trimmed.

I decorated this one with:

    [RequiresUnreferencedCode (ExpressionRequiresUnreferencedCode)]

`Type.GetNestedType()` usage:

    src\Java.Interop\Java.Interop\JniRuntime.JniTypeManager.cs(447,28):
    warning IL2070: 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.NonPublicNestedTypes' in call to 'System.Type.GetNestedType(String, BindingFlags)'. The parameter 'type' of method 'Java.Interop.JniRuntime.JniTypeManager.TryLoadJniMarshalMethods(JniType, Type, String)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.

Added:

    [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.NonPublicNestedTypes)]

`Activator.CreateInstance()` usage:

    src\Java.Interop\Java.Interop\JniRuntime.JniValueManager.cs(542,33): warning IL2072: 'type' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicParameterlessConstructor' in call to 'System.Activator.CreateInstance(Type)'. The return value of method 'Java.Interop.JniValueMarshalerAttribute.MarshalerType.get' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.

Added:

    [return: DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)]

~~ Code that Actually Changed ~~

As I enabled more attributes, these snowball into more and more
warnings! I eventually had to make `GetPeerType()` look like:

    [return: DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)]
    static Type GetPeerType ([DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] Type type)

The analyzer was not able to understand code like:

    static  readonly    KeyValuePair<Type, Type>[]      PeerTypeMappings = new []{
        new KeyValuePair<Type, Type>(typeof (object),           typeof (JavaObject)),
        new KeyValuePair<Type, Type>(typeof (IJavaPeerable),    typeof (JavaObject)),
        new KeyValuePair<Type, Type>(typeof (Exception),        typeof (JavaException)),
    };
    //...
    foreach (var m in PeerTypeMappings) {
        if (m.Key == type)
            return m.Value;
    }

Simply removing the `PeerTypeMappings` array and using `if` statements
solved the warnings. This may be the only real code change if any.
jonathanpeppers added a commit to jonathanpeppers/java.interop that referenced this issue Feb 8, 2024
Context: dotnet#1157

On the path to enabling `IsAotCompatible`, we can enable some settings
to get started:

    <IsTrimmable>true</IsTrimmable>
    <EnableSingleFileAnalyzer>true</EnableSingleFileAnalyzer>
    <EnableAotAnalyzer>true</EnableAotAnalyzer>

This opts into the analyzers without declaring the assembly is fully
AOT-compatible.

Starting out, I got 33 warnings: this is an attempt to address the
ones that don't require too much thinking. Unfortunately, solving 1
warning likely will create dozens more -- as you have to update all
callers.

This results in 24 warnings remaining. Since `Release` builds have
`$(TreatWarningsAsErrors)`, I will wait to enable the analyzers until
all warnings are addressed.

~~ Example Warnings ~~

`System.Linq.Expression` usage:

    src\Java.Interop\Java.Interop\JniRuntime.JniValueManager.cs(672,58):
    warning IL2026: Using member 'System.Linq.Expressions.Expression.Property(Expression, String)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Creating Expressions requires unreferenced code because the members being referenced by the Expression may be trimmed.

I decorated this one with:

    [RequiresUnreferencedCode (ExpressionRequiresUnreferencedCode)]

`Type.GetNestedType()` usage:

    src\Java.Interop\Java.Interop\JniRuntime.JniTypeManager.cs(447,28):
    warning IL2070: 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.NonPublicNestedTypes' in call to 'System.Type.GetNestedType(String, BindingFlags)'. The parameter 'type' of method 'Java.Interop.JniRuntime.JniTypeManager.TryLoadJniMarshalMethods(JniType, Type, String)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.

Added:

    [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.NonPublicNestedTypes)]

`Activator.CreateInstance()` usage:

    src\Java.Interop\Java.Interop\JniRuntime.JniValueManager.cs(542,33): warning IL2072: 'type' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicParameterlessConstructor' in call to 'System.Activator.CreateInstance(Type)'. The return value of method 'Java.Interop.JniValueMarshalerAttribute.MarshalerType.get' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.

Added:

    [return: DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)]

~~ Code that Actually Changed ~~

As I enabled more attributes, these snowball into more and more
warnings! I eventually had to make `GetPeerType()` look like:

    [return: DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)]
    static Type GetPeerType ([DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] Type type)

The analyzer was not able to understand code like:

    static  readonly    KeyValuePair<Type, Type>[]      PeerTypeMappings = new []{
        new KeyValuePair<Type, Type>(typeof (object),           typeof (JavaObject)),
        new KeyValuePair<Type, Type>(typeof (IJavaPeerable),    typeof (JavaObject)),
        new KeyValuePair<Type, Type>(typeof (Exception),        typeof (JavaException)),
    };
    //...
    foreach (var m in PeerTypeMappings) {
        if (m.Key == type)
            return m.Value;
    }

Simply removing the `PeerTypeMappings` array and using `if` statements
solved the warnings. This may be the only real code change if any.
jonathanpeppers added a commit to jonathanpeppers/java.interop that referenced this issue Feb 8, 2024
Context: dotnet#1157

On the path to enabling `IsAotCompatible`, we can enable some settings
to get started:

    <IsTrimmable>true</IsTrimmable>
    <EnableSingleFileAnalyzer>true</EnableSingleFileAnalyzer>
    <EnableAotAnalyzer>true</EnableAotAnalyzer>

This opts into the analyzers without declaring the assembly is fully
AOT-compatible.

Starting out, I got 33 warnings: this is an attempt to address the
ones that don't require too much thinking. Unfortunately, solving 1
warning likely will create dozens more -- as you have to update all
callers.

This results in 24 warnings remaining. Since `Release` builds have
`$(TreatWarningsAsErrors)`, I will wait to enable the analyzers until
all warnings are addressed.

~~ Example Warnings ~~

`System.Linq.Expression` usage:

    src\Java.Interop\Java.Interop\JniRuntime.JniValueManager.cs(672,58):
    warning IL2026: Using member 'System.Linq.Expressions.Expression.Property(Expression, String)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Creating Expressions requires unreferenced code because the members being referenced by the Expression may be trimmed.

I decorated this one with:

    [RequiresUnreferencedCode (ExpressionRequiresUnreferencedCode)]

`Type.GetNestedType()` usage:

    src\Java.Interop\Java.Interop\JniRuntime.JniTypeManager.cs(447,28):
    warning IL2070: 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.NonPublicNestedTypes' in call to 'System.Type.GetNestedType(String, BindingFlags)'. The parameter 'type' of method 'Java.Interop.JniRuntime.JniTypeManager.TryLoadJniMarshalMethods(JniType, Type, String)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.

Added:

    [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.NonPublicNestedTypes)]

`Activator.CreateInstance()` usage:

    src\Java.Interop\Java.Interop\JniRuntime.JniValueManager.cs(542,33): warning IL2072: 'type' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicParameterlessConstructor' in call to 'System.Activator.CreateInstance(Type)'. The return value of method 'Java.Interop.JniValueMarshalerAttribute.MarshalerType.get' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.

Added:

    [return: DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)]

~~ Code that Actually Changed ~~

As I enabled more attributes, these snowball into more and more
warnings! I eventually had to make `GetPeerType()` look like:

    [return: DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)]
    static Type GetPeerType ([DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] Type type)

The analyzer was not able to understand code like:

    static  readonly    KeyValuePair<Type, Type>[]      PeerTypeMappings = new []{
        new KeyValuePair<Type, Type>(typeof (object),           typeof (JavaObject)),
        new KeyValuePair<Type, Type>(typeof (IJavaPeerable),    typeof (JavaObject)),
        new KeyValuePair<Type, Type>(typeof (Exception),        typeof (JavaException)),
    };
    //...
    foreach (var m in PeerTypeMappings) {
        if (m.Key == type)
            return m.Value;
    }

Simply removing the `PeerTypeMappings` array and using `if` statements
solved the warnings. This may be the only real code change if any.
jonpryor pushed a commit that referenced this issue Feb 13, 2024
Context: #1157

We want to enable `$(IsAotCompatible)`=true, so we can identify and
fix trimmer warnings within `Java.Interop.dll`.  (Then later work our
way "up the stack", fixing trimmer warnings within `Mono.Android.dll`
and `Microsoft.Maui.dll` and…)

On the path to enabling `$(IsAotCompatible)`=true, we can enable some
settings to get started:

	<IsTrimmable>true</IsTrimmable>
	<EnableSingleFileAnalyzer>true</EnableSingleFileAnalyzer>
	<EnableAotAnalyzer>true</EnableAotAnalyzer>

This opts into the analyzers without declaring that the assembly is
fully AOT-compatible.

Starting out, I got 33 warnings: this is an attempt to address the
ones that don't require too much thinking.  Unfortunately, solving
one warning likely will create dozens more -- as you have to update
all callers.

This results in 24 warnings remaining.  Since `Release` builds have
`$(TreatWarningsAsErrors)`, I will wait to enable the analyzers until
all warnings are addressed.

~~ Example Warnings ~~

**`System.Linq.Expression` usage:**
`JniRuntime.JniValueManager.CreateParameterFromManagedExpression()`

	/* 638 */ partial class JavaPeerableValueMarshaler {
	/* 667 */     public override Expression CreateParameterFromManagedExpression (JniValueMarshalerContext context, ParameterExpression sourceValue, ParameterAttributes synchronize)
	/* 668 */     {
	/* 669 */         var r = CreateIntermediaryExpressionFromManagedExpression (context, sourceValue);
	/* 670 */         var h = Expression.Variable (typeof (IntPtr), sourceValue.Name + "_handle");
	/* 671 */         context.LocalVariables.Add (h);
	/* 672 */         context.CreationStatements.Add (Expression.Assign (h, Expression.Property (r, "Handle")));
	/* 674 */         return h;
	/* 675 */     }
	/* 710 */ }

emits an IL2026:

	src\Java.Interop\Java.Interop\JniRuntime.JniValueManager.cs(672,58):
	warning IL2026: Using member 'System.Linq.Expressions.Expression.Property(Expression, String)'
	which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code.
	Creating Expressions requires unreferenced code because the members being referenced by the Expression may be trimmed.

I updated this with:

	partial class JniValueMarshaler {
	    internal const string ExpressionRequiresUnreferencedCode = "System.Linq.Expression usage may trim away required code.";

	    [RequiresUnreferencedCode (ExpressionRequiresUnreferencedCode)]
	    public virtual Expression CreateParameterFromManagedExpression (JniValueMarshalerContext context, ParameterExpression sourceValue, ParameterAttributes synchronize) => …
	}
	partial class JavaPeerableValueMarshaler /* indirectly inherits JniValueMarshaler */ {
	    [RequiresUnreferencedCode (ExpressionRequiresUnreferencedCode)]
	    public override Expression CreateParameterFromManagedExpression (JniValueMarshalerContext context, ParameterExpression sourceValue, ParameterAttributes synchronize) => …
	}

**`Type.GetNestedType()` usage:**
`JniRuntime.JniTypeManager.TryLoadJniMarshalMethods()`:

	/*  82 */ partial class JniRuntime.JniTypeManager {
	/* 445 */     bool TryLoadJniMarshalMethods (JniType nativeClass, Type type, string? methods)
	/* 446 */     {
	/* 447 */         var marshalType = type?.GetNestedType ("__<$>_jni_marshal_methods", BindingFlags.NonPublic);

emits an IL2070 warning:

	src\Java.Interop\Java.Interop\JniRuntime.JniTypeManager.cs(447,28):
	warning IL2070: 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.NonPublicNestedTypes' in call to
	'System.Type.GetNestedType(String, BindingFlags)'.
	The parameter 'type' of method 'Java.Interop.JniRuntime.JniTypeManager.TryLoadJniMarshalMethods(JniType, Type, String)'
	does not have matching annotations.
	The source value must declare at least the same requirements as those declared on the target location it is assigned to.

I updated this with:

	partial class JniRuntime.JniTypeManager {
	    bool TryLoadJniMarshalMethods (
	            JniType nativeClass,
	            [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.NonPublicNestedTypes)]
	            Type type,
	            string? methods) => …

**`Activator.CreateInstance()` usage:**
`JniRuntime.JniValueManager.GetValueMarshaler()`:

	/*  50 */ partial class JniRuntime.JniValueManager {
	/* 530 */     public JniValueMarshaler GetValueMarshaler (Type type)
	/* 531 */     {
	/* 541 */         if (marshalerAttr != null)
	/* 542 */             return (JniValueMarshaler) Activator.CreateInstance (marshalerAttr.MarshalerType)!;
	/* 591 */     }
	/* 612 */ }

emits an IL2072 warning:

	src\Java.Interop\Java.Interop\JniRuntime.JniValueManager.cs(542,33): warning IL2072:
	'type' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicParameterlessConstructor'
	in call to 'System.Activator.CreateInstance(Type)'.
	The return value of method 'Java.Interop.JniValueMarshalerAttribute.MarshalerType.get' does not have matching annotations.
	The source value must declare at least the same requirements as those declared on the target location it is assigned to.

I updated this with:

	partial class JniRuntime.JniValueManager {
	    public JniValueMarshaler GetValueMarshaler (
	            [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.Interfaces)]
	            Type type)
	        => …
	}
	partial class JniValueMarshalerAttribute {
	    public Type MarshalerType {
	        [return: DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)]
	        get;
	    }
	}


~~ Code that Actually Changed ~~

As I added more attributes, these snowballed into more and more
warnings!  I eventually had to make
`JniRuntime.JniValueManager.GetPeerType()` look like:

	partial class JniRuntime.JniValueManager {
	    internal const DynamicallyAccessedMemberTypes Constructors = DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors;

	    [return: DynamicallyAccessedMembers (Constructors)]
	    static Type GetPeerType ([DynamicallyAccessedMembers Constructors)] Type type) => …
	}

The analyzer was not able to understand code like:

	partial class JniRuntime.JniValueManager {
	    static  readonly    KeyValuePair<Type, Type>[]      PeerTypeMappings = new []{
	        new KeyValuePair<Type, Type>(typeof (object),           typeof (JavaObject)),
	        new KeyValuePair<Type, Type>(typeof (IJavaPeerable),    typeof (JavaObject)),
	        new KeyValuePair<Type, Type>(typeof (Exception),        typeof (JavaException)),
	    };
	    static Type GetPeerType (Type type)
	    {
	        foreach (var m in PeerTypeMappings) {
	            if (m.Key == type)
	                return m.Value;
	        }
	    }
	}

Simply removing the `PeerTypeMappings` array and using `if` statements
solved the warnings.  This may be the only real code change if any.
jonathanpeppers added a commit to jonathanpeppers/java.interop that referenced this issue Feb 13, 2024
Fixes: dotnet#1157

The remaining trimmer warnings are around usage of System.Reflection
such as:

* Create an `Invoker` type from an original type via
  `Type.Assembly.GetType()`

* Create a `Type` based on a `ParameterInfo.ParameterType` and/or
  create instances

* Use `Type.MakeGenericType()`

* Use `Type.MakeArrayType()`

The trimming warnings themselves all indicate that these types "might
be trimmed", in that nothing explicitly preserves them. However, we
have a plethora of custom trimmer steps that work to preserve these
types involved in Java interop, such as:

* https://github.com/xamarin/xamarin-android/blob/main/src/Microsoft.Android.Sdk.ILLink/PreserveApplications.cs
* https://github.com/xamarin/xamarin-android/blob/main/src/Microsoft.Android.Sdk.ILLink/PreserveExportedTypes.cs
* https://github.com/xamarin/xamarin-android/blob/main/src/Microsoft.Android.Sdk.ILLink/PreserveJavaExceptions.cs
* https://github.com/xamarin/xamarin-android/blob/main/src/Microsoft.Android.Sdk.ILLink/PreserveJavaInterfaces.cs
* https://github.com/xamarin/xamarin-android/blob/main/src/Microsoft.Android.Sdk.ILLink/PreserveRegistrations.cs

One day, in a perfect world, we could remove these trimmer steps and
completely rely on the trimmer's warnings & attributing mechanisms.
That day doesn't have to be today -- as our goal is begin surfacing
trimmer warnings to users in their own code and dependencies.

With these warnings ignored, we can fully enable analyzers with:

    <IsTrimmable>true</IsTrimmable>
    <EnableAotAnalyzer>true</EnableAotAnalyzer>

We can then remove:

    [assembly: AssemblyMetadata ("IsTrimmable", "True")]

As the MSBuild property does this for us, in addition to enabling
analyzers.

I don't think we should enable `$(IsAotCompatible)` quite yet, as
`Java.Interop.dll` likely *won't work* yet on NativeAOT, and this
places metadata that says an assembly is supported. We should just use
the `$(EnableAotAnalyzer)` analyzers for now, so we don't introduce
new issues.
jonpryor pushed a commit that referenced this issue Feb 14, 2024
Fixes: #1157

The remaining trimmer warnings are around usage of System.Reflection
such as:

  * Creating an `Invoker` type from an original type via
    `Type.Assembly.GetType()`, within
    `JniRuntime.JniValueManager.GetInvokerType()`.

  * Create a `JniValueMarshaler` corresponding to
    `ParameterInfo.ParameterType` in
    `JniRuntime.JniMarshalMemberBuilder.GetParameterMarshaler()`.

  * Create a value corresponding to `ParameterInfo.ParameterType` in
    `ManagedPeer.GetValues()`.

  * Use `typeof(JavaObjectArray<>).MakeGenericType()` in
    `JniRuntime.JniTypeManager.GetTypes(JniTypeSignature)`.

  * Use `Type.MakeArrayType()` in
    `JniRuntime.JniTypeManager.GetTypes(JniTypeSignature)`.

The trimming warnings themselves all indicate that these types "might
be trimmed", in that nothing explicitly preserves them.  However, we
have a plethora of custom trimmer steps that work to preserve these
types involved in Java interop, such as:

  * https://github.com/xamarin/xamarin-android/blob/71b6fcc92f9e86246e0fe575bc1f44bbf73c3132/src/Microsoft.Android.Sdk.ILLink/PreserveApplications.cs
  * https://github.com/xamarin/xamarin-android/blob/71b6fcc92f9e86246e0fe575bc1f44bbf73c3132/src/Microsoft.Android.Sdk.ILLink/PreserveExportedTypes.cs
  * https://github.com/xamarin/xamarin-android/blob/71b6fcc92f9e86246e0fe575bc1f44bbf73c3132/src/Microsoft.Android.Sdk.ILLink/PreserveJavaExceptions.cs
  * https://github.com/xamarin/xamarin-android/blob/71b6fcc92f9e86246e0fe575bc1f44bbf73c3132/src/Microsoft.Android.Sdk.ILLink/PreserveJavaInterfaces.cs
  * https://github.com/xamarin/xamarin-android/blob/71b6fcc92f9e86246e0fe575bc1f44bbf73c3132/src/Microsoft.Android.Sdk.ILLink/PreserveRegistrations.cs

One day, in a perfect world, we could remove these trimmer steps and
completely rely on the trimmer's warnings & attributing mechanisms.
That day doesn't have to be today -- as our goal is begin surfacing
trimmer warnings to users in their own code and dependencies.

With these warnings ignored, we can fully enable analyzers with:

	<IsTrimmable>true</IsTrimmable>
	<EnableAotAnalyzer>true</EnableAotAnalyzer>

We can then remove:

	[assembly: AssemblyMetadata ("IsTrimmable", "True")]

As the MSBuild property does this for us, in addition to enabling
analyzers.

I don't think we should enable `$(IsAotCompatible)` quite yet, as
`Java.Interop.dll` likely *won't work* yet on NativeAOT, and this
places metadata that says an assembly is supported.  We should just
use the `$(EnableAotAnalyzer)` analyzers for now, so we don't
introduce new issues.
jonpryor added a commit that referenced this issue Feb 22, 2024
Context: 28849ec
Context: bc5bcf4
Context: 25850ba
Context: 56955d9
Context: #1157
Context: c6c487b
Context: dotnet/android@180dd52

Commit 28849ec noted:

> [JNI][0] supports *two* modes of operation:
> 
>  1. Native code creates the JVM, e.g. via [`JNI_CreateJavaVM()`][1]
> 
>  2. The JVM already exists, and when Java code calls
>     [`System.loadLibrary()`][3], the JVM calls the
>     [`JNI_OnLoad()`][2] function on the specified library.
>
> [0]: https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/jniTOC.html
> [1]: https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/invocation.html#creating_the_vm
> [2]: https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/lang/Runtime.html#loadLibrary(java.lang.String)
> [3]: https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/invocation.html#JNJI_OnLoad

Our existing unit tests, e.g. `tests/Java.Interop-Tests`, exercise
the first mode of operation.

The second mode of operation is how .NET Android executes.

Add `samples/Hello-NativeAOTFromJNI`, a new sample which exercises
the second mode of operation:

 1. `Hello-NativeAOTFromJNI` contains C# methods which use
    [`UnmanagedCallersOnlyAttribute`][4] to export `JNI_OnLoad()` and
    various other native symbols for JNI to execute.

 2. `dotnet publish Hello-NativeAOTFromJNI.csproj` will use
    [NativeAOT][5] to produce a native library which can be loaded
    and used by a desktop JVM.

 3. `Hello-NativeAOTFromJNI` references `Java.Base.dll` (bc5bcf4),
     and contains a C# `Java.Lang.Object` subclass, which contains a
    `[JavaCallable]` method.

 4. `jcw-gen` is called during the build process to generate
    Java Callable Wrappers for (3), containing the `[JavaCallable]`
    method.

 5. `jnimarshalmethod-gen` (25850ba) is called during the build
    process to generate marshal methods for (3), as `generator`
    output for JavaInterop1 doesn't contain marshal methods (unlike
    XAJavaInterop1, used by .NET Android), and NativeAOT doesn't
    support System.Reflection.Emit.

 6. The type in (3) is instantiated *from Java*, and Java invokes the
    `[JavaCallable]` method.

 7. `Hello-NativeAOTFromJNI` also contains Java code including a
    `main()` method (to start execution from Java)

Conceptually straightforward!

See `samples/Hello-NativeAOTFromJNI/README.md` for more details.


~~ `$(Standalone)`=true is now the default build config ~~

The `$(Standalone)` build config (c6c487b) is now enabled by
default.  This means that neither `Java.Interop.dll` nor
`Java.Runtime.Environment.dll` *require* a `java-interop` native
library, which (1) was already the case for xamarin-android as of
dotnet/android@180dd520, and (2) simplifies NativeAOT
support immensely.


~~ Avoid Adding Public API ~~

In order to avoid adding new public API to `Java.Interop.dll`, update
`Java.Interop.dll` to have `[InternalsVisibleTo]` for
`Java.Runtime.Environment.dll`.  This allows `JreTypeManager` to use
`JniRuntime.UseMarshalMemberBuilder`, without needing to worry about
`try`/`catch` blocks.

This in turn requires renaming `NativeMethods` within
`Java.Runtime.Environment.dll` to `JreNativeMethods`, as
`Java.Interop.dll` *also* has a `NativeMethods` type.


~~ NativeAOT Integration ~~

Conceptually straightforward, but not without lots of pitfalls.

In order for Java code to call into C# code, there must be a
Java Callable Wrapper, which contains Java `native` methods, and
those Java `native` methods must be *registered with Java* before
they are executed.

Method registration involves lots of delegates, which need to be
stored in the `JniNativeMethodRegistration.Delegate` field, of type
`System.Delegate`.

NativeAOT doesn't like this by default.  In order to convince
NativeAOT to support this, every delegate instance provided to
`JniNativeMethodRegistration.Delegate` must be of a delegate type
which has [`UnmanagedFunctionPointerAttribute`][6].  This coerces
NativeAOT to emit "stubs" for the referenced method, allowing things
to work with fewer changes.

Linux builds require `-Wl,-soname` to be set manually in order for
the resulting `.so` to work properly.


~~ Trimmer Warnings ~~

Fix additional trimmer warnings, a'la #1157.

`Type.MakeGenericType()` needs to ensure constructors are preserved:

	src/Java.Interop/Java.Interop/JniRuntime.JniValueManager.cs(378): Trim analysis warning IL2068:
	  Java.Interop.JniRuntime.JniValueManager.<GetInvokerType>g__MakeGenericType|31_1(Type,Type[]):
	  'Java.Interop.JniRuntime.JniValueManager.<GetInvokerType>g__MakeGenericType|31_1(Type,Type[])' method return value does not satisfy
	  'DynamicallyAccessedMemberTypes.PublicConstructors', 'DynamicallyAccessedMemberTypes.NonPublicConstructors' requirements.
	  The parameter 'type' of method 'Java.Interop.JniRuntime.JniValueManager.<GetInvokerType>g__MakeGenericType|31_1(Type,Type[])'
	  does not have matching annotations.
	  The source value must declare at least the same requirements as those declared on the target location it is assigned to.

`Expression.New()` requires the constructor be kept, so "fake it" by
wrapping `Type.GetType()` w/ `[DynamicallyAccessedMembers]`:

	src/Java.Interop/Java.Interop/JniValueMarshaler.cs(175): Trim analysis warning IL2072:
	  Java.Interop.JniValueMarshaler.CreateSelf(JniValueMarshalerContext,ParameterExpression):
	  'type' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicParameterlessConstructor' in call to 'System.Linq.Expressions.Expression.New(Type)'.
	  The return value of method 'System.Object.GetType()' does not have matching annotations.
	  The source value must declare at least the same requirements as those declared on the target location it is assigned to.

`[DynamicallyAccessedMembers]` should be on properties, not getters:

	src/Java.Interop/Java.Interop/JniValueMarshalerAttribute.cs(33): Trim analysis warning IL2078:
	  Java.Interop.JniValueMarshalerAttribute.MarshalerType.get:
	  'Java.Interop.JniValueMarshalerAttribute.MarshalerType.get' method return value does not satisfy
	  'DynamicallyAccessedMemberTypes.PublicParameterlessConstructor', 'DynamicallyAccessedMemberTypes.Interfaces' requirements.
	  The field 'Java.Interop.JniValueMarshalerAttribute.<MarshalerType>k__BackingField' does not have matching annotations.
	  The source value must declare at least the same requirements as those declared on the target location it is assigned to.

Silence use of `Assembly.Location`, when it's optional:

	src/Java.Runtime.Environment/Java.Interop/JreRuntime.cs(106): warning IL3000:
	  Java.Interop.JreRuntime.CreateJreVM(JreRuntimeOptions):
	  'System.Reflection.Assembly.Location.get' always returns an empty string for assemblies embedded in a single-file app.
	  If the path to the app directory is needed, consider calling 'System.AppContext.BaseDirectory'.
	src/Java.Runtime.Environment/Java.Interop/JreRuntime.cs(323): warning IL3000: Java.Interop.JreNativeMethods..cctor():
	  'System.Reflection.Assembly.Location.get' always returns an empty string for assemblies embedded in a single-file app.
	  If the path to the app directory is needed, consider calling 'System.AppContext.BaseDirectory'.


~~ Future Work ~~

Make `JniRuntime.UseMarshalMemberBuilder` public for .NET 9, and
remove the `InternalsVisibleTo` from `Java.Interop.dll` to
`Java.Runtime.Environment.dll`.

`generator` should be updated to apply
`UnmanagedFunctionPointerAttribute` onto all `_JniMarshal_*` delegate
type declarations (56955d9).


[4]: https://learn.microsoft.com/dotnet/api/system.runtime.interopservices.unmanagedcallersonlyattribute?view=net-8.0
[5]: https://learn.microsoft.com/dotnet/core/deploying/native-aot
[6]: https://learn.microsoft.com/dotnet/api/system.runtime.interopservices.unmanagedfunctionpointerattribute?view=net-8.0
@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Proposed change to current functionality java-interop Runtime bridge between .NET and Java
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants