Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Use Java generic type parameters constraints #669

Open
jonpryor opened this issue Jun 19, 2020 · 3 comments
Open

Use Java generic type parameters constraints #669

jonpryor opened this issue Jun 19, 2020 · 3 comments
Labels
enhancement Proposed change to current functionality generator Issues binding a Java library (generator, class-parse, etc.)

Comments

@jonpryor
Copy link
Member

generator doesn't "forward" and use generic type constraints in bindings. Consider this Java type:

package com.xamarin.android;

public interface InvokeRunnable<T extends Runnable> {
    void invoke (T runnable);
}

Here, T is contained to be a type that implements java.lang.Runnable.

However, the binding that generator emits is:

	[Register ("com/xamarin/android/InvokeRunnable", "", "Com.Xamarin.Android.IInvokeRunnableInvoker")]
	[global::Java.Interop.JavaTypeParameters (new string [] {"T extends java.lang.Runnable"})]
	public partial interface IInvokeRunnable : IJavaObject, IJavaPeerable {

		// Metadata.xml XPath method reference: path="/api/package[@name='com.xamarin.android']/interface[@name='InvokeRunnable']/method[@name='invoke' and count(parameter)=1 and parameter[1][@type='T']]"
		[Register ("invoke", "(Ljava/lang/Runnable;)V", "GetInvoke_Ljava_lang_Runnable_Handler:Com.Xamarin.Android.IInvokeRunnableInvoker, Xamarin.Android.McwGen-Tests")]
		void Invoke (global::Java.Lang.Object p0);

	}

The Java-side parameter, which must be a java.lang.Runnable or type implementing that interface, is bound as Java.Lang.Object.

Feature request: "forward" this constraint information, and use it in the binding, so that we instead emit:

	public partial interface IInvokeRunnable : IJavaObject, IJavaPeerable {

		[Register ("invoke", "(Ljava/lang/Runnable;)V", "GetInvoke_Ljava_lang_Runnable_Handler:Com.Xamarin.Android.IInvokeRunnableInvoker, Xamarin.Android.McwGen-Tests")]
		void Invoke (global::Java.Lang.IRunnable p0);
	}
@jpobst jpobst removed their assignment Jun 19, 2020
@jpobst jpobst added enhancement Proposed change to current functionality generator Issues binding a Java library (generator, class-parse, etc.) labels Jun 19, 2020
@AmrAlSayed0
Copy link

While trying to fix issues in a Java library I'm binding I came upon this issue. Easily fixable with Metadata attr BUT I'm curious as to why the C# interface isn't a real generic interface like

public partial interface IInvokeRunnable <T> : IJavaObject, IJavaPeerable
where T : global::Java.Lang.IRunnable
{
    [Register ("invoke", "(Ljava/lang/Runnable;)V", "GetInvoke_Ljava_lang_Runnable_Handler:Com.Xamarin.Android.IInvokeRunnableInvoker, Xamarin.Android.McwGen-Tests")]
    void Invoke (T p0);
}

I know there is a reason I'm completely overlooking because I've seen this "issue" in classes too.

@jpobst
Copy link
Contributor

jpobst commented Feb 8, 2021

Unlike .NET, Java generics do not exist in compiled code or at runtime. They are simply compiler magic that the compiler removes when compiling code.

Since we bind .jar files, the generic information is gone.

Assuming that could be fixed, we have another issue at runtime. If Java gives us an arbitrary ArrayList object and we want to convert it to a ArrayList<T>, we don't know what T is.

For these reasons, our bindings reflect the Java compiled API (without generics) and not the source API.

@AmrAlSayed0
Copy link

But generics with constraints is fine cuz you know what T is, got it. I suspected that would be the reason.

I really want to see this implemented (+ covariant returns) because I have seen Builder<T extends Builder<T>> classes basically become useless when all of the return types of all the methods become Java.Lang.Object when it's clearly more than that.

jonpryor added a commit that referenced this issue Dec 7, 2021
Context: #858

What do *I* want?  To be able to use our wonderful Java binding
infrastructure against *Desktop Java*, not just Android.

At the same time, I don't want "Android-isms" "leaking" into such a
binding.  *Just* Java.Interop, no xamarin-android.

"Take over" the `generator --codegen-target=JavaInterop1` format
so that it *isn't* useful for Xamarin.Android, and is instead usable
for non-Android usage.

This is a work-in-progress, and *far* from complete.  For prototype
purposes, this *only* binds:

  * `java.lang.Class`
  * `java.lang.Math`
  * `java.lang.Number`
  * `java.lang.Object`
  * `java.lang.Throwable`

The `Java.Base` binding is only for .NET 6 and above.  I'm not
interested in .NET Standard support at this point in time.

~~ Build Changes ~~

Additionally, this `Java.Base` binding is only for the
`java.base.jmod` file provided by JDK-11.  `make prepare` is updated
to look for a JDK-11 installation path.

~~ Test Changes ~~

Update `samples/Hello` so that it (1) works, and (2) instantiates the
`Java.Lang.Object` binding:

	dotnet run --project samples/Hello

Update `tests/generator-Tests` so that
`tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/JavaInterop1`
is now used *exclusively* for `generator --codegen-target=JavaInterop1`
output; `generator --codegen-target=XAJavaInterop1` now has its own
separate files in
`tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1`.
(This contributes to much of the commit diff.)

Update `tests/generator-Tests/SupportFiles` so that types in
`Android.*` are excluded when `JAVA_INTEROP1` is `#define`d, and
update the unit test infrastructure so that building for `JavaInterop1`
causes `JAVA_INTEROP1` to be `#define`d.  This allows many of the unit
tests to ensure that *some* `JavaInterop1` constructs *compile*.

However, not all unit tests compile under `JavaInterop1`.  The new
`BaseGeneratorTest.TryJavaInterop1` property can be set to false to
prevent compiling a test suite using `JavaInterop1`.  This will allow
us to slowly implement `JavaInterop1` support over time.

The build logs will also now emit a command-line that can be used to
manually compile unit test code.  See e.g.
`bin/TestDebug/TestOutput-generator-Tests.txt`.

~~ API Changes ~~

Update `Java.Interop.JavaObject` so that
`JavaObject.DisposeUnlessReferenced()` is now `virtual`.
Override `DisposeUnlessReferenced()` from the `Java*Array` types
so that if the instance was created via the new
`JniEnvironment.Arrays.CreateMarshal*Array()` methods, the array
instance will be disposed.  This is intended for marshaling array
parameters:

	public void WithArray(int[] array)
	{
	    const string __id = "withArray.[I";
	    var native_array = JniEnvironment.Arrays.CreateMarshalInt32Array (array);
	    try {
	        JniArgumentValue* __args = stackalloc JniArgumentValue [1];
	        __args [0] = new JniArgumentValue (native_array);
	        _members.StaticMethods.InvokeVoidMethod (__id, __args);
	    } finally {
	        if (array != null) native_array.DisposeUnlessReferenced ();
	    }
	}

Add `Java.Interop.JavaTypeParametersAttribute(string[] typeParameters)`
from Xamarin.Android.

~~ Bindings vs. Xamarin.Android ~~

Pull in `src/Java.Base/Transforms/map.csv` [from xamarin-android][0],
removing the `android*` types.

Instead of `[Android.Runtime.RegisterAttribute]` on types, use
`[Java.Interop.JniTypeSignatureAttribute]`.

Java arrays are bound as appropriate `IList<T>`, using the
`Java.Interop.Java*Array` types as an intermediary.  This should help
reduce marshaling logic & overhead, as if the "source" array is a
`Java*Array`, it doesn't need to be "deep copied".  The exception is
C# `params` arrays, which continue to be bound as arrays, and are
marshaled via an appropriate `Java*Array` type.

`java.io.InputStream` isn't bound as `System.IO.Stream`, etc.

"Java.Interop-style" constructors are used (25de1f3), e.g.

	// This
	DeclaringType (ref JniObjectReference reference, JniObjectReferenceOptions options);

	// Not Xamarin.Android-style
	DeclaringType (IntPtr handle, JniHandleOwnership transfer);

"Java.Interop-style" wrapper construction is used, e.g.

	// This
	var wrapper = JniEnvironment.Runtime.ValueManager.GetValue<DeclaringType>(ref h, JniObjectReferenceOptions.CopyAndDispose);

	// Not this
	var wrapper = Java.Lang.Object.GetObject<DeclaringType>(handle);

~~ TODO: Marshal Methods ~~

Marshal methods are currently skipped.  Java-to-managed invocations
are not currently supported.

Xamarin.Android uses Java Callable Wrappers + `Runtime.register()`
to specify which methods to register, via lots of reflection, etc.

Proposal: For Desktop, JCW's shouldn't need to specify all the
methods to register.  Instead, use the `jnimarshalmethod-gen`-
originated strategy of `[JniAddNativeMethodRegistrationAttribute]`
within the binding, and then have it use `MarshalMethodBuilder` to
generate the marshal methods.  Need to update `MarshalMethodBuilder`
to look for overrides in addition to methods with `[JavaCallable]`,
which in turn may require an equivalent to
`[Android.Runtime.RegisterAttribute(…)]`.

Perhaps `[JniMethodSignatureAttribute(string name, string sig)]`?

In the meantime, `Java.Base` will skip all marshal-method logic
plus runtime method generation.  Leave that for later.

~~ TODO: Other Binding Changes? ~~

We should eventually "unify" `java.lang.Object` and `System.Object`.
Consider `java.lang.Class`:

	/* partial */ class Class<T> {
	    public boolean isInstance(java.lang.Object);
	    public java.lang.Object[] getSigners();
	}

If we unify `java.lang.Object` and `System.Object`, we'd have a
binding of:

	partial class Class {
	    public bool IsInstance (object value);
	    public IList<object> GetSigners();
	}

~~ Open Questions ~~

What's up with `java.lang.Class.getAnnotationsByType()`?

During an iteration of this PR, I got:

	public unsafe Java.Interop.JavaObjectArray<Java.Lang.Object>? GetAnnotationsByType (Java.Lang.Class? annotationClass)
	{
	    const string __id = "getAnnotationsByType.(Ljava/lang/Class;)[Ljava/lang/annotation/Annotation;";

From `__id` we see that the Java return type is `Annotation[]`, yet
we bind it as an `Object` array?  This is because of
[#669][1].

That said, it's currently "differently *worse*"; I don't know why,
but `__id` is now:

	const string __id = "getAnnotationsByType.(Ljava/lang/Class;)[Ljava/lang/Object;";

i.e. the return type is an `Object` array instead of an `Annotation`
array, which is wrong, as per `javap`:

	% javap -s java.lang.Class
	…
	  public <A extends java.lang.annotation.Annotation> A getAnnotation(java.lang.Class<A>);
	    descriptor: (Ljava/lang/Class;)Ljava/lang/annotation/Annotation;

This needs additional investigation.

[0]: https://github.com/xamarin/xamarin-android/blob/99523feab02e8622a3357e9e6a025f5afc44c970/src/Mono.Android/map.csv
[1]: #669
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Proposed change to current functionality generator Issues binding a Java library (generator, class-parse, etc.)
Projects
None yet
Development

No branches or pull requests

3 participants