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.

Sign up for GitHub

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

Jump to bottom

JNI Marshal Method Generation, Registration #14

Open
jonpryor opened this issue Apr 7, 2015 · 0 comments
Open

JNI Marshal Method Generation, Registration #14

jonpryor opened this issue Apr 7, 2015 · 0 comments
Labels
callable-wrappers Issues with Java Callable Wrappers enhancement Proposed change to current functionality

Comments

@jonpryor
Copy link
Member

jonpryor commented Apr 7, 2015

"Big picture" method registration picture

  • Remove marshal methods from binding assemblies
  • Support three method registration approaches:
    • Fully dynamic, via System.Linq.Expresssions/Java.Interop.Export
    • Using the above jnimarshalmethod-gen.exe-generated marshaling assembly; see above.
      • This would be for non-AOT "Release" registration
    • Using generated C code.
      • Possibly most performant; for each method to register, we'd emit a Java_... method as per Java/native convention.
      • Allows using the default Java linker to resolve method symbols!
      • Generated C code would use mono_method_invoke()/etc. to invoke the jnimarshalmethod-gen.exe-generated marshal methods.
@jpobst jpobst added callable-wrappers Issues with Java Callable Wrappers enhancement Proposed change to current functionality labels Apr 16, 2020
jonpryor added a commit to jonpryor/java.interop that referenced this issue May 4, 2022
@jonpryor
[Java.Base-Tests] Test Java-to-Managed invocations for Java.Base
2947810 
Commit bc5bcf4 (Desktop `Java.Base` binding) had a TODO:

> ~~ TODO: Marshal Methods ~~
>
> Marshal methods are currently skipped.  Java-to-managed invocations
> are not currently supported.

No marshal methods means no facility for Java-to-managed invocations.

Add a new `tests/Java.Base-Tests` unit test assembly, and test
Java-to-managed invocations.

This requires touching and updating ~everything. 😅

Update `Java.Interop.dll` to contain a new
`Java.Interop.JniMethodSignatureAttribute` custom attribute.

Update `generator` to begin emitting `[JniMethodSignature]` on bound
methods.  This is necessary so that `jcw-gen` knows the JNI method
signature of Java methods to emit.  As part of this, *remove* the
`JniTypeSignatureAttr` type added in bc5bcf4; trying to follow the
`JniTypeSignatureAttr` pattern for `JniMethodSignatureAttr` would
make for more intrusive code changes.  Instead, "re-use" the existing
`RegisterAttr` type, adding a `RegisterAttr.MemberType` property so
that we can select between Android `[Register]` output vs. "Desktop"
`[JniTypeSignature]` and `[JniMethodSignature]` output.  This in turn
requires updating many `generator-Tests` artifacts.

Of particular note is that this change updates `XAJavaInterop1`
output to have `RegisterAttribute.DoNotGenerateAcw=true` on bound
interfaces.  This "broke" unit tests which didn't expect it, but this
shouldn't break anything, and would be otherwise useful if we ever
begin emitting Java Peers for managed interfaces.

Update `Java.Interop.Tools.JavaCallableWrappers` to look for
`Java.Interop.JniTypeSignatureAttribute` and
`Java.Interop.JniMethodSignatureAttribute`.  This allows
`jcw-gen Java.Base-Tests.dll` to generate Java Callable Wrappers for
`Java.BaseTests.MyRunnable`.

Update `Java.Interop.Export.dll` so that `MarshalMemberBuilder`
doesn't use `Expression.GetActionType()` or
`Expression.GetFuncType()`, as .NET doesn't support the use of
generic types in [`Marshal.GetFunctionPointerForDelegate()`][0].
Instead, we need to use `System.Reflection.Emit` to define our own
custom delegate types.

~~ Comparison with Android ~~

Android bindings (both Classic Xamarin.Android and
.NET SDK for Android) use `generator`-emitted "connector methods"
which are specified in the `[Register]` attribute (see also 99897b2):

	namespace Java.Lang {
	    [Register ("java/lang/Object" DoNotGenerateAcw=true)]
	    partial class Object {
	        [Register ("equals", "(Ljava/lang/Object;)Z", "GetEquals_Ljava_lang_Object_Handler")]
	        public virtual unsafe bool Equals (Java.Lang.Object? obj)
	            => …

	        static Delegate GetEquals_Ljava_lang_Object_Handler()
	            => JNINativeWrapper.CreateDelegate((_JniMarshal_PPL_Z) n_Equals_Ljava_lang_Object_);
	        static bool n_Equals_Ljava_lang_Object_ (IntPtr jnienv, IntPtr native__this, IntPtr native_obj)
	            => …
	    }
	}

`jcw-gen` emits the "connector method" into the resulting
Java Callable Wrappers for appropriate subclasses:

	String __md_methods =
	    "n_equals:(Ljava/lang/Object;)Z:GetEquals_Ljava_lang_Object_Handler\n";

At runtime, `AndroidTypeManager.RegisterNativeMembers()` will lookup
`Object.GetEquals_Ljava_lang_Object_Handler()` and invoke it.
The returned `Delegate` instance is provided to
`JNIEnv::RegisterNatives()`.

The problem with this approach is that it's inflexible: there is no
way to participate in the "connector method" infrastructure to alter
marshaling behavior.  If you need custom behavior, e.g.
`Android.Graphics.Color` customizations, you need to update
`generator` and rebuild the binding assemblies.

(This has been "fine" for the past 10+ years, so this hasn't been a
deal breaker.)

For `Java.Base`, @jonpryor wants to support the custom marshaling
infrastructure introduced in 77a6bf8.  This would allow types to
participate in JNI marshal method ("connector method") generation
*at runtime*, allowing specialization based on the current set of
types and assemblies.

This means we *don't* need to specify a "connector method" in
`[JniMethodSignatureAttribute]`, nor generate them.

	namespace Java.Lang {
	    [JniTypeSignatureAttribute("java/lang/Object", GenerateJavaPeer=false)]
	    partial class Object {
	        [JniMethodSignatureAttribute("equals", "(Ljava/lang/Object;)Z")]
	        public virtual unsafe bool Equals (Java.Lang.Object? obj)
	            => …

	        // No GetEquals_Ljava_lang_Object_Handler()
	        // No n_Equals_Ljava_lang_Object_()
	    }
	}

This should result in smaller binding assemblies.

The downside is that runtime costs *increase*, significantly.
Instead of looking up and invoking a "connector method", the method
must be *generated* via System.Linq.Expressions expression trees,
then compiled into IL, then JIT'd, all before it can be used.

We have no timing data to indicate how "bad" this overhead will be.

As always, the hope is that `tools/jnimarshalmethod-gen` (176240d)
can be used to get the "best of both worlds": the flexibility of
custom marshalers, without the runtime overhead.  But…

  * dotnet#14
  * dotnet#616

At this point in time, `jnimarshalmethod-gen` *cannot* work under
.NET 6.  This will need to be addressed in order for custom marshalers
to be a useful solution.

`Java.Base` will use "connector method"-less bindings to act as a
"forcing function" in getting `jnimarshalmethod-gen` working in .NET.

[0]: https://docs.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.marshal.getfunctionpointerfordelegate?view=net-6.0
@jonpryor jonpryor mentioned this issue May 4, 2022
Merged
jonpryor added a commit to jonpryor/java.interop that referenced this issue May 4, 2022
@jonpryor
[Java.Base-Tests] Test Java-to-Managed invocations for Java.Base
bab160e 
Commit bc5bcf4 (Desktop `Java.Base` binding) had a TODO:

> ~~ TODO: Marshal Methods ~~
>
> Marshal methods are currently skipped.  Java-to-managed invocations
> are not currently supported.

No marshal methods means no facility for Java-to-managed invocations.

Add a new `tests/Java.Base-Tests` unit test assembly, and test
Java-to-managed invocations.

This requires touching and updating ~everything. 😅

Update `Java.Interop.dll` to contain a new
`Java.Interop.JniMethodSignatureAttribute` custom attribute.

Update `generator` to begin emitting `[JniMethodSignature]` on bound
methods.  This is necessary so that `jcw-gen` knows the JNI method
signature of Java methods to emit.  As part of this, *remove* the
`JniTypeSignatureAttr` type added in bc5bcf4; trying to follow the
`JniTypeSignatureAttr` pattern for `JniMethodSignatureAttr` would
make for more intrusive code changes.  Instead, "re-use" the existing
`RegisterAttr` type, adding a `RegisterAttr.MemberType` property so
that we can select between Android `[Register]` output vs. "Desktop"
`[JniTypeSignature]` and `[JniMethodSignature]` output.  This in turn
requires updating many `generator-Tests` artifacts.

Of particular note is that this change updates `XAJavaInterop1`
output to have `RegisterAttribute.DoNotGenerateAcw=true` on bound
interfaces.  This "broke" unit tests which didn't expect it, but this
shouldn't break anything, and would be otherwise useful if we ever
begin emitting Java Peers for managed interfaces.

Update `Java.Interop.Tools.JavaCallableWrappers` to look for
`Java.Interop.JniTypeSignatureAttribute` and
`Java.Interop.JniMethodSignatureAttribute`.  This allows
`jcw-gen Java.Base-Tests.dll` to generate Java Callable Wrappers for
`Java.BaseTests.MyRunnable`.

Update `Java.Interop.Export.dll` so that `MarshalMemberBuilder`
doesn't use `Expression.GetActionType()` or
`Expression.GetFuncType()`, as .NET doesn't support the use of
generic types in [`Marshal.GetFunctionPointerForDelegate()`][0].
Instead, we need to use `System.Reflection.Emit` to define our own
custom delegate types.

~~ Comparison with Android ~~

Android bindings (both Classic Xamarin.Android and
.NET SDK for Android) use `generator`-emitted "connector methods"
which are specified in the `[Register]` attribute (see also 99897b2):

	namespace Java.Lang {
	    [Register ("java/lang/Object" DoNotGenerateAcw=true)]
	    partial class Object {
	        [Register ("equals", "(Ljava/lang/Object;)Z", "GetEquals_Ljava_lang_Object_Handler")]
	        public virtual unsafe bool Equals (Java.Lang.Object? obj)
	            => …

	        static Delegate GetEquals_Ljava_lang_Object_Handler()
	            => JNINativeWrapper.CreateDelegate((_JniMarshal_PPL_Z) n_Equals_Ljava_lang_Object_);
	        static bool n_Equals_Ljava_lang_Object_ (IntPtr jnienv, IntPtr native__this, IntPtr native_obj)
	            => …
	    }
	}

`jcw-gen` emits the "connector method" into the resulting
Java Callable Wrappers for appropriate subclasses:

	String __md_methods =
	    "n_equals:(Ljava/lang/Object;)Z:GetEquals_Ljava_lang_Object_Handler\n";

At runtime, `AndroidTypeManager.RegisterNativeMembers()` will lookup
`Object.GetEquals_Ljava_lang_Object_Handler()` and invoke it.
The returned `Delegate` instance is provided to
`JNIEnv::RegisterNatives()`.

The problem with this approach is that it's inflexible: there is no
way to participate in the "connector method" infrastructure to alter
marshaling behavior.  If you need custom behavior, e.g.
`Android.Graphics.Color` customizations, you need to update
`generator` and rebuild the binding assemblies.

(This has been "fine" for the past 10+ years, so this hasn't been a
deal breaker.)

For `Java.Base`, @jonpryor wants to support the custom marshaling
infrastructure introduced in 77a6bf8.  This would allow types to
participate in JNI marshal method ("connector method") generation
*at runtime*, allowing specialization based on the current set of
types and assemblies.

This means we *don't* need to specify a "connector method" in
`[JniMethodSignatureAttribute]`, nor generate them.

	namespace Java.Lang {
	    [JniTypeSignatureAttribute("java/lang/Object", GenerateJavaPeer=false)]
	    partial class Object {
	        [JniMethodSignatureAttribute("equals", "(Ljava/lang/Object;)Z")]
	        public virtual unsafe bool Equals (Java.Lang.Object? obj)
	            => …

	        // No GetEquals_Ljava_lang_Object_Handler()
	        // No n_Equals_Ljava_lang_Object_()
	    }
	}

This should result in smaller binding assemblies.

The downside is that runtime costs *increase*, significantly.
Instead of looking up and invoking a "connector method", the method
must be *generated* via System.Linq.Expressions expression trees,
then compiled into IL, then JIT'd, all before it can be used.

We have no timing data to indicate how "bad" this overhead will be.

As always, the hope is that `tools/jnimarshalmethod-gen` (176240d)
can be used to get the "best of both worlds": the flexibility of
custom marshalers, without the runtime overhead.  But…

  * dotnet#14
  * dotnet#616

At this point in time, `jnimarshalmethod-gen` *cannot* work under
.NET 6.  This will need to be addressed in order for custom marshalers
to be a useful solution.

`Java.Base` will use "connector method"-less bindings to act as a
"forcing function" in getting `jnimarshalmethod-gen` working in .NET.

[0]: https://docs.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.marshal.getfunctionpointerfordelegate?view=net-6.0
jonpryor added a commit to jonpryor/java.interop that referenced this issue May 4, 2022
@jonpryor
[Java.Base-Tests] Test Java-to-Managed invocations for Java.Base
fc0f953 
Commit bc5bcf4 (Desktop `Java.Base` binding) had a TODO:

> ~~ TODO: Marshal Methods ~~
>
> Marshal methods are currently skipped.  Java-to-managed invocations
> are not currently supported.

No marshal methods means no facility for Java-to-managed invocations.

Add a new `tests/Java.Base-Tests` unit test assembly, and test
Java-to-managed invocations.

This requires touching and updating ~everything. 😅

Update `Java.Interop.dll` to contain a new
`Java.Interop.JniMethodSignatureAttribute` custom attribute.

Update `generator` to begin emitting `[JniMethodSignature]` on bound
methods.  This is necessary so that `jcw-gen` knows the JNI method
signature of Java methods to emit.  As part of this, *remove* the
`JniTypeSignatureAttr` type added in bc5bcf4; trying to follow the
`JniTypeSignatureAttr` pattern for `JniMethodSignatureAttr` would
make for more intrusive code changes.  Instead, "re-use" the existing
`RegisterAttr` type, adding a `RegisterAttr.MemberType` property so
that we can select between Android `[Register]` output vs. "Desktop"
`[JniTypeSignature]` and `[JniMethodSignature]` output.  This in turn
requires updating many `generator-Tests` artifacts.

Of particular note is that this change updates `XAJavaInterop1`
output to have `RegisterAttribute.DoNotGenerateAcw=true` on bound
interfaces.  This "broke" unit tests which didn't expect it, but this
shouldn't break anything, and would be otherwise useful if we ever
begin emitting Java Peers for managed interfaces.

Update `Java.Interop.Tools.JavaCallableWrappers` to look for
`Java.Interop.JniTypeSignatureAttribute` and
`Java.Interop.JniMethodSignatureAttribute`.  This allows
`jcw-gen Java.Base-Tests.dll` to generate Java Callable Wrappers for
`Java.BaseTests.MyRunnable`.

Update `Java.Interop.Export.dll` so that `MarshalMemberBuilder`
doesn't use `Expression.GetActionType()` or
`Expression.GetFuncType()`, as .NET doesn't support the use of
generic types in [`Marshal.GetFunctionPointerForDelegate()`][0].
Instead, we need to use `System.Reflection.Emit` to define our own
custom delegate types.

~~ Comparison with Android ~~

Android bindings (both Classic Xamarin.Android and
.NET SDK for Android) use `generator`-emitted "connector methods"
which are specified in the `[Register]` attribute (see also 99897b2):

	namespace Java.Lang {
	    [Register ("java/lang/Object" DoNotGenerateAcw=true)]
	    partial class Object {
	        [Register ("equals", "(Ljava/lang/Object;)Z", "GetEquals_Ljava_lang_Object_Handler")]
	        public virtual unsafe bool Equals (Java.Lang.Object? obj)
	            => …

	        static Delegate GetEquals_Ljava_lang_Object_Handler()
	            => JNINativeWrapper.CreateDelegate((_JniMarshal_PPL_Z) n_Equals_Ljava_lang_Object_);
	        static bool n_Equals_Ljava_lang_Object_ (IntPtr jnienv, IntPtr native__this, IntPtr native_obj)
	            => …
	    }
	}

`jcw-gen` emits the "connector method" into the resulting
Java Callable Wrappers for appropriate subclasses:

	String __md_methods =
	    "n_equals:(Ljava/lang/Object;)Z:GetEquals_Ljava_lang_Object_Handler\n";

At runtime, `AndroidTypeManager.RegisterNativeMembers()` will lookup
`Object.GetEquals_Ljava_lang_Object_Handler()` and invoke it.
The returned `Delegate` instance is provided to
`JNIEnv::RegisterNatives()`.

The problem with this approach is that it's inflexible: there is no
way to participate in the "connector method" infrastructure to alter
marshaling behavior.  If you need custom behavior, e.g.
`Android.Graphics.Color` customizations, you need to update
`generator` and rebuild the binding assemblies.

(This has been "fine" for the past 10+ years, so this hasn't been a
deal breaker.)

For `Java.Base`, @jonpryor wants to support the custom marshaling
infrastructure introduced in 77a6bf8.  This would allow types to
participate in JNI marshal method ("connector method") generation
*at runtime*, allowing specialization based on the current set of
types and assemblies.

This means we *don't* need to specify a "connector method" in
`[JniMethodSignatureAttribute]`, nor generate them.

	namespace Java.Lang {
	    [JniTypeSignatureAttribute("java/lang/Object", GenerateJavaPeer=false)]
	    partial class Object {
	        [JniMethodSignatureAttribute("equals", "(Ljava/lang/Object;)Z")]
	        public virtual unsafe bool Equals (Java.Lang.Object? obj)
	            => …

	        // No GetEquals_Ljava_lang_Object_Handler()
	        // No n_Equals_Ljava_lang_Object_()
	    }
	}

This should result in smaller binding assemblies.

The downside is that runtime costs *increase*, significantly.
Instead of looking up and invoking a "connector method", the method
must be *generated* via System.Linq.Expressions expression trees,
then compiled into IL, then JIT'd, all before it can be used.

We have no timing data to indicate how "bad" this overhead will be.

As always, the hope is that `tools/jnimarshalmethod-gen` (176240d)
can be used to get the "best of both worlds": the flexibility of
custom marshalers, without the runtime overhead.  But…

  * dotnet#14
  * dotnet#616

At this point in time, `jnimarshalmethod-gen` *cannot* work under
.NET 6.  This will need to be addressed in order for custom marshalers
to be a useful solution.

`Java.Base` will use "connector method"-less bindings to act as a
"forcing function" in getting `jnimarshalmethod-gen` working in .NET.

[0]: https://docs.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.marshal.getfunctionpointerfordelegate?view=net-6.0
jonpryor added a commit to jonpryor/java.interop that referenced this issue May 4, 2022
@jonpryor
[Java.Base-Tests] Test Java-to-Managed invocations for Java.Base
0399f91 
Commit bc5bcf4 (Desktop `Java.Base` binding) had a TODO:

> ~~ TODO: Marshal Methods ~~
>
> Marshal methods are currently skipped.  Java-to-managed invocations
> are not currently supported.

No marshal methods means no facility for Java-to-managed invocations.

Add a new `tests/Java.Base-Tests` unit test assembly, and test
Java-to-managed invocations.

This requires touching and updating ~everything. 😅

Update `Java.Interop.dll` to contain a new
`Java.Interop.JniMethodSignatureAttribute` custom attribute.

Update `generator` to begin emitting `[JniMethodSignature]` on bound
methods.  This is necessary so that `jcw-gen` knows the JNI method
signature of Java methods to emit.  As part of this, *remove* the
`JniTypeSignatureAttr` type added in bc5bcf4; trying to follow the
`JniTypeSignatureAttr` pattern for `JniMethodSignatureAttr` would
make for more intrusive code changes.  Instead, "re-use" the existing
`RegisterAttr` type, adding a `RegisterAttr.MemberType` property so
that we can select between Android `[Register]` output vs. "Desktop"
`[JniTypeSignature]` and `[JniMethodSignature]` output.  This in turn
requires updating many `generator-Tests` artifacts.

Update `Java.Interop.Tools.JavaCallableWrappers` to look for
`Java.Interop.JniTypeSignatureAttribute` and
`Java.Interop.JniMethodSignatureAttribute`.  This allows
`jcw-gen Java.Base-Tests.dll` to generate Java Callable Wrappers for
`Java.BaseTests.MyRunnable`.

Update `Java.Interop.Export.dll` so that `MarshalMemberBuilder`
doesn't use `Expression.GetActionType()` or
`Expression.GetFuncType()`, as .NET doesn't support the use of
generic types in [`Marshal.GetFunctionPointerForDelegate()`][0].
Instead, we need to use `System.Reflection.Emit` to define our own
custom delegate types.

~~ Comparison with Android ~~

Android bindings (both Classic Xamarin.Android and
.NET SDK for Android) use `generator`-emitted "connector methods"
which are specified in the `[Register]` attribute (see also 99897b2):

	namespace Java.Lang {
	    [Register ("java/lang/Object" DoNotGenerateAcw=true)]
	    partial class Object {
	        [Register ("equals", "(Ljava/lang/Object;)Z", "GetEquals_Ljava_lang_Object_Handler")]
	        public virtual unsafe bool Equals (Java.Lang.Object? obj)
	            => …

	        static Delegate GetEquals_Ljava_lang_Object_Handler()
	            => JNINativeWrapper.CreateDelegate((_JniMarshal_PPL_Z) n_Equals_Ljava_lang_Object_);
	        static bool n_Equals_Ljava_lang_Object_ (IntPtr jnienv, IntPtr native__this, IntPtr native_obj)
	            => …
	    }
	}

`jcw-gen` emits the "connector method" into the resulting
Java Callable Wrappers for appropriate subclasses:

	String __md_methods =
	    "n_equals:(Ljava/lang/Object;)Z:GetEquals_Ljava_lang_Object_Handler\n";

At runtime, `AndroidTypeManager.RegisterNativeMembers()` will lookup
`Object.GetEquals_Ljava_lang_Object_Handler()` and invoke it.
The returned `Delegate` instance is provided to
`JNIEnv::RegisterNatives()`.

The problem with this approach is that it's inflexible: there is no
way to participate in the "connector method" infrastructure to alter
marshaling behavior.  If you need custom behavior, e.g.
`Android.Graphics.Color` customizations, you need to update
`generator` and rebuild the binding assemblies.

(This has been "fine" for the past 10+ years, so this hasn't been a
deal breaker.)

For `Java.Base`, @jonpryor wants to support the custom marshaling
infrastructure introduced in 77a6bf8.  This would allow types to
participate in JNI marshal method ("connector method") generation
*at runtime*, allowing specialization based on the current set of
types and assemblies.

This means we *don't* need to specify a "connector method" in
`[JniMethodSignatureAttribute]`, nor generate them.

	namespace Java.Lang {
	    [JniTypeSignatureAttribute("java/lang/Object", GenerateJavaPeer=false)]
	    partial class Object {
	        [JniMethodSignatureAttribute("equals", "(Ljava/lang/Object;)Z")]
	        public virtual unsafe bool Equals (Java.Lang.Object? obj)
	            => …

	        // No GetEquals_Ljava_lang_Object_Handler()
	        // No n_Equals_Ljava_lang_Object_()
	    }
	}

This should result in smaller binding assemblies.

The downside is that runtime costs *increase*, significantly.
Instead of looking up and invoking a "connector method", the method
must be *generated* via System.Linq.Expressions expression trees,
then compiled into IL, then JIT'd, all before it can be used.

We have no timing data to indicate how "bad" this overhead will be.

As always, the hope is that `tools/jnimarshalmethod-gen` (176240d)
can be used to get the "best of both worlds": the flexibility of
custom marshalers, without the runtime overhead.  But…

  * dotnet#14
  * dotnet#616

At this point in time, `jnimarshalmethod-gen` *cannot* work under
.NET 6.  This will need to be addressed in order for custom marshalers
to be a useful solution.

`Java.Base` will use "connector method"-less bindings to act as a
"forcing function" in getting `jnimarshalmethod-gen` working in .NET.

[0]: https://docs.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.marshal.getfunctionpointerfordelegate?view=net-6.0
jonpryor added a commit to jonpryor/java.interop that referenced this issue May 4, 2022
@jonpryor
[Java.Base-Tests] Test Java-to-Managed invocations for Java.Base
538f429 
Commit bc5bcf4 (Desktop `Java.Base` binding) had a TODO:

> ~~ TODO: Marshal Methods ~~
>
> Marshal methods are currently skipped.  Java-to-managed invocations
> are not currently supported.

No marshal methods means no facility for Java-to-managed invocations.

Add a new `tests/Java.Base-Tests` unit test assembly, and test
Java-to-managed invocations.

This requires touching and updating ~everything. 😅

Update `Java.Interop.dll` to contain a new
`Java.Interop.JniMethodSignatureAttribute` custom attribute.

Update `generator` to begin emitting `[JniMethodSignature]` on bound
methods.  This is necessary so that `jcw-gen` knows the JNI method
signature of Java methods to emit.  As part of this, *remove* the
`JniTypeSignatureAttr` type added in bc5bcf4; trying to follow the
`JniTypeSignatureAttr` pattern for `JniMethodSignatureAttr` would
make for more intrusive code changes.  Instead, "re-use" the existing
`RegisterAttr` type, adding a `RegisterAttr.MemberType` property so
that we can select between Android `[Register]` output vs. "Desktop"
`[JniTypeSignature]` and `[JniMethodSignature]` output.  This in turn
requires updating many `generator-Tests` artifacts.

Update `Java.Interop.Tools.JavaCallableWrappers` to look for
`Java.Interop.JniTypeSignatureAttribute` and
`Java.Interop.JniMethodSignatureAttribute`.  This allows
`jcw-gen Java.Base-Tests.dll` to generate Java Callable Wrappers for
`Java.BaseTests.MyRunnable`.

Update `Java.Interop.Export.dll` so that `MarshalMemberBuilder`
doesn't use `Expression.GetActionType()` or
`Expression.GetFuncType()`, as .NET doesn't support the use of
generic types in [`Marshal.GetFunctionPointerForDelegate()`][0].
Instead, we need to use `System.Reflection.Emit` to define our own
custom delegate types.

~~ Comparison with Android ~~

Android bindings (both Classic Xamarin.Android and
.NET SDK for Android) use `generator`-emitted "connector methods"
which are specified in the `[Register]` attribute (see also 99897b2):

	namespace Java.Lang {
	    [Register ("java/lang/Object" DoNotGenerateAcw=true)]
	    partial class Object {
	        [Register ("equals", "(Ljava/lang/Object;)Z", "GetEquals_Ljava_lang_Object_Handler")]
	        public virtual unsafe bool Equals (Java.Lang.Object? obj)
	            => …

	        static Delegate GetEquals_Ljava_lang_Object_Handler()
	            => JNINativeWrapper.CreateDelegate((_JniMarshal_PPL_Z) n_Equals_Ljava_lang_Object_);
	        static bool n_Equals_Ljava_lang_Object_ (IntPtr jnienv, IntPtr native__this, IntPtr native_obj)
	            => …
	    }
	}

`jcw-gen` emits the "connector method" into the resulting
Java Callable Wrappers for appropriate subclasses:

	String __md_methods =
	    "n_equals:(Ljava/lang/Object;)Z:GetEquals_Ljava_lang_Object_Handler\n";

At runtime, `AndroidTypeManager.RegisterNativeMembers()` will lookup
`Object.GetEquals_Ljava_lang_Object_Handler()` and invoke it.
The returned `Delegate` instance is provided to
`JNIEnv::RegisterNatives()`.

The problem with this approach is that it's inflexible: there is no
way to participate in the "connector method" infrastructure to alter
marshaling behavior.  If you need custom behavior, e.g.
`Android.Graphics.Color` customizations, you need to update
`generator` and rebuild the binding assemblies.

(This has been "fine" for the past 10+ years, so this hasn't been a
deal breaker.)

For `Java.Base`, @jonpryor wants to support the custom marshaling
infrastructure introduced in 77a6bf8.  This would allow types to
participate in JNI marshal method ("connector method") generation
*at runtime*, allowing specialization based on the current set of
types and assemblies.

This means we *don't* need to specify a "connector method" in
`[JniMethodSignatureAttribute]`, nor generate them.

	namespace Java.Lang {
	    [JniTypeSignatureAttribute("java/lang/Object", GenerateJavaPeer=false)]
	    partial class Object {
	        [JniMethodSignatureAttribute("equals", "(Ljava/lang/Object;)Z")]
	        public virtual unsafe bool Equals (Java.Lang.Object? obj)
	            => …

	        // No GetEquals_Ljava_lang_Object_Handler()
	        // No n_Equals_Ljava_lang_Object_()
	    }
	}

This should result in smaller binding assemblies.

The downside is that runtime costs *increase*, significantly.
Instead of looking up and invoking a "connector method", the method
must be *generated* via System.Linq.Expressions expression trees,
then compiled into IL, then JIT'd, all before it can be used.

We have no timing data to indicate how "bad" this overhead will be.

As always, the hope is that `tools/jnimarshalmethod-gen` (176240d)
can be used to get the "best of both worlds": the flexibility of
custom marshalers, without the runtime overhead.  But…

  * dotnet#14
  * dotnet#616

At this point in time, `jnimarshalmethod-gen` *cannot* work under
.NET 6.  This will need to be addressed in order for custom marshalers
to be a useful solution.

`Java.Base` will use "connector method"-less bindings to act as a
"forcing function" in getting `jnimarshalmethod-gen` working in .NET.

[0]: https://docs.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.marshal.getfunctionpointerfordelegate?view=net-6.0
jonpryor added a commit to jonpryor/java.interop that referenced this issue May 4, 2022
@jonpryor
[Java.Base-Tests] Test Java-to-Managed invocations for Java.Base
2b75b66 
Commit bc5bcf4 (Desktop `Java.Base` binding) had a TODO:

> ~~ TODO: Marshal Methods ~~
>
> Marshal methods are currently skipped.  Java-to-managed invocations
> are not currently supported.

No marshal methods means no facility for Java-to-managed invocations.

Add a new `tests/Java.Base-Tests` unit test assembly, and test
Java-to-managed invocations.

This requires touching and updating ~everything. 😅

Update `Java.Interop.dll` to contain a new
`Java.Interop.JniMethodSignatureAttribute` custom attribute.

Update `generator` to begin emitting `[JniMethodSignature]` on bound
methods.  This is necessary so that `jcw-gen` knows the JNI method
signature of Java methods to emit.  As part of this, *remove* the
`JniTypeSignatureAttr` type added in bc5bcf4; trying to follow the
`JniTypeSignatureAttr` pattern for `JniMethodSignatureAttr` would
make for more intrusive code changes.  Instead, "re-use" the existing
`RegisterAttr` type, adding a `RegisterAttr.MemberType` property so
that we can select between Android `[Register]` output vs. "Desktop"
`[JniTypeSignature]` and `[JniMethodSignature]` output.  This in turn
requires updating many `generator-Tests` artifacts.

Update `Java.Interop.Tools.JavaCallableWrappers` to look for
`Java.Interop.JniTypeSignatureAttribute` and
`Java.Interop.JniMethodSignatureAttribute`.  This allows
`jcw-gen Java.Base-Tests.dll` to generate Java Callable Wrappers for
`Java.BaseTests.MyRunnable`.

Update `Java.Interop.Export.dll` so that `MarshalMemberBuilder`
doesn't use `Expression.GetActionType()` or
`Expression.GetFuncType()`, as .NET doesn't support the use of
generic types in [`Marshal.GetFunctionPointerForDelegate()`][0].
Instead, we need to use `System.Reflection.Emit` to define our own
custom delegate types.

~~ Comparison with Android ~~

Android bindings (both Classic Xamarin.Android and
.NET SDK for Android) use `generator`-emitted "connector methods"
which are specified in the `[Register]` attribute (see also 99897b2):

	namespace Java.Lang {
	    [Register ("java/lang/Object" DoNotGenerateAcw=true)]
	    partial class Object {
	        [Register ("equals", "(Ljava/lang/Object;)Z", "GetEquals_Ljava_lang_Object_Handler")]
	        public virtual unsafe bool Equals (Java.Lang.Object? obj)
	            => …

	        static Delegate GetEquals_Ljava_lang_Object_Handler()
	            => JNINativeWrapper.CreateDelegate((_JniMarshal_PPL_Z) n_Equals_Ljava_lang_Object_);
	        static bool n_Equals_Ljava_lang_Object_ (IntPtr jnienv, IntPtr native__this, IntPtr native_obj)
	            => …
	    }
	}

`jcw-gen` emits the "connector method" into the resulting
Java Callable Wrappers for appropriate subclasses:

	String __md_methods =
	    "n_equals:(Ljava/lang/Object;)Z:GetEquals_Ljava_lang_Object_Handler\n";

At runtime, `AndroidTypeManager.RegisterNativeMembers()` will lookup
`Object.GetEquals_Ljava_lang_Object_Handler()` and invoke it.
The returned `Delegate` instance is provided to
`JNIEnv::RegisterNatives()`.

The problem with this approach is that it's inflexible: there is no
way to participate in the "connector method" infrastructure to alter
marshaling behavior.  If you need custom behavior, e.g.
`Android.Graphics.Color` customizations, you need to update
`generator` and rebuild the binding assemblies.

(This has been "fine" for the past 10+ years, so this hasn't been a
deal breaker.)

For `Java.Base`, @jonpryor wants to support the custom marshaling
infrastructure introduced in 77a6bf8.  This would allow types to
participate in JNI marshal method ("connector method") generation
*at runtime*, allowing specialization based on the current set of
types and assemblies.

This means we *don't* need to specify a "connector method" in
`[JniMethodSignatureAttribute]`, nor generate them.

	namespace Java.Lang {
	    [JniTypeSignatureAttribute("java/lang/Object", GenerateJavaPeer=false)]
	    partial class Object {
	        [JniMethodSignatureAttribute("equals", "(Ljava/lang/Object;)Z")]
	        public virtual unsafe bool Equals (Java.Lang.Object? obj)
	            => …

	        // No GetEquals_Ljava_lang_Object_Handler()
	        // No n_Equals_Ljava_lang_Object_()
	    }
	}

This should result in smaller binding assemblies.

The downside is that runtime costs *increase*, significantly.
Instead of looking up and invoking a "connector method", the method
must be *generated* via System.Linq.Expressions expression trees,
then compiled into IL, then JIT'd, all before it can be used.

We have no timing data to indicate how "bad" this overhead will be.

As always, the hope is that `tools/jnimarshalmethod-gen` (176240d)
can be used to get the "best of both worlds": the flexibility of
custom marshalers, without the runtime overhead.  But…

  * dotnet#14
  * dotnet#616

At this point in time, `jnimarshalmethod-gen` *cannot* work under
.NET 6.  This will need to be addressed in order for custom marshalers
to be a useful solution.

`Java.Base` will use "connector method"-less bindings to act as a
"forcing function" in getting `jnimarshalmethod-gen` working in .NET.

[0]: https://docs.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.marshal.getfunctionpointerfordelegate?view=net-6.0
jonpryor added a commit to jonpryor/java.interop that referenced this issue May 4, 2022
@jonpryor
[Java.Base-Tests] Test Java-to-Managed invocations for Java.Base
bd1cce6 
Commit bc5bcf4 (Desktop `Java.Base` binding) had a TODO:

> ~~ TODO: Marshal Methods ~~
>
> Marshal methods are currently skipped.  Java-to-managed invocations
> are not currently supported.

No marshal methods means no facility for Java-to-managed invocations.

Add a new `tests/Java.Base-Tests` unit test assembly, and test
Java-to-managed invocations.

This requires touching and updating ~everything. 😅

Update `Java.Interop.dll` to contain a new
`Java.Interop.JniMethodSignatureAttribute` custom attribute.

Update `generator` to begin emitting `[JniMethodSignature]` on bound
methods.  This is necessary so that `jcw-gen` knows the JNI method
signature of Java methods to emit.  As part of this, *remove* the
`JniTypeSignatureAttr` type added in bc5bcf4; trying to follow the
`JniTypeSignatureAttr` pattern for `JniMethodSignatureAttr` would
make for more intrusive code changes.  Instead, "re-use" the existing
`RegisterAttr` type, adding a `RegisterAttr.MemberType` property so
that we can select between Android `[Register]` output vs. "Desktop"
`[JniTypeSignature]` and `[JniMethodSignature]` output.  This in turn
requires updating many `generator-Tests` artifacts.

Update `Java.Interop.Tools.JavaCallableWrappers` to look for
`Java.Interop.JniTypeSignatureAttribute` and
`Java.Interop.JniMethodSignatureAttribute`.  This allows
`jcw-gen Java.Base-Tests.dll` to generate Java Callable Wrappers for
`Java.BaseTests.MyRunnable`.

Update `Java.Interop.Export.dll` so that `MarshalMemberBuilder`
doesn't use `Expression.GetActionType()` or
`Expression.GetFuncType()`, as .NET doesn't support the use of
generic types in [`Marshal.GetFunctionPointerForDelegate()`][0].
Instead, we need to use `System.Reflection.Emit` to define our own
custom delegate types.

~~ Comparison with Android ~~

Android bindings (both Classic Xamarin.Android and
.NET SDK for Android) use `generator`-emitted "connector methods"
which are specified in the `[Register]` attribute (see also 99897b2):

	namespace Java.Lang {
	    [Register ("java/lang/Object" DoNotGenerateAcw=true)]
	    partial class Object {
	        [Register ("equals", "(Ljava/lang/Object;)Z", "GetEquals_Ljava_lang_Object_Handler")]
	        public virtual unsafe bool Equals (Java.Lang.Object? obj)
	            => …

	        static Delegate GetEquals_Ljava_lang_Object_Handler()
	            => JNINativeWrapper.CreateDelegate((_JniMarshal_PPL_Z) n_Equals_Ljava_lang_Object_);
	        static bool n_Equals_Ljava_lang_Object_ (IntPtr jnienv, IntPtr native__this, IntPtr native_obj)
	            => …
	    }
	}

`jcw-gen` emits the "connector method" into the resulting
Java Callable Wrappers for appropriate subclasses:

	String __md_methods =
	    "n_equals:(Ljava/lang/Object;)Z:GetEquals_Ljava_lang_Object_Handler\n";

At runtime, `AndroidTypeManager.RegisterNativeMembers()` will lookup
`Object.GetEquals_Ljava_lang_Object_Handler()` and invoke it.
The returned `Delegate` instance is provided to
`JNIEnv::RegisterNatives()`.

The problem with this approach is that it's inflexible: there is no
way to participate in the "connector method" infrastructure to alter
marshaling behavior.  If you need custom behavior, e.g.
`Android.Graphics.Color` customizations, you need to update
`generator` and rebuild the binding assemblies.

(This has been "fine" for the past 10+ years, so this hasn't been a
deal breaker.)

For `Java.Base`, @jonpryor wants to support the custom marshaling
infrastructure introduced in 77a6bf8.  This would allow types to
participate in JNI marshal method ("connector method") generation
*at runtime*, allowing specialization based on the current set of
types and assemblies.

This means we *don't* need to specify a "connector method" in
`[JniMethodSignatureAttribute]`, nor generate them.

	namespace Java.Lang {
	    [JniTypeSignatureAttribute("java/lang/Object", GenerateJavaPeer=false)]
	    partial class Object {
	        [JniMethodSignatureAttribute("equals", "(Ljava/lang/Object;)Z")]
	        public virtual unsafe bool Equals (Java.Lang.Object? obj)
	            => …

	        // No GetEquals_Ljava_lang_Object_Handler()
	        // No n_Equals_Ljava_lang_Object_()
	    }
	}

This should result in smaller binding assemblies.

The downside is that runtime costs *increase*, significantly.
Instead of looking up and invoking a "connector method", the method
must be *generated* via System.Linq.Expressions expression trees,
then compiled into IL, then JIT'd, all before it can be used.

We have no timing data to indicate how "bad" this overhead will be.

As always, the hope is that `tools/jnimarshalmethod-gen` (176240d)
can be used to get the "best of both worlds": the flexibility of
custom marshalers, without the runtime overhead.  But…

  * dotnet#14
  * dotnet#616

At this point in time, `jnimarshalmethod-gen` *cannot* work under
.NET 6.  This will need to be addressed in order for custom marshalers
to be a useful solution.

`Java.Base` will use "connector method"-less bindings to act as a
"forcing function" in getting `jnimarshalmethod-gen` working in .NET.

[0]: https://docs.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.marshal.getfunctionpointerfordelegate?view=net-6.0
jonpryor added a commit that referenced this issue May 5, 2022
@jonpryor
[Java.Base-Tests] Test Java-to-Managed invocations for Java.Base (#975)
4787e01 
Commit bc5bcf4 (Desktop `Java.Base` binding) had a TODO:

> ~~ TODO: Marshal Methods ~~
>
> Marshal methods are currently skipped.  Java-to-managed invocations
> are not currently supported.

No marshal methods means no facility for Java-to-managed invocations.

Add a new `tests/Java.Base-Tests` unit test assembly, and test
Java-to-managed invocations.

This requires touching and updating ~everything. 😅

Update `Java.Interop.dll` to contain a new
`Java.Interop.JniMethodSignatureAttribute` custom attribute.

Update `generator` to begin emitting `[JniMethodSignature]` on bound
methods.  This is necessary so that `jcw-gen` knows the JNI method
signature of Java methods to emit.  As part of this, *remove* the
`JniTypeSignatureAttr` type added in bc5bcf4; trying to follow the
`JniTypeSignatureAttr` pattern for `JniMethodSignatureAttr` would
make for more intrusive code changes.  Instead, "re-use" the existing
`RegisterAttr` type, adding a `RegisterAttr.MemberType` property so
that we can select between Android `[Register]` output vs. "Desktop"
`[JniTypeSignature]` and `[JniMethodSignature]` output.  This in turn
requires updating many `generator-Tests` artifacts.

Update `Java.Interop.Tools.JavaCallableWrappers` to look for
`Java.Interop.JniTypeSignatureAttribute` and
`Java.Interop.JniMethodSignatureAttribute`.  This allows
`jcw-gen Java.Base-Tests.dll` to generate Java Callable Wrappers for
`Java.BaseTests.MyRunnable`.

Update `Java.Interop.Export.dll` so that `MarshalMemberBuilder`
doesn't use `Expression.GetActionType()` or
`Expression.GetFuncType()`, as .NET doesn't support the use of
generic types in [`Marshal.GetFunctionPointerForDelegate()`][0].
Instead, we need to use `System.Reflection.Emit` to define our own
custom delegate types.

~~ Comparison with Android ~~

Android bindings (both Classic Xamarin.Android and
.NET SDK for Android) use `generator`-emitted "connector methods"
which are specified in the `[Register]` attribute (see also 99897b2):

	namespace Java.Lang {
	    [Register ("java/lang/Object" DoNotGenerateAcw=true)]
	    partial class Object {
	        [Register ("equals", "(Ljava/lang/Object;)Z", "GetEquals_Ljava_lang_Object_Handler")]
	        public virtual unsafe bool Equals (Java.Lang.Object? obj)
	            => …

	        static Delegate GetEquals_Ljava_lang_Object_Handler()
	            => JNINativeWrapper.CreateDelegate((_JniMarshal_PPL_Z) n_Equals_Ljava_lang_Object_);
	        static bool n_Equals_Ljava_lang_Object_ (IntPtr jnienv, IntPtr native__this, IntPtr native_obj)
	            => …
	    }
	}

`jcw-gen` emits the "connector method" into the resulting
Java Callable Wrappers for appropriate subclasses:

	String __md_methods =
	    "n_equals:(Ljava/lang/Object;)Z:GetEquals_Ljava_lang_Object_Handler\n";

At runtime, `AndroidTypeManager.RegisterNativeMembers()` will lookup
`Object.GetEquals_Ljava_lang_Object_Handler()` and invoke it.
The returned `Delegate` instance is provided to
`JNIEnv::RegisterNatives()`.

The problem with this approach is that it's inflexible: there is no
way to participate in the "connector method" infrastructure to alter
marshaling behavior.  If you need custom behavior, e.g.
`Android.Graphics.Color` customizations, you need to update
`generator` and rebuild the binding assemblies.

(This has been "fine" for the past 10+ years, so this hasn't been a
deal breaker.)

For `Java.Base`, @jonpryor wants to support the custom marshaling
infrastructure introduced in 77a6bf8.  This would allow types to
participate in JNI marshal method ("connector method") generation
*at runtime*, allowing specialization based on the current set of
types and assemblies.

This means we *don't* need to specify a "connector method" in
`[JniMethodSignatureAttribute]`, nor generate them.

	namespace Java.Lang {
	    [JniTypeSignatureAttribute("java/lang/Object", GenerateJavaPeer=false)]
	    partial class Object {
	        [JniMethodSignatureAttribute("equals", "(Ljava/lang/Object;)Z")]
	        public virtual unsafe bool Equals (Java.Lang.Object? obj)
	            => …

	        // No GetEquals_Ljava_lang_Object_Handler()
	        // No n_Equals_Ljava_lang_Object_()
	    }
	}

This should result in smaller binding assemblies.

The downside is that runtime costs *increase*, significantly.
Instead of looking up and invoking a "connector method", the method
must be *generated* via System.Linq.Expressions expression trees,
then compiled into IL, then JIT'd, all before it can be used.

We have no timing data to indicate how "bad" this overhead will be.

As always, the hope is that `tools/jnimarshalmethod-gen` (176240d)
can be used to get the "best of both worlds": the flexibility of
custom marshalers, without the runtime overhead.  But…

  * #14
  * #616

At this point in time, `jnimarshalmethod-gen` *cannot* work under
.NET 6.  This will need to be addressed in order for custom marshalers
to be a useful solution.

`Java.Base` will use "connector method"-less bindings to act as a
"forcing function" in getting `jnimarshalmethod-gen` working in .NET.

[0]: https://docs.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.marshal.getfunctionpointerfordelegate?view=net-6.0
@jonpryor jonpryor mentioned this issue Feb 1, 2023
Merged
jonpryor added a commit to jonpryor/java.interop that referenced this issue Feb 1, 2023
@jonpryor
[Java.Interop.Tools.Expressions] Add Java.Interop.Tools.Expressions
e510fbb 
Fixes: dotnet#616

Context: dotnet#14
Context: ff4053c
Context: da5d1b8
Context: 4787e01
Context: 41ba348

Remember `jnimarshalmethod-gen` (176240d)?  And it's crazy idea to
use the System.Linq.Expressions-based custom marshaling
infrastructure (ff4053c, da5d1b8) to generate JNI marshal methods
at build/packaging time.  And how we had to back burner it because
it depended upon System.Reflection.Emit being able to write
assemblies to disk, which is a feature that never made it to
.NET Core, and is still lacking as of .NET 7?

Add `src/Java.Interop.Tools.Expressions`, which contains code which
uses Mono.Cecil to compile `Expression<T>` expressions to IL.

Then update jnimarshalmethod-gen to use it!

Testing this puppy:

	% mkdir _x
	% dotnet bin/Debug-net7.0/jnimarshalmethod-gen.dll \
	  bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll \
	  -v --keeptemp \
	  --jvm /Library/Java/JavaVirtualMachines/microsoft-11.jdk/Contents/Home/lib/jli/libjli.dylib \
	  -o _x \
	  -L bin/TestDebug-net7.0 \
	  -L /usr/local/share/dotnet/shared/Microsoft.NETCore.App/7.0.0

First param is assembly to process; `Java.Interop.Export-Tests.dll`
is handy because that's what the `run-test-jnimarshal` target in
`Makefile` processes.

`-v` is verbose output, `--keeptemp` is keep temporary files
(and may be vestigial).

`--jvm PATH` is the path to the JVM library to load+use.

`-o DIR` is where to place output files; this will create
`_x/Java.Interop.Export-Tests.dll`.

`-L DIR` adds `DIR` to library resolution paths; this adds
`bin/TestDebug/net7.0` (dependencies of `Java.Interop.Export-Tests.dll`)
and `Microsoft.NETCore.App/7.0.0-rc.1.22422.12` (net7 libs).

What does that *do*?

	% ikdasm bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll > beg.il
	% ikdasm _x/Java.Interop.Export-Tests.dll > end.il
	% git diff --no-index beg.il end.il

is a ~2KB diff which shows, paraphrasing greatly:

	public partial class ExportTest {
	    partial class __<$>_jni_marshal_methods {
	        static IntPtr funcIJavaObject (IntPtr jnienv, IntPtr this) => …
	        // …
	        [JniAddNativeMethodRegistration]
	        static void __RegisterNativeMembers (JniNativeMethodRegistrationArguments args) => …
	    }
	}
	internal delegate long _JniMarshal_PP_J (IntPtr jnienv, IntPtr self);
	// …

wherein `ExportTest._<$>_jni_marshal_methods` and the `_JniMarshal*`
delegate types are added to the assembly.

This also unblocks the desire stated in 4787e01:

> For `Java.Base`, @jonpryor wants to support the custom marshaling
> infrastructure introduced in 77a6bf8.  This would allow types to
> participate in JNI marshal method ("connector method") generation
> *at runtime*, allowing specialization based on the current set of
> types and assemblies.

One-off tests: ensure that the generated assembly can be decompiled:

	% ikdasm  bin/TestDebug-net7.0/Java.Interop.Tools.Expressions-Tests-ExpressionAssemblyBuilderTests.dll
	% monodis bin/TestDebug-net7.0/Java.Interop.Tools.Expressions-Tests-ExpressionAssemblyBuilderTests.dll

	% ikdasm  _x/Java.Interop.Export-Tests.dll
	% monodis _x/Java.Interop.Export-Tests.dll

Re-enable most of `Java.Interop.Export-Tests.dll` for .NET 7;
see 41ba348, which disabled those tests.

TODO: we should be able to use `jnimarshalmethod-gen` output
as part of the unit tests, a'la c8f3e51.  Unfortunately,
`dotnet test` doesn't like the updated assembly.  Why?

	# sanity test: tests run!
	% % dotnet test bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll
	…
	Passed!  - Failed:     0, Passed:    17, Skipped:     0, Total:    17, Duration: 110 ms - Java.Interop.Export-Tests.dll (net7.0)

	# backup!
	% cp bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll .

	# …previous jnimarshalmethod-gen.dll invocation…

	# replace the test assembly
	% \cp _x/Java.Interop.Export-Tests.dll bin/TestDebug-net7.0

	# run tests for updated assembly
	% dotnet test bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll
	…
	No test is available in …/bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll.
	Make sure that test discoverer & executors are registered and platform & framework version settings are appropriate and try again.

	Additionally, path to test adapters can be specified using /TestAdapterPath command. Example  /TestAdapterPath:<pathToCustomAdapters>.

Huh?

Assembly diff: https://gist.github.com/jonpryor/b8233444f2e51043732bea922f6afc81
jonpryor added a commit to jonpryor/java.interop that referenced this issue Feb 8, 2023
@jonpryor
[Java.Interop.Tools.Expressions] Add Java.Interop.Tools.Expressions
7ab1209 
Fixes: dotnet#616

Context: dotnet#14
Context: ff4053c
Context: da5d1b8
Context: 4787e01
Context: 41ba348

Remember `jnimarshalmethod-gen` (176240d)?  And it's crazy idea to
use the `System.Linq.Expressions`-based custom marshaling
infrastructure (ff4053c, da5d1b8) to generate JNI marshal methods
at build/packaging time.  And how we had to back burner it because
it depended upon `System.Reflection.Emit` being able to write
assemblies to disk, which is a feature that never made it to
.NET Core, and is still lacking as of .NET 7?

Add `src/Java.Interop.Tools.Expressions`, which contains code which
uses Mono.Cecil to compile `Expression<T>` expressions to IL.

Then update `jnimarshalmethod-gen` to use it!

~~ Usage ~~

	% dotnet bin/Debug-net7.0/jnimarshalmethod-gen.dll \
	  bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll \
	  -v -v --keeptemp \
	  --jvm /Library/Java/JavaVirtualMachines/microsoft-11.jdk/Contents/Home/lib/jli/libjli.dylib \
	  -o _x \
	  -L bin/TestDebug-net7.0 \
	  -L /usr/local/share/dotnet/shared/Microsoft.NETCore.App/7.0.0

First param is assembly to process; `Java.Interop.Export-Tests.dll`
is handy because that's what the `run-test-jnimarshal` target in
`Makefile` processed.

  * `-v -v` is *really* verbose output

  * `--keeptemp` is keep temporary files, in this case
    `_x/Java.Interop.Export-Tests.dll.cecil`.

  * `--jvm PATH` is the path to the JVM library to load+use.

  * `-o DIR` is where to place output files; this will create
    `_x/Java.Interop.Export-Tests.dll`.

  * `-L DIR` adds `DIR` to library resolution paths; this adds
    `bin/TestDebug/net7.0` (dependencies of
    `Java.Interop.Export-Tests.dll`) and
    `Microsoft.NETCore.App/7.0.0-rc.1.22422.12` (net7 libs).

By default the directories containing input assemblies and the
directory containing `System.Private.CoreLib.dll` are part of the
default `-L` list.

When running in-tree, e.g. AzDO pipeline execution, `--jvm PATH`
will attempt to read the path in `bin/Build*/JdkInfo.props`
a'la `TestJVM` (002dea4).  This allows an in-place update in
`core-tests.yaml` which does:

	dotnet bin/Debug-net7.0/jnimarshalmethod-gen.dll \
	  bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll \
	  -v -v --keeptemp -o bin/TestDebug-net7.0

~~ Using `jnimarshalmethod-gen` output ~~

What does `jnimarshalmethod-gen` *do*?

	% ikdasm bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll > beg.il
	% ikdasm _x/Java.Interop.Export-Tests.dll > end.il
	% git diff --no-index beg.il end.il
	# https://gist.github.com/jonpryor/b8233444f2e51043732bea922f6afc81

is a ~1KB diff which shows, paraphrasing greatly:

	public partial class ExportTest {
	    partial class '__<$>_jni_marshal_methods' {
	        static IntPtr funcIJavaObject (IntPtr jnienv, IntPtr this) => …
	        // …
	        [JniAddNativeMethodRegistration]
	        static void __RegisterNativeMembers (JniNativeMethodRegistrationArguments args) => …
	    }
	}
	internal delegate long _JniMarshal_PP_L (IntPtr jnienv, IntPtr self);
	// …

wherein `ExportTest._<$>_jni_marshal_methods` and the `_JniMarshal*`
delegate types are added to the assembly.

This also unblocks the desire stated in 4787e01:

> For `Java.Base`, @jonpryor wants to support the custom marshaling
> infrastructure introduced in 77a6bf8.  This would allow types to
> participate in JNI marshal method ("connector method") generation
> *at runtime*, allowing specialization based on the current set of
> types and assemblies.

What can we do with this `jnimarshalmethod-gen` output?  Use it!

First, make sure the tests work:

	% dotnet test --logger "console;verbosity=detailed" bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll
	…
	Passed!  - Failed:     0, Passed:    17, Skipped:     0, Total:    17, Duration: 103 ms - Java.Interop.Export-Tests.dll (net7.0)

Then update/replace the unit test assembly with
`jnimarshalmethod-gen` output:

	% \cp _x/Java.Interop.Export-Tests.dll bin/TestDebug-net7.0
	% dotnet test --logger "console;verbosity=detailed" bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll
	…
	Total tests: 17
	     Passed: 17

`core-tests.yaml` has been updated to do this.

~~ One-Off Tests ~~

One-off tests: ensure that the generated assembly can be decompiled:

	% ikdasm  bin/TestDebug-net7.0/Java.Interop.Tools.Expressions-Tests-ExpressionAssemblyBuilderTests.dll
	% monodis bin/TestDebug-net7.0/Java.Interop.Tools.Expressions-Tests-ExpressionAssemblyBuilderTests.dll

	% ikdasm  _x/Java.Interop.Export-Tests.dll
	% monodis _x/Java.Interop.Export-Tests.dll
	# which currently fails :-()

Re-enable most of `Java.Interop.Export-Tests.dll` for .NET 7;
see 41ba348, which disabled those tests.

To verify the generated IL, use the [dotnet-ilverify][0] tool:

	dotnet tool install --global dotnet-ilverify

Usage of which is "weird":

	$HOME/.dotnet/tools/ilverify _x/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.0/*.dll'
	All Classes and Methods in /Volumes/Xamarin-Work/src/xamarin/Java.Interop/_x/Java.Interop.Export-Tests.dll Verified.
	# no errors!

where:

  * `--tokens`: Include metadata tokens in error messages.
  * `--system-module NAME`: set the "System module name".  Defaults
    to `mscorlib`, which is wrong for .NET 5+, so this must be set to
    `System.Private.CoreLib` (no `.dll` suffix!).
  * `-r FILE-GLOB`: Where to resolve assembly references for the
    input assembly.  Fortunately file globs are supported…

~~ Removing `System.Private.CoreLib` ~~

`System.Private.CoreLib.dll` is *private*; it's not part of the
public assembly surface area, so you can't use
`csc -r:System.Private.CoreLib …` and expect it to work.  This makes
things interesting because *at runtime* everything "important" is in
`System.Private.CoreLib.dll`, like `System.Object`.

Specifically, if we do the "obvious" thing and do:

	newTypeDefinition.BaseType = assemblyDefinition.MainModule
	    .ImportReference (typeof (object));

you're gonna have a bad type, because the resulting IL for
`newTypeDefinition` will have a base class of
`[System.Private.CoreLib]System.Object`, which isn't usable.

Fix this by:

 1. Writing the assembly to a `Stream`.
 2. Reading the `Stream` from (1)
 3. Fixing all member references and assembly references so that
    `System.Private.CoreLib` is not referenced.

If `jnimarshalmethod-gen.dll --keeptemp` is used, then a `.cecil`
file is created with the contents of (1).

Additionally, and unexpectedly -- [jbevain/cecil#895][1] -- Mono.Cecil
adds a reference to the assembly being modified.  Remove the declaring
assembly from `AssemblyReferences`.

[0]: https://www.nuget.org/packages/dotnet-ilverify
[1]: jbevain/cecil#895
jonpryor added a commit to jonpryor/java.interop that referenced this issue Feb 9, 2023
@jonpryor
[Java.Interop.Tools.Expressions] Add Java.Interop.Tools.Expressions
132e49c 
Fixes: dotnet#616

Context: dotnet#14
Context: ff4053c
Context: da5d1b8
Context: 4787e01
Context: 41ba348

Remember `jnimarshalmethod-gen` (176240d)?  And it's crazy idea to
use the `System.Linq.Expressions`-based custom marshaling
infrastructure (ff4053c, da5d1b8) to generate JNI marshal methods
at build/packaging time.  And how we had to back burner it because
it depended upon `System.Reflection.Emit` being able to write
assemblies to disk, which is a feature that never made it to
.NET Core, and is still lacking as of .NET 7?

Add `src/Java.Interop.Tools.Expressions`, which contains code which
uses Mono.Cecil to compile `Expression<T>` expressions to IL.

Then update `jnimarshalmethod-gen` to use it!

~~ Usage ~~

	% dotnet bin/Debug-net7.0/jnimarshalmethod-gen.dll \
	  bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll \
	  -v -v --keeptemp \
	  --jvm /Library/Java/JavaVirtualMachines/microsoft-11.jdk/Contents/Home/lib/jli/libjli.dylib \
	  -o _x \
	  -L bin/TestDebug-net7.0 \
	  -L /usr/local/share/dotnet/shared/Microsoft.NETCore.App/7.0.0

First param is assembly to process; `Java.Interop.Export-Tests.dll`
is handy because that's what the `run-test-jnimarshal` target in
`Makefile` processed.

  * `-v -v` is *really* verbose output

  * `--keeptemp` is keep temporary files, in this case
    `_x/Java.Interop.Export-Tests.dll.cecil`.

  * `--jvm PATH` is the path to the JVM library to load+use.

  * `-o DIR` is where to place output files; this will create
    `_x/Java.Interop.Export-Tests.dll`.

  * `-L DIR` adds `DIR` to library resolution paths; this adds
    `bin/TestDebug/net7.0` (dependencies of
    `Java.Interop.Export-Tests.dll`) and
    `Microsoft.NETCore.App/7.0.0-rc.1.22422.12` (net7 libs).

By default the directories containing input assemblies and the
directory containing `System.Private.CoreLib.dll` are part of the
default `-L` list.

When running in-tree, e.g. AzDO pipeline execution, `--jvm PATH`
will attempt to read the path in `bin/Build*/JdkInfo.props`
a'la `TestJVM` (002dea4).  This allows an in-place update in
`core-tests.yaml` which does:

	dotnet bin/Debug-net7.0/jnimarshalmethod-gen.dll \
	  bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll \
	  -v -v --keeptemp -o bin/TestDebug-net7.0

~~ Using `jnimarshalmethod-gen` output ~~

What does `jnimarshalmethod-gen` *do*?

	% ikdasm bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll > beg.il
	% ikdasm _x/Java.Interop.Export-Tests.dll > end.il
	% git diff --no-index beg.il end.il
	# https://gist.github.com/jonpryor/b8233444f2e51043732bea922f6afc81

is a ~1KB diff which shows, paraphrasing greatly:

	public partial class ExportTest {
	    partial class '__<$>_jni_marshal_methods' {
	        static IntPtr funcIJavaObject (IntPtr jnienv, IntPtr this) => …
	        // …
	        [JniAddNativeMethodRegistration]
	        static void __RegisterNativeMembers (JniNativeMethodRegistrationArguments args) => …
	    }
	}
	internal delegate long _JniMarshal_PP_L (IntPtr jnienv, IntPtr self);
	// …

wherein `ExportTest._<$>_jni_marshal_methods` and the `_JniMarshal*`
delegate types are added to the assembly.

This also unblocks the desire stated in 4787e01:

> For `Java.Base`, @jonpryor wants to support the custom marshaling
> infrastructure introduced in 77a6bf8.  This would allow types to
> participate in JNI marshal method ("connector method") generation
> *at runtime*, allowing specialization based on the current set of
> types and assemblies.

What can we do with this `jnimarshalmethod-gen` output?  Use it!

First, make sure the tests work:

	% dotnet test --logger "console;verbosity=detailed" bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll
	…
	Passed!  - Failed:     0, Passed:    17, Skipped:     0, Total:    17, Duration: 103 ms - Java.Interop.Export-Tests.dll (net7.0)

Then update/replace the unit test assembly with
`jnimarshalmethod-gen` output:

	% \cp _x/Java.Interop.Export-Tests.dll bin/TestDebug-net7.0
	% dotnet test --logger "console;verbosity=detailed" bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll
	…
	Total tests: 17
	     Passed: 17

`core-tests.yaml` has been updated to do this.

~~ One-Off Tests ~~

One-off tests: ensure that the generated assembly can be decompiled:

	% ikdasm  bin/TestDebug-net7.0/Java.Interop.Tools.Expressions-Tests-ExpressionAssemblyBuilderTests.dll
	% monodis bin/TestDebug-net7.0/Java.Interop.Tools.Expressions-Tests-ExpressionAssemblyBuilderTests.dll

	% ikdasm  _x/Java.Interop.Export-Tests.dll
	% monodis _x/Java.Interop.Export-Tests.dll
	# which currently fails :-()

Re-enable most of `Java.Interop.Export-Tests.dll` for .NET 7;
see 41ba348, which disabled those tests.

To verify the generated IL, use the [dotnet-ilverify][0] tool:

	dotnet tool install --global dotnet-ilverify

Usage of which is "weird":

	$HOME/.dotnet/tools/ilverify _x/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.0/*.dll'
	All Classes and Methods in /Volumes/Xamarin-Work/src/xamarin/Java.Interop/_x/Java.Interop.Export-Tests.dll Verified.
	# no errors!

where:

  * `--tokens`: Include metadata tokens in error messages.
  * `--system-module NAME`: set the "System module name".  Defaults
    to `mscorlib`, which is wrong for .NET 5+, so this must be set to
    `System.Private.CoreLib` (no `.dll` suffix!).
  * `-r FILE-GLOB`: Where to resolve assembly references for the
    input assembly.  Fortunately file globs are supported…

~~ Removing `System.Private.CoreLib` ~~

`System.Private.CoreLib.dll` is *private*; it's not part of the
public assembly surface area, so you can't use
`csc -r:System.Private.CoreLib …` and expect it to work.  This makes
things interesting because *at runtime* everything "important" is in
`System.Private.CoreLib.dll`, like `System.Object`.

Specifically, if we do the "obvious" thing and do:

	newTypeDefinition.BaseType = assemblyDefinition.MainModule
	    .ImportReference (typeof (object));

you're gonna have a bad type, because the resulting IL for
`newTypeDefinition` will have a base class of
`[System.Private.CoreLib]System.Object`, which isn't usable.

Fix this by:

 1. Writing the assembly to a `Stream`.
 2. Reading the `Stream` from (1)
 3. Fixing all member references and assembly references so that
    `System.Private.CoreLib` is not referenced.

If `jnimarshalmethod-gen.dll --keeptemp` is used, then a `.cecil`
file is created with the contents of (1).

Additionally, and unexpectedly -- [jbevain/cecil#895][1] -- Mono.Cecil
adds a reference to the assembly being modified.  Remove the declaring
assembly from `AssemblyReferences`.

[0]: https://www.nuget.org/packages/dotnet-ilverify
[1]: jbevain/cecil#895
jonpryor added a commit to jonpryor/java.interop that referenced this issue Feb 9, 2023
@jonpryor
[Java.Interop.Tools.Expressions] Add Java.Interop.Tools.Expressions
eca8388 
Fixes: dotnet#616

Context: dotnet#14
Context: ff4053c
Context: da5d1b8
Context: 4787e01
Context: 41ba348

Remember `jnimarshalmethod-gen` (176240d)?  And it's crazy idea to
use the `System.Linq.Expressions`-based custom marshaling
infrastructure (ff4053c, da5d1b8) to generate JNI marshal methods
at build/packaging time.  And how we had to back burner it because
it depended upon `System.Reflection.Emit` being able to write
assemblies to disk, which is a feature that never made it to
.NET Core, and is still lacking as of .NET 7?

Add `src/Java.Interop.Tools.Expressions`, which contains code which
uses Mono.Cecil to compile `Expression<T>` expressions to IL.

Then update `jnimarshalmethod-gen` to use it!

~~ Usage ~~

	% dotnet bin/Debug-net7.0/jnimarshalmethod-gen.dll \
	  bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll \
	  -v -v --keeptemp \
	  --jvm /Library/Java/JavaVirtualMachines/microsoft-11.jdk/Contents/Home/lib/jli/libjli.dylib \
	  -o _x \
	  -L bin/TestDebug-net7.0 \
	  -L /usr/local/share/dotnet/shared/Microsoft.NETCore.App/7.0.0

First param is assembly to process; `Java.Interop.Export-Tests.dll`
is handy because that's what the `run-test-jnimarshal` target in
`Makefile` processed.

  * `-v -v` is *really* verbose output

  * `--keeptemp` is keep temporary files, in this case
    `_x/Java.Interop.Export-Tests.dll.cecil`.

  * `--jvm PATH` is the path to the JVM library to load+use.

  * `-o DIR` is where to place output files; this will create
    `_x/Java.Interop.Export-Tests.dll`.

  * `-L DIR` adds `DIR` to library resolution paths; this adds
    `bin/TestDebug/net7.0` (dependencies of
    `Java.Interop.Export-Tests.dll`) and
    `Microsoft.NETCore.App/7.0.0-rc.1.22422.12` (net7 libs).

By default the directories containing input assemblies and the
directory containing `System.Private.CoreLib.dll` are part of the
default `-L` list.

When running in-tree, e.g. AzDO pipeline execution, `--jvm PATH`
will attempt to read the path in `bin/Build*/JdkInfo.props`
a'la `TestJVM` (002dea4).  This allows an in-place update in
`core-tests.yaml` which does:

	dotnet bin/Debug-net7.0/jnimarshalmethod-gen.dll \
	  bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll \
	  -v -v --keeptemp -o bin/TestDebug-net7.0

~~ Using `jnimarshalmethod-gen` output ~~

What does `jnimarshalmethod-gen` *do*?

	% ikdasm bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll > beg.il
	% ikdasm _x/Java.Interop.Export-Tests.dll > end.il
	% git diff --no-index beg.il end.il
	# https://gist.github.com/jonpryor/b8233444f2e51043732bea922f6afc81

is a ~1KB diff which shows, paraphrasing greatly:

	public partial class ExportTest {
	    partial class '__<$>_jni_marshal_methods' {
	        static IntPtr funcIJavaObject (IntPtr jnienv, IntPtr this) => …
	        // …
	        [JniAddNativeMethodRegistration]
	        static void __RegisterNativeMembers (JniNativeMethodRegistrationArguments args) => …
	    }
	}
	internal delegate long _JniMarshal_PP_L (IntPtr jnienv, IntPtr self);
	// …

wherein `ExportTest._<$>_jni_marshal_methods` and the `_JniMarshal*`
delegate types are added to the assembly.

This also unblocks the desire stated in 4787e01:

> For `Java.Base`, @jonpryor wants to support the custom marshaling
> infrastructure introduced in 77a6bf8.  This would allow types to
> participate in JNI marshal method ("connector method") generation
> *at runtime*, allowing specialization based on the current set of
> types and assemblies.

What can we do with this `jnimarshalmethod-gen` output?  Use it!

First, make sure the tests work:

	% dotnet test --logger "console;verbosity=detailed" bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll
	…
	Passed!  - Failed:     0, Passed:    17, Skipped:     0, Total:    17, Duration: 103 ms - Java.Interop.Export-Tests.dll (net7.0)

Then update/replace the unit test assembly with
`jnimarshalmethod-gen` output:

	% \cp _x/Java.Interop.Export-Tests.dll bin/TestDebug-net7.0
	% dotnet test --logger "console;verbosity=detailed" bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll
	…
	Total tests: 17
	     Passed: 17

`core-tests.yaml` has been updated to do this.

~~ One-Off Tests ~~

One-off tests: ensure that the generated assembly can be decompiled:

	% ikdasm  bin/TestDebug-net7.0/Java.Interop.Tools.Expressions-Tests-ExpressionAssemblyBuilderTests.dll
	% monodis bin/TestDebug-net7.0/Java.Interop.Tools.Expressions-Tests-ExpressionAssemblyBuilderTests.dll

	% ikdasm  _x/Java.Interop.Export-Tests.dll
	% monodis _x/Java.Interop.Export-Tests.dll
	# which currently fails :-()

Re-enable most of `Java.Interop.Export-Tests.dll` for .NET 7;
see 41ba348, which disabled those tests.

To verify the generated IL, use the [dotnet-ilverify][0] tool:

	dotnet tool install --global dotnet-ilverify

Usage of which is "weird":

	$HOME/.dotnet/tools/ilverify _x/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.0/*.dll'
	All Classes and Methods in /Volumes/Xamarin-Work/src/xamarin/Java.Interop/_x/Java.Interop.Export-Tests.dll Verified.
	# no errors!

where:

  * `--tokens`: Include metadata tokens in error messages.
  * `--system-module NAME`: set the "System module name".  Defaults
    to `mscorlib`, which is wrong for .NET 5+, so this must be set to
    `System.Private.CoreLib` (no `.dll` suffix!).
  * `-r FILE-GLOB`: Where to resolve assembly references for the
    input assembly.  Fortunately file globs are supported…

~~ Removing `System.Private.CoreLib` ~~

`System.Private.CoreLib.dll` is *private*; it's not part of the
public assembly surface area, so you can't use
`csc -r:System.Private.CoreLib …` and expect it to work.  This makes
things interesting because *at runtime* everything "important" is in
`System.Private.CoreLib.dll`, like `System.Object`.

Specifically, if we do the "obvious" thing and do:

	newTypeDefinition.BaseType = assemblyDefinition.MainModule
	    .ImportReference (typeof (object));

you're gonna have a bad type, because the resulting IL for
`newTypeDefinition` will have a base class of
`[System.Private.CoreLib]System.Object`, which isn't usable.

Fix this by:

 1. Writing the assembly to a `Stream`.
 2. Reading the `Stream` from (1)
 3. Fixing all member references and assembly references so that
    `System.Private.CoreLib` is not referenced.

If `jnimarshalmethod-gen.dll --keeptemp` is used, then a `.cecil`
file is created with the contents of (1).

Additionally, and unexpectedly -- [jbevain/cecil#895][1] -- Mono.Cecil
adds a reference to the assembly being modified.  Remove the declaring
assembly from `AssemblyReferences`.

[0]: https://www.nuget.org/packages/dotnet-ilverify
[1]: jbevain/cecil#895
jonpryor added a commit to jonpryor/java.interop that referenced this issue Feb 10, 2023
@jonpryor
[Java.Interop.Tools.Expressions] Add Java.Interop.Tools.Expressions
20fa0aa 
Fixes: dotnet#616

Context: dotnet#14
Context: ff4053c
Context: da5d1b8
Context: 4787e01
Context: 41ba348

Remember `jnimarshalmethod-gen` (176240d)?  And it's crazy idea to
use the `System.Linq.Expressions`-based custom marshaling
infrastructure (ff4053c, da5d1b8) to generate JNI marshal methods
at build/packaging time.  And how we had to back burner it because
it depended upon `System.Reflection.Emit` being able to write
assemblies to disk, which is a feature that never made it to
.NET Core, and is still lacking as of .NET 7?

Add `src/Java.Interop.Tools.Expressions`, which contains code which
uses Mono.Cecil to compile `Expression<T>` expressions to IL.

Then update `jnimarshalmethod-gen` to use it!

~~ Usage ~~

	% dotnet bin/Debug-net7.0/jnimarshalmethod-gen.dll \
	  bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll \
	  -v -v --keeptemp \
	  --jvm /Library/Java/JavaVirtualMachines/microsoft-11.jdk/Contents/Home/lib/jli/libjli.dylib \
	  -o _x \
	  -L bin/TestDebug-net7.0 \
	  -L /usr/local/share/dotnet/shared/Microsoft.NETCore.App/7.0.0

First param is assembly to process; `Java.Interop.Export-Tests.dll`
is handy because that's what the `run-test-jnimarshal` target in
`Makefile` processed.

  * `-v -v` is *really* verbose output

  * `--keeptemp` is keep temporary files, in this case
    `_x/Java.Interop.Export-Tests.dll.cecil`.

  * `--jvm PATH` is the path to the JVM library to load+use.

  * `-o DIR` is where to place output files; this will create
    `_x/Java.Interop.Export-Tests.dll`.

  * `-L DIR` adds `DIR` to library resolution paths; this adds
    `bin/TestDebug/net7.0` (dependencies of
    `Java.Interop.Export-Tests.dll`) and
    `Microsoft.NETCore.App/7.0.0-rc.1.22422.12` (net7 libs).

By default the directories containing input assemblies and the
directory containing `System.Private.CoreLib.dll` are part of the
default `-L` list.

When running in-tree, e.g. AzDO pipeline execution, `--jvm PATH`
will attempt to read the path in `bin/Build*/JdkInfo.props`
a'la `TestJVM` (002dea4).  This allows an in-place update in
`core-tests.yaml` which does:

	dotnet bin/Debug-net7.0/jnimarshalmethod-gen.dll \
	  bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll \
	  -v -v --keeptemp -o bin/TestDebug-net7.0

~~ Using `jnimarshalmethod-gen` output ~~

What does `jnimarshalmethod-gen` *do*?

	% ikdasm bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll > beg.il
	% ikdasm _x/Java.Interop.Export-Tests.dll > end.il
	% git diff --no-index beg.il end.il
	# https://gist.github.com/jonpryor/b8233444f2e51043732bea922f6afc81

is a ~1KB diff which shows, paraphrasing greatly:

	public partial class ExportTest {
	    partial class '__<$>_jni_marshal_methods' {
	        static IntPtr funcIJavaObject (IntPtr jnienv, IntPtr this) => …
	        // …
	        [JniAddNativeMethodRegistration]
	        static void __RegisterNativeMembers (JniNativeMethodRegistrationArguments args) => …
	    }
	}
	internal delegate long _JniMarshal_PP_L (IntPtr jnienv, IntPtr self);
	// …

wherein `ExportTest._<$>_jni_marshal_methods` and the `_JniMarshal*`
delegate types are added to the assembly.

This also unblocks the desire stated in 4787e01:

> For `Java.Base`, @jonpryor wants to support the custom marshaling
> infrastructure introduced in 77a6bf8.  This would allow types to
> participate in JNI marshal method ("connector method") generation
> *at runtime*, allowing specialization based on the current set of
> types and assemblies.

What can we do with this `jnimarshalmethod-gen` output?  Use it!

First, make sure the tests work:

	% dotnet test --logger "console;verbosity=detailed" bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll
	…
	Passed!  - Failed:     0, Passed:    17, Skipped:     0, Total:    17, Duration: 103 ms - Java.Interop.Export-Tests.dll (net7.0)

Then update/replace the unit test assembly with
`jnimarshalmethod-gen` output:

	% \cp _x/Java.Interop.Export-Tests.dll bin/TestDebug-net7.0
	% dotnet test --logger "console;verbosity=detailed" bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll
	…
	Total tests: 17
	     Passed: 17

`core-tests.yaml` has been updated to do this.

~~ One-Off Tests ~~

One-off tests: ensure that the generated assembly can be decompiled:

	% ikdasm  bin/TestDebug-net7.0/Java.Interop.Tools.Expressions-Tests-ExpressionAssemblyBuilderTests.dll
	% monodis bin/TestDebug-net7.0/Java.Interop.Tools.Expressions-Tests-ExpressionAssemblyBuilderTests.dll

	% ikdasm  _x/Java.Interop.Export-Tests.dll
	% monodis _x/Java.Interop.Export-Tests.dll
	# which currently fails :-()

Re-enable most of `Java.Interop.Export-Tests.dll` for .NET 7;
see 41ba348, which disabled those tests.

To verify the generated IL, use the [dotnet-ilverify][0] tool:

	dotnet tool install --global dotnet-ilverify

Usage of which is "weird":

	$HOME/.dotnet/tools/ilverify _x/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.0/*.dll'
	All Classes and Methods in /Volumes/Xamarin-Work/src/xamarin/Java.Interop/_x/Java.Interop.Export-Tests.dll Verified.
	# no errors!

where:

  * `--tokens`: Include metadata tokens in error messages.
  * `--system-module NAME`: set the "System module name".  Defaults
    to `mscorlib`, which is wrong for .NET 5+, so this must be set to
    `System.Private.CoreLib` (no `.dll` suffix!).
  * `-r FILE-GLOB`: Where to resolve assembly references for the
    input assembly.  Fortunately file globs are supported…

~~ Removing `System.Private.CoreLib` ~~

`System.Private.CoreLib.dll` is *private*; it's not part of the
public assembly surface area, so you can't use
`csc -r:System.Private.CoreLib …` and expect it to work.  This makes
things interesting because *at runtime* everything "important" is in
`System.Private.CoreLib.dll`, like `System.Object`.

Specifically, if we do the "obvious" thing and do:

	newTypeDefinition.BaseType = assemblyDefinition.MainModule
	    .ImportReference (typeof (object));

you're gonna have a bad type, because the resulting IL for
`newTypeDefinition` will have a base class of
`[System.Private.CoreLib]System.Object`, which isn't usable.

Fix this by:

 1. Writing the assembly to a `Stream`.
 2. Reading the `Stream` from (1)
 3. Fixing all member references and assembly references so that
    `System.Private.CoreLib` is not referenced.

If `jnimarshalmethod-gen.dll --keeptemp` is used, then a `.cecil`
file is created with the contents of (1).

Additionally, and unexpectedly -- [jbevain/cecil#895][1] -- Mono.Cecil
adds a reference to the assembly being modified.  Remove the declaring
assembly from `AssemblyReferences`.

[0]: https://www.nuget.org/packages/dotnet-ilverify
[1]: jbevain/cecil#895
jonpryor added a commit to jonpryor/java.interop that referenced this issue Feb 10, 2023
@jonpryor
[Java.Interop.Tools.Expressions] Add Java.Interop.Tools.Expressions
d176b58 
Fixes: dotnet#616

Context: dotnet#14
Context: ff4053c
Context: da5d1b8
Context: 4787e01
Context: 41ba348

Remember `jnimarshalmethod-gen` (176240d)?  And it's crazy idea to
use the `System.Linq.Expressions`-based custom marshaling
infrastructure (ff4053c, da5d1b8) to generate JNI marshal methods
at build/packaging time.  And how we had to back burner it because
it depended upon `System.Reflection.Emit` being able to write
assemblies to disk, which is a feature that never made it to
.NET Core, and is still lacking as of .NET 7?

Add `src/Java.Interop.Tools.Expressions`, which contains code which
uses Mono.Cecil to compile `Expression<T>` expressions to IL.

Then update `jnimarshalmethod-gen` to use it!

~~ Usage ~~

	% dotnet bin/Debug-net7.0/jnimarshalmethod-gen.dll \
	  bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll \
	  -v -v --keeptemp \
	  --jvm /Library/Java/JavaVirtualMachines/microsoft-11.jdk/Contents/Home/lib/jli/libjli.dylib \
	  -o _x \
	  -L bin/TestDebug-net7.0 \
	  -L /usr/local/share/dotnet/shared/Microsoft.NETCore.App/7.0.0

First param is assembly to process; `Java.Interop.Export-Tests.dll`
is handy because that's what the `run-test-jnimarshal` target in
`Makefile` processed.

  * `-v -v` is *really* verbose output

  * `--keeptemp` is keep temporary files, in this case
    `_x/Java.Interop.Export-Tests.dll.cecil`.

  * `--jvm PATH` is the path to the JVM library to load+use.

  * `-o DIR` is where to place output files; this will create
    `_x/Java.Interop.Export-Tests.dll`.

  * `-L DIR` adds `DIR` to library resolution paths; this adds
    `bin/TestDebug/net7.0` (dependencies of
    `Java.Interop.Export-Tests.dll`) and
    `Microsoft.NETCore.App/7.0.0-rc.1.22422.12` (net7 libs).

By default the directories containing input assemblies and the
directory containing `System.Private.CoreLib.dll` are part of the
default `-L` list.

When running in-tree, e.g. AzDO pipeline execution, `--jvm PATH`
will attempt to read the path in `bin/Build*/JdkInfo.props`
a'la `TestJVM` (002dea4).  This allows an in-place update in
`core-tests.yaml` which does:

	dotnet bin/Debug-net7.0/jnimarshalmethod-gen.dll \
	  bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll \
	  -v -v --keeptemp -o bin/TestDebug-net7.0

~~ Using `jnimarshalmethod-gen` output ~~

What does `jnimarshalmethod-gen` *do*?

	% ikdasm bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll > beg.il
	% ikdasm _x/Java.Interop.Export-Tests.dll > end.il
	% git diff --no-index beg.il end.il
	# https://gist.github.com/jonpryor/b8233444f2e51043732bea922f6afc81

is a ~1KB diff which shows, paraphrasing greatly:

	public partial class ExportTest {
	    partial class '__<$>_jni_marshal_methods' {
	        static IntPtr funcIJavaObject (IntPtr jnienv, IntPtr this) => …
	        // …
	        [JniAddNativeMethodRegistration]
	        static void __RegisterNativeMembers (JniNativeMethodRegistrationArguments args) => …
	    }
	}
	internal delegate long _JniMarshal_PP_L (IntPtr jnienv, IntPtr self);
	// …

wherein `ExportTest._<$>_jni_marshal_methods` and the `_JniMarshal*`
delegate types are added to the assembly.

This also unblocks the desire stated in 4787e01:

> For `Java.Base`, @jonpryor wants to support the custom marshaling
> infrastructure introduced in 77a6bf8.  This would allow types to
> participate in JNI marshal method ("connector method") generation
> *at runtime*, allowing specialization based on the current set of
> types and assemblies.

What can we do with this `jnimarshalmethod-gen` output?  Use it!

First, make sure the tests work:

	% dotnet test --logger "console;verbosity=detailed" bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll
	…
	Passed!  - Failed:     0, Passed:    17, Skipped:     0, Total:    17, Duration: 103 ms - Java.Interop.Export-Tests.dll (net7.0)

Then update/replace the unit test assembly with
`jnimarshalmethod-gen` output:

	% \cp _x/Java.Interop.Export-Tests.dll bin/TestDebug-net7.0
	% dotnet test --logger "console;verbosity=detailed" bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll
	…
	Total tests: 17
	     Passed: 17

`core-tests.yaml` has been updated to do this.

~~ One-Off Tests ~~

One-off tests: ensure that the generated assembly can be decompiled:

	% ikdasm  bin/TestDebug-net7.0/Java.Interop.Tools.Expressions-Tests-ExpressionAssemblyBuilderTests.dll
	% monodis bin/TestDebug-net7.0/Java.Interop.Tools.Expressions-Tests-ExpressionAssemblyBuilderTests.dll

	% ikdasm  _x/Java.Interop.Export-Tests.dll
	% monodis _x/Java.Interop.Export-Tests.dll
	# which currently fails :-()

Re-enable most of `Java.Interop.Export-Tests.dll` for .NET 7;
see 41ba348, which disabled those tests.

To verify the generated IL, use the [dotnet-ilverify][0] tool:

	dotnet tool install --global dotnet-ilverify

Usage of which is "weird":

	$HOME/.dotnet/tools/ilverify _x/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.0/*.dll'
	All Classes and Methods in /Volumes/Xamarin-Work/src/xamarin/Java.Interop/_x/Java.Interop.Export-Tests.dll Verified.
	# no errors!

where:

  * `--tokens`: Include metadata tokens in error messages.
  * `--system-module NAME`: set the "System module name".  Defaults
    to `mscorlib`, which is wrong for .NET 5+, so this must be set to
    `System.Private.CoreLib` (no `.dll` suffix!).
  * `-r FILE-GLOB`: Where to resolve assembly references for the
    input assembly.  Fortunately file globs are supported…

~~ Removing `System.Private.CoreLib` ~~

`System.Private.CoreLib.dll` is *private*; it's not part of the
public assembly surface area, so you can't use
`csc -r:System.Private.CoreLib …` and expect it to work.  This makes
things interesting because *at runtime* everything "important" is in
`System.Private.CoreLib.dll`, like `System.Object`.

Specifically, if we do the "obvious" thing and do:

	newTypeDefinition.BaseType = assemblyDefinition.MainModule
	    .ImportReference (typeof (object));

you're gonna have a bad type, because the resulting IL for
`newTypeDefinition` will have a base class of
`[System.Private.CoreLib]System.Object`, which isn't usable.

Fix this by:

 1. Writing the assembly to a `Stream`.
 2. Reading the `Stream` from (1)
 3. Fixing all member references and assembly references so that
    `System.Private.CoreLib` is not referenced.

If `jnimarshalmethod-gen.dll --keeptemp` is used, then a `.cecil`
file is created with the contents of (1).

Additionally, and unexpectedly -- [jbevain/cecil#895][1] -- Mono.Cecil
adds a reference to the assembly being modified.  Remove the declaring
assembly from `AssemblyReferences`.

[0]: https://www.nuget.org/packages/dotnet-ilverify
[1]: jbevain/cecil#895
jonpryor added a commit to jonpryor/java.interop that referenced this issue Feb 15, 2023
@jonpryor
[Java.Interop.Tools.Expressions] Add Java.Interop.Tools.Expressions
32e7f55 
Fixes: dotnet#616

Context: dotnet#14
Context: ff4053c
Context: da5d1b8
Context: 4787e01
Context: 41ba348

Remember `jnimarshalmethod-gen` (176240d)?  And it's crazy idea to
use the `System.Linq.Expressions`-based custom marshaling
infrastructure (ff4053c, da5d1b8) to generate JNI marshal methods
at build/packaging time.  And how we had to back burner it because
it depended upon `System.Reflection.Emit` being able to write
assemblies to disk, which is a feature that never made it to
.NET Core, and is still lacking as of .NET 7?

Add `src/Java.Interop.Tools.Expressions`, which contains code which
uses Mono.Cecil to compile `Expression<T>` expressions to IL.

Then update `jnimarshalmethod-gen` to use it!

~~ Usage ~~

	% dotnet bin/Debug-net7.0/jnimarshalmethod-gen.dll \
	  bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll \
	  -v -v --keeptemp \
	  --jvm /Library/Java/JavaVirtualMachines/microsoft-11.jdk/Contents/Home/lib/jli/libjli.dylib \
	  -o _x \
	  -L bin/TestDebug-net7.0 \
	  -L /usr/local/share/dotnet/shared/Microsoft.NETCore.App/7.0.0

First param is assembly to process; `Java.Interop.Export-Tests.dll`
is handy because that's what the `run-test-jnimarshal` target in
`Makefile` processed.

  * `-v -v` is *really* verbose output

  * `--keeptemp` is keep temporary files, in this case
    `_x/Java.Interop.Export-Tests.dll.cecil`.

  * `--jvm PATH` is the path to the JVM library to load+use.

  * `-o DIR` is where to place output files; this will create
    `_x/Java.Interop.Export-Tests.dll`.

  * `-L DIR` adds `DIR` to library resolution paths; this adds
    `bin/TestDebug/net7.0` (dependencies of
    `Java.Interop.Export-Tests.dll`) and
    `Microsoft.NETCore.App/7.0.0-rc.1.22422.12` (net7 libs).

By default the directories containing input assemblies and the
directory containing `System.Private.CoreLib.dll` are part of the
default `-L` list.

When running in-tree, e.g. AzDO pipeline execution, `--jvm PATH`
will attempt to read the path in `bin/Build*/JdkInfo.props`
a'la `TestJVM` (002dea4).  This allows an in-place update in
`core-tests.yaml` which does:

	dotnet bin/Debug-net7.0/jnimarshalmethod-gen.dll \
	  bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll \
	  -v -v --keeptemp -o bin/TestDebug-net7.0

~~ Using `jnimarshalmethod-gen` output ~~

What does `jnimarshalmethod-gen` *do*?

	% ikdasm bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll > beg.il
	% ikdasm _x/Java.Interop.Export-Tests.dll > end.il
	% git diff --no-index beg.il end.il
	# https://gist.github.com/jonpryor/b8233444f2e51043732bea922f6afc81

is a ~1KB diff which shows, paraphrasing greatly:

	public partial class ExportTest {
	    partial class '__<$>_jni_marshal_methods' {
	        static IntPtr funcIJavaObject (IntPtr jnienv, IntPtr this) => …
	        // …
	        [JniAddNativeMethodRegistration]
	        static void __RegisterNativeMembers (JniNativeMethodRegistrationArguments args) => …
	    }
	}
	internal delegate long _JniMarshal_PP_L (IntPtr jnienv, IntPtr self);
	// …

wherein `ExportTest._<$>_jni_marshal_methods` and the `_JniMarshal*`
delegate types are added to the assembly.

This also unblocks the desire stated in 4787e01:

> For `Java.Base`, @jonpryor wants to support the custom marshaling
> infrastructure introduced in 77a6bf8.  This would allow types to
> participate in JNI marshal method ("connector method") generation
> *at runtime*, allowing specialization based on the current set of
> types and assemblies.

What can we do with this `jnimarshalmethod-gen` output?  Use it!

First, make sure the tests work:

	% dotnet test --logger "console;verbosity=detailed" bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll
	…
	Passed!  - Failed:     0, Passed:    17, Skipped:     0, Total:    17, Duration: 103 ms - Java.Interop.Export-Tests.dll (net7.0)

Then update/replace the unit test assembly with
`jnimarshalmethod-gen` output:

	% \cp _x/Java.Interop.Export-Tests.dll bin/TestDebug-net7.0
	% dotnet test --logger "console;verbosity=detailed" bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll
	…
	Total tests: 17
	     Passed: 17

`core-tests.yaml` has been updated to do this.

~~ One-Off Tests ~~

One-off tests: ensure that the generated assembly can be decompiled:

	% ikdasm  bin/TestDebug-net7.0/Java.Interop.Tools.Expressions-Tests-ExpressionAssemblyBuilderTests.dll
	% monodis bin/TestDebug-net7.0/Java.Interop.Tools.Expressions-Tests-ExpressionAssemblyBuilderTests.dll

	% ikdasm  _x/Java.Interop.Export-Tests.dll
	% monodis _x/Java.Interop.Export-Tests.dll
	# which currently fails :-()

Re-enable most of `Java.Interop.Export-Tests.dll` for .NET 7;
see 41ba348, which disabled those tests.

To verify the generated IL, use the [dotnet-ilverify][0] tool:

	dotnet tool install --global dotnet-ilverify

Usage of which is "weird":

	$HOME/.dotnet/tools/ilverify _x/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.0/*.dll'
	All Classes and Methods in /Volumes/Xamarin-Work/src/xamarin/Java.Interop/_x/Java.Interop.Export-Tests.dll Verified.
	# no errors!

where:

  * `--tokens`: Include metadata tokens in error messages.
  * `--system-module NAME`: set the "System module name".  Defaults
    to `mscorlib`, which is wrong for .NET 5+, so this must be set to
    `System.Private.CoreLib` (no `.dll` suffix!).
  * `-r FILE-GLOB`: Where to resolve assembly references for the
    input assembly.  Fortunately file globs are supported…

~~ Removing `System.Private.CoreLib` ~~

`System.Private.CoreLib.dll` is *private*; it's not part of the
public assembly surface area, so you can't use
`csc -r:System.Private.CoreLib …` and expect it to work.  This makes
things interesting because *at runtime* everything "important" is in
`System.Private.CoreLib.dll`, like `System.Object`.

Specifically, if we do the "obvious" thing and do:

	newTypeDefinition.BaseType = assemblyDefinition.MainModule
	    .ImportReference (typeof (object));

you're gonna have a bad type, because the resulting IL for
`newTypeDefinition` will have a base class of
`[System.Private.CoreLib]System.Object`, which isn't usable.

Fix this by:

 1. Writing the assembly to a `Stream`.
 2. Reading the `Stream` from (1)
 3. Fixing all member references and assembly references so that
    `System.Private.CoreLib` is not referenced.

If `jnimarshalmethod-gen.dll --keeptemp` is used, then a `.cecil`
file is created with the contents of (1).

Additionally, and unexpectedly -- [jbevain/cecil#895][1] -- Mono.Cecil
adds a reference to the assembly being modified.  Remove the declaring
assembly from `AssemblyReferences`.

[0]: https://www.nuget.org/packages/dotnet-ilverify
[1]: jbevain/cecil#895
jonpryor added a commit to jonpryor/java.interop that referenced this issue Feb 23, 2023
@jonpryor
[Java.Interop.Tools.Expressions] Add Java.Interop.Tools.Expressions
6247614 
Fixes: dotnet#616

Context: dotnet#14
Context: ff4053c
Context: da5d1b8
Context: 4787e01
Context: 41ba348

Remember `jnimarshalmethod-gen` (176240d)?  And it's crazy idea to
use the `System.Linq.Expressions`-based custom marshaling
infrastructure (ff4053c, da5d1b8) to generate JNI marshal methods
at build/packaging time.  And how we had to back burner it because
it depended upon `System.Reflection.Emit` being able to write
assemblies to disk, which is a feature that never made it to
.NET Core, and is still lacking as of .NET 7?

Add `src/Java.Interop.Tools.Expressions`, which contains code which
uses Mono.Cecil to compile `Expression<T>` expressions to IL.

Then update `jnimarshalmethod-gen` to use it!

~~ Usage ~~

	% dotnet bin/Debug-net7.0/jnimarshalmethod-gen.dll \
	  bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll \
	  -v -v --keeptemp \
	  --jvm /Library/Java/JavaVirtualMachines/microsoft-11.jdk/Contents/Home/lib/jli/libjli.dylib \
	  -o _x \
	  -L bin/TestDebug-net7.0 \
	  -L /usr/local/share/dotnet/shared/Microsoft.NETCore.App/7.0.0

First param is assembly to process; `Java.Interop.Export-Tests.dll`
is handy because that's what the `run-test-jnimarshal` target in
`Makefile` processed.

  * `-v -v` is *really* verbose output

  * `--keeptemp` is keep temporary files, in this case
    `_x/Java.Interop.Export-Tests.dll.cecil`.

  * `--jvm PATH` is the path to the JVM library to load+use.

  * `-o DIR` is where to place output files; this will create
    `_x/Java.Interop.Export-Tests.dll`.

  * `-L DIR` adds `DIR` to library resolution paths; this adds
    `bin/TestDebug/net7.0` (dependencies of
    `Java.Interop.Export-Tests.dll`) and
    `Microsoft.NETCore.App/7.0.0-rc.1.22422.12` (net7 libs).

By default the directories containing input assemblies and the
directory containing `System.Private.CoreLib.dll` are part of the
default `-L` list.

When running in-tree, e.g. AzDO pipeline execution, `--jvm PATH`
will attempt to read the path in `bin/Build*/JdkInfo.props`
a'la `TestJVM` (002dea4).  This allows an in-place update in
`core-tests.yaml` which does:

	dotnet bin/Debug-net7.0/jnimarshalmethod-gen.dll \
	  bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll \
	  -v -v --keeptemp -o bin/TestDebug-net7.0

~~ Using `jnimarshalmethod-gen` output ~~

What does `jnimarshalmethod-gen` *do*?

	% ikdasm bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll > beg.il
	% ikdasm _x/Java.Interop.Export-Tests.dll > end.il
	% git diff --no-index beg.il end.il
	# https://gist.github.com/jonpryor/b8233444f2e51043732bea922f6afc81

is a ~1KB diff which shows, paraphrasing greatly:

	public partial class ExportTest {
	    partial class '__<$>_jni_marshal_methods' {
	        static IntPtr funcIJavaObject (IntPtr jnienv, IntPtr this) => …
	        // …
	        [JniAddNativeMethodRegistration]
	        static void __RegisterNativeMembers (JniNativeMethodRegistrationArguments args) => …
	    }
	}
	internal delegate long _JniMarshal_PP_L (IntPtr jnienv, IntPtr self);
	// …

wherein `ExportTest._<$>_jni_marshal_methods` and the `_JniMarshal*`
delegate types are added to the assembly.

This also unblocks the desire stated in 4787e01:

> For `Java.Base`, @jonpryor wants to support the custom marshaling
> infrastructure introduced in 77a6bf8.  This would allow types to
> participate in JNI marshal method ("connector method") generation
> *at runtime*, allowing specialization based on the current set of
> types and assemblies.

What can we do with this `jnimarshalmethod-gen` output?  Use it!

First, make sure the tests work:

	% dotnet test --logger "console;verbosity=detailed" bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll
	…
	Passed!  - Failed:     0, Passed:    17, Skipped:     0, Total:    17, Duration: 103 ms - Java.Interop.Export-Tests.dll (net7.0)

Then update/replace the unit test assembly with
`jnimarshalmethod-gen` output:

	% \cp _x/Java.Interop.Export-Tests.dll bin/TestDebug-net7.0
	% dotnet test --logger "console;verbosity=detailed" bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll
	…
	Total tests: 17
	     Passed: 17

`core-tests.yaml` has been updated to do this.

~~ One-Off Tests ~~

One-off tests: ensure that the generated assembly can be decompiled:

	% ikdasm  bin/TestDebug-net7.0/Java.Interop.Tools.Expressions-Tests-ExpressionAssemblyBuilderTests.dll
	% monodis bin/TestDebug-net7.0/Java.Interop.Tools.Expressions-Tests-ExpressionAssemblyBuilderTests.dll

	% ikdasm  _x/Java.Interop.Export-Tests.dll
	% monodis _x/Java.Interop.Export-Tests.dll
	# which currently fails :-()

Re-enable most of `Java.Interop.Export-Tests.dll` for .NET 7;
see 41ba348, which disabled those tests.

To verify the generated IL, use the [dotnet-ilverify][0] tool:

	dotnet tool install --global dotnet-ilverify

Usage of which is "weird":

	$HOME/.dotnet/tools/ilverify _x/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.0/*.dll'
	All Classes and Methods in /Volumes/Xamarin-Work/src/xamarin/Java.Interop/_x/Java.Interop.Export-Tests.dll Verified.
	# no errors!

where:

  * `--tokens`: Include metadata tokens in error messages.
  * `--system-module NAME`: set the "System module name".  Defaults
    to `mscorlib`, which is wrong for .NET 5+, so this must be set to
    `System.Private.CoreLib` (no `.dll` suffix!).
  * `-r FILE-GLOB`: Where to resolve assembly references for the
    input assembly.  Fortunately file globs are supported…

~~ Removing `System.Private.CoreLib` ~~

`System.Private.CoreLib.dll` is *private*; it's not part of the
public assembly surface area, so you can't use
`csc -r:System.Private.CoreLib …` and expect it to work.  This makes
things interesting because *at runtime* everything "important" is in
`System.Private.CoreLib.dll`, like `System.Object`.

Specifically, if we do the "obvious" thing and do:

	newTypeDefinition.BaseType = assemblyDefinition.MainModule
	    .ImportReference (typeof (object));

you're gonna have a bad type, because the resulting IL for
`newTypeDefinition` will have a base class of
`[System.Private.CoreLib]System.Object`, which isn't usable.

Fix this by:

 1. Writing the assembly to a `Stream`.
 2. Reading the `Stream` from (1)
 3. Fixing all member references and assembly references so that
    `System.Private.CoreLib` is not referenced.

If `jnimarshalmethod-gen.dll --keeptemp` is used, then a `.cecil`
file is created with the contents of (1).

Additionally, and unexpectedly -- [jbevain/cecil#895][1] -- Mono.Cecil
adds a reference to the assembly being modified.  Remove the declaring
assembly from `AssemblyReferences`.

[0]: https://www.nuget.org/packages/dotnet-ilverify
[1]: jbevain/cecil#895
jonpryor added a commit that referenced this issue Feb 28, 2023
@jonpryor
[Java.Interop.Tools.Expressions] Add Java.Interop.Tools.Expressions (#…
77800dd 
…1046)

Fixes: #616

Context: #14
Context: ff4053c
Context: da5d1b8
Context: 4787e01
Context: 41ba348

Remember `jnimarshalmethod-gen` (176240d)?  And it's crazy idea to
use the `System.Linq.Expressions`-based custom marshaling
infrastructure (ff4053c, da5d1b8) to generate JNI marshal methods
at build/packaging time.  And how we had to back burner it because
it depended upon `System.Reflection.Emit` being able to write
assemblies to disk, which is a feature that never made it to .NET Core,
and is still lacking as of .NET 7 (#616)?

Add `src/Java.Interop.Tools.Expressions`, which contains code which
uses Mono.Cecil to compile `Expression<T>` expressions to IL.

Then update `jnimarshalmethod-gen` to use it!

~~ Usage ~~

	% dotnet bin/Debug-net7.0/jnimarshalmethod-gen.dll \
	  bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll \
	  -v -v --keeptemp \
	  --jvm /Library/Java/JavaVirtualMachines/microsoft-11.jdk/Contents/Home/lib/jli/libjli.dylib \
	  -o _x \
	  -L bin/TestDebug-net7.0 \
	  -L /usr/local/share/dotnet/shared/Microsoft.NETCore.App/7.0.0

First param is assembly to process; `Java.Interop.Export-Tests.dll`
is handy because that's what the `run-test-jnimarshal` target in
`Makefile` processed.

  * `-v -v` is *really* verbose output

  * `--keeptemp` is keep temporary files, in this case
    `_x/Java.Interop.Export-Tests.dll.cecil`.

  * `--jvm PATH` is the path to the JVM library to load+use.

  * `-o DIR` is where to place output files; this will create
    `_x/Java.Interop.Export-Tests.dll`.

  * `-L DIR` adds `DIR` to library resolution paths; this adds
    `bin/TestDebug/net7.0` (dependencies of
    `Java.Interop.Export-Tests.dll`) and
    `Microsoft.NETCore.App/7.0.0-rc.1.22422.12` (net7 libs).

By default the directories containing input assemblies and the
directory containing `System.Private.CoreLib.dll` are part of the
default `-L` list.

When running in-tree, e.g. AzDO pipeline execution, when
`--jvm PATH` isn't specified, `jnimarshalmethod-gen`
will attempt to read the path in `bin/Build*/JdkInfo.props`
a'la `TestJVM` (002dea4).  This allows an in-place update in
`core-tests.yaml` which does:

	dotnet bin/Debug-net7.0/jnimarshalmethod-gen.dll \
	  bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll \
	  -v -v --keeptemp -o bin/TestDebug-net7.0

~~ Using `jnimarshalmethod-gen` output ~~

What does `jnimarshalmethod-gen` *do*?

	% ikdasm bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll.orig > beg.il
	% ikdasm bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll      > end.il
	% git diff --no-index beg.il end.il
	# https://gist.github.com/jonpryor/b8233444f2e51043732bea922f6afc81

is a ~1KB diff which shows, paraphrasing greatly:

	public partial class ExportTest {
	    partial class '__<$>_jni_marshal_methods' {
	        static IntPtr funcIJavaObject (IntPtr jnienv, IntPtr this) => …
	        // …
	        [JniAddNativeMethodRegistration]
	        static void __RegisterNativeMembers (JniNativeMethodRegistrationArguments args) => …
	    }
	}
	internal delegate long _JniMarshal_PP_L (IntPtr jnienv, IntPtr self);
	// …

wherein `ExportTest._<$>_jni_marshal_methods` and the `_JniMarshal*`
delegate types are added to the assembly.

This also unblocks the desire stated in 4787e01:

> For `Java.Base`, @jonpryor wants to support the custom marshaling
> infrastructure introduced in 77a6bf8.  This would allow types to
> participate in JNI marshal method ("connector method") generation
> *at runtime*, allowing specialization based on the current set of
> types and assemblies.

What can we do with this `jnimarshalmethod-gen` output?  Use it!

First, make sure the tests work:

	# do this *before* running above `dotnet jnimarshalmethod-gen.dll` command…
	% dotnet test --logger "console;verbosity=detailed" bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll
	…
	Passed!  - Failed:     0, Passed:    17, Skipped:     0, Total:    17, Duration: 103 ms - Java.Interop.Export-Tests.dll (net7.0)

Then after running the above `dotnet jnimarshalmethod-gen.dll` command,
re-run the tests:

	% dotnet test --logger "console;verbosity=detailed" bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll
	…
	Total tests: 17
	     Passed: 17

`core-tests.yaml` has been updated to do this.

~~ One-Off Tests ~~

One-off tests: ensure that the generated assembly can be decompiled:

	% ikdasm  bin/TestDebug-net7.0/Java.Interop.Tools.Expressions-Tests-ExpressionAssemblyBuilderTests.dll
	% monodis bin/TestDebug-net7.0/Java.Interop.Tools.Expressions-Tests-ExpressionAssemblyBuilderTests.dll

	% ikdasm  _x/Java.Interop.Export-Tests.dll
	% monodis _x/Java.Interop.Export-Tests.dll
	# which currently fails :-(

Re-enable most of `Java.Interop.Export-Tests.dll` for .NET 7;
see 41ba348, which disabled those tests.

To verify the generated IL, use the [dotnet-ilverify][0] tool:

	dotnet tool install --global dotnet-ilverify

Usage of which is "weird":

	$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.0/*.dll'
	All Classes and Methods in /Volumes/Xamarin-Work/src/xamarin/Java.Interop/_x/Java.Interop.Export-Tests.dll Verified.
	# no errors!

where:

  * `--tokens`: Include metadata tokens in error messages.
  * `--system-module NAME`: set the "System module name".  Defaults
    to `mscorlib`, which is wrong for .NET 5+, so this must be set to
    `System.Private.CoreLib` (no `.dll` suffix!).
  * `-r FILE-GLOB`: Where to resolve assembly references for the
    input assembly.  Fortunately file globs are supported…

~~ Removing `System.Private.CoreLib` ~~

`System.Private.CoreLib.dll` is *private*; it's not part of the
public assembly surface area, so you can't use
`csc -r:System.Private.CoreLib …` and expect it to work.  This makes
things interesting because *at runtime* everything "important" is in
`System.Private.CoreLib.dll`, like `System.Object`.

Specifically, if we do the "obvious" thing and do:

	newTypeDefinition.BaseType = assemblyDefinition.MainModule
	    .ImportReference (typeof (object));

you're gonna have a bad type, because the resulting IL for
`newTypeDefinition` will have a base class of
`[System.Private.CoreLib]System.Object`, which isn't usable.

Fix this by:

 1. Writing the assembly to a `Stream`.
 2. Reading the `Stream` from (1)
 3. Fixing all member references and assembly references so that
    `System.Private.CoreLib` is not referenced.

If `jnimarshalmethod-gen.dll --keeptemp` is used, then a `.cecil`
file is created with the contents of (1).

Additionally, and unexpectedly -- [jbevain/cecil#895][1] -- Mono.Cecil
adds a reference to the assembly being modified.  Remove the declaring
assembly from `AssemblyReferences`.

[0]: https://www.nuget.org/packages/dotnet-ilverify
[1]: jbevain/cecil#895
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
callable-wrappers Issues with Java Callable Wrappers enhancement Proposed change to current functionality
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@jonpryor @jpobst