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

[generator] Qualify Java.Lang.Object.Handle references #38

Merged
merged 1 commit into from
May 30, 2016

Conversation

jonpryor
Copy link
Member

Android N Preview 2 adds some methods named handle(), such as
java.util.concurrent.CompletableFuture.handle(). This clashes with
the Java.Lang.Object.Handle property, resulting in C# compiler errors
within the generated binding code:

if (Handle != IntPtr.Zero)
// Java.Util.Concurrent.CompletableFuture.cs(101,8): error CS0019: Operator `!=' cannot be applied to operands of type `method group' and `System.IntPtr'

__args [0] = new JniArgumentValue ((other == null) ? IntPtr.Zero : other.Handle);
// Java.Util.Concurrent.CompletableFuture.cs(254,65): error CS0173: Type of conditional expression cannot be determined because there is no implicit conversion between `System.IntPtr' and `method group'

There are two possible fixes:

  1. Forbid the name Handle, and rename any occurrences to something
    else, such as InvokeHandle.
  2. Qualify the Java.Lang.Object.Handle property reference.

(1) is kinda ugly.

(2) is possible by using a cast: instead of value.Handle, use e.g.
((Java.Lang.Object) value).Handle, which causes the compiler to use
the intended member, and not whatever may be hiding it in the current
scope.

(2) results in an arguably nicer API -- maybe? -- as the bound API
more closely mirrors the Java API.

(2) also results in uglier binding code. However, who reads it?

Note: Why cast to e.g. Java.Lang.Object and not IJavaObject? To avoid
virtual calls. ((Java.Lang.Object)value).Handle results in a direct
invocation of the Object.get_Handle() method, avoiding a virtual call.
Casting to IJavaObject would result in a virtual interface method
invocation, causing additional runtime overhead to lookup the value.

Android N Preview 2 adds some methods named `handle()`, such as
`java.util.concurrent.CompletableFuture.handle()`. This clashes with
the Java.Lang.Object.Handle property, resulting in C# compiler errors
within the generated binding code:

	if (Handle != IntPtr.Zero)
	// Java.Util.Concurrent.CompletableFuture.cs(101,8): error CS0019: Operator `!=' cannot be applied to operands of type `method group' and `System.IntPtr'

	__args [0] = new JniArgumentValue ((other == null) ? IntPtr.Zero : other.Handle);
	// Java.Util.Concurrent.CompletableFuture.cs(254,65): error CS0173: Type of conditional expression cannot be determined because there is no implicit conversion between `System.IntPtr' and `method group'

There are two possible fixes:

1. Forbid the name `Handle`, and *rename* any occurrences to something
    else, such as `InvokeHandle`.

2. *Qualify* the `Java.Lang.Object.Handle` property reference.

(1) is kinda ugly.

(2) is possible by using a cast: instead of `value.Handle`, use e.g.
`((Java.Lang.Object) value).Handle`, which causes the compiler to use
the intended member, and not whatever may be hiding it in the current
scope.

(2) results in an arguably nicer API -- maybe? -- as the bound API
more closely mirrors the Java API.

(2) also results in uglier binding code. However, who reads it?

Note: Why cast to e.g. Java.Lang.Object and not IJavaObject? To avoid
virtual calls. `((Java.Lang.Object)value).Handle` results in a direct
invocation of the Object.get_Handle() method, avoiding a virtual call.
Casting to IJavaObject would result in a virtual interface method
invocation, causing additional runtime overhead to lookup the value.
@atsushieno
Copy link
Contributor

I'm a bit scared of passing expression by string but we can add more arguments later if required.

@atsushieno atsushieno merged commit dd1d972 into dotnet:master May 30, 2016
radekdoulik added a commit to radekdoulik/java.interop that referenced this pull request Aug 27, 2018
Changes in xamarin-android-tools between 75530565b6aa903b3a0e52b61df4dd94475a19fc and 9e78d6ee586b498d0ea082b3bc00432c23583dd1:

9e78d6e (HEAD, origin/master, origin/HEAD, master) [tests] fix test failures on Windows (dotnet#47)
bdf0158 Better support no installed JDKs on macOS (dotnet#48)
6353659 Log what is happening during path selection (dotnet#46)
3ef860b Take BUILD_NUMBER into consideration for Version sorting (dotnet#45)
d3de054 Allow an optional locator to be provided to JdkInfo (dotnet#43)
917d3b3 Don't require quotes around `release` values (dotnet#41)
7427692 [tests] Unit tests for finding NDK location based on $PATH (dotnet#40)
dbc517b Merge pull request dotnet#38 from jonpryor/jonp-ndk-via-path
511d580 Allow finding NDK location based on $PATH
b42c217 [tests] Fix DetectAndSetPreferredJavaSdkPathToLatest() test (dotnet#37)
a4aad18 Add AndroidSdkInfo.DetectAndSetPreferredJavaSdkPathToLatest() (dotnet#35)
fae7e0a [tests] Remove temporary directories (dotnet#36)
07c4c2b [Xamarin.Android.Tools.AndroidSdk] Revert JDK validation (dotnet#34)
radekdoulik added a commit that referenced this pull request Aug 27, 2018
Changes in xamarin-android-tools between 75530565b6aa903b3a0e52b61df4dd94475a19fc and 9e78d6ee586b498d0ea082b3bc00432c23583dd1:

9e78d6e (HEAD, origin/master, origin/HEAD, master) [tests] fix test failures on Windows (#47)
bdf0158 Better support no installed JDKs on macOS (#48)
6353659 Log what is happening during path selection (#46)
3ef860b Take BUILD_NUMBER into consideration for Version sorting (#45)
d3de054 Allow an optional locator to be provided to JdkInfo (#43)
917d3b3 Don't require quotes around `release` values (#41)
7427692 [tests] Unit tests for finding NDK location based on $PATH (#40)
dbc517b Merge pull request #38 from jonpryor/jonp-ndk-via-path
511d580 Allow finding NDK location based on $PATH
b42c217 [tests] Fix DetectAndSetPreferredJavaSdkPathToLatest() test (#37)
a4aad18 Add AndroidSdkInfo.DetectAndSetPreferredJavaSdkPathToLatest() (#35)
fae7e0a [tests] Remove temporary directories (#36)
07c4c2b [Xamarin.Android.Tools.AndroidSdk] Revert JDK validation (#34)
@github-actions github-actions bot locked and limited conversation to collaborators Apr 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants