[generator] Fix potential callback naming conflict. #1239
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
|
Aside on this: why don't we encounter this in So I look at a build of java-interop, at xpath -e '//class[method[@bridge="true"]]/@jni-signature' src/Java.Base/obj/Debug-net8.0/mcw/api.xmlwhich returns hundreds of matches, including % javap java.lang.Long
public final class java.lang.Long extends java.lang.Number implements java.lang.Comparable<java.lang.Long> …
…
public int compareTo(java.lang.Long);
…
public int compareTo(java.lang.Object);OK, this makes sense to me. So I look at I assume this is due to some fixups within Assuming that this is due to parameter changes, what about return type changes? Alas, I can't find a public type with a bridge method that has a return type change in So instead of desktop Java, what about % dotnet bin/Debug-net8.0/class-parse.dll ~/android-toolchain/sdk/platforms/android-35/android.jar > android-35.xml
% grep 'bridge="true"' android-35.xml | wc -l
712Compare to 56 bridge methods within So now I'm further confused: how are we generating By the time we get to None of these appear to make it into bindings: one of those is for (I'm also at a loss for why So I've just managed to raise more unanswered questions. |
Contributor
|
Commit message for review: Fixes: https://github.com/dotnet/java-interop/issues/1155
Context: https://docs.oracle.com/javase/tutorial/java/generics/bridgeMethods.html
A *bridge method* is a Java compiler-generated method "overload"
which deals with type coercion, to support Java type erasure:
package com.google.mlkit.vision.objects;
// https://developers.google.com/android/reference/com/google/mlkit/vision/objects/ObjectDetectorOptionsBase
public class ObjectDetectorOptionsBase {
/** @hide */ public static class Builder<B extends Builder> {
public B setDetectorMode(int detectorMode) {…}
}
}
package com.google.mlkit.vision.objects.defaults;
public class ObjectDetectorOptions {
// https://developers.google.com/android/reference/com/google/mlkit/vision/objects/defaults/ObjectDetectorOptions.Builder
public static class Builder extends ObjectDetectorOptionsBase.Builder<Builder> {
public Builder setDetectorMode(int detectorMode) {…}
}
}
It *looks like* there is only one
`ObjectDetectorOptions.Builder.setDetectorMode()` method, but there
are in fact *two*. The first is the bridge method (`ACC_BRIDGE`),
the second method is the "real" method; they differ on return type:
% javap -cp classes.jar -s -v 'com/google/mlkit/vision/objects/defaults/ObjectDetectorOptions$Builder'
…
public final com.google.mlkit.vision.objects.ObjectDetectorOptionsBase$Builder setDetectorMode(int);
descriptor: (I)Lcom/google/mlkit/vision/objects/ObjectDetectorOptionsBase$Builder;
flags: (0x1051) ACC_PUBLIC, ACC_FINAL, ACC_BRIDGE, ACC_SYNTHETIC
…
public com.google.mlkit.vision.objects.defaults.ObjectDetectorOptions$Builder setDetectorMode(int);
descriptor: (I)Lcom/google/mlkit/vision/objects/defaults/ObjectDetectorOptions$Builder;
flags: (0x0001) ACC_PUBLIC
This is represented within `class-parse` XML as:
<method abstract="false" deprecated="not deprecated" final="false" name="setDetectorMode" jni-signature="(I)Lcom/google/mlkit/vision/objects/defaults/ObjectDetectorOptions$Builder;" bridge="false" native="false" return="com.google.mlkit.vision.objects.defaults.ObjectDetectorOptions.Builder" jni-return="Lcom/google/mlkit/vision/objects/defaults/ObjectDetectorOptions$Builder;" static="false" synchronized="false" synthetic="false" visibility="public" return-not-null="true">
<parameter name="detectorMode" type="int" jni-type="I" />
</method>
<method abstract="false" deprecated="not deprecated" final="true" name="setDetectorMode" jni-signature="(I)Lcom/google/mlkit/vision/objects/ObjectDetectorOptionsBase$Builder;" bridge="true" native="false" return="java.lang.Object" jni-return="Lcom/google/mlkit/vision/objects/ObjectDetectorOptionsBase$Builder;" static="false" synchronized="false" synthetic="true" visibility="public" return-not-null="true">
<parameter name="p0" type="int" jni-type="I" />
</method>
The method with `bridge="true"` is the bridge method.
When bound in C#, this causes an issue because two methods cannot
have the same method name and parameter types:
// C# binding
public partial class ObjectDetectorOptions {
public partial class Builder {
public virtual MLKit.Vision.Objects.Defaults.ObjectDetectorOptions.Builder SetDetectorMode (int detectorMode);
public virtual MLKit.Vision.Objects.ObjectDetectorOptionsBase.Builder SetDetectorMode (int p0);
}
}
We can set `managedName` metadata to rename one of these methods:
<attr path="//method[@name='setDetectorMode' and @bridge='true']" name="managedName>SetDetectorMode2</attr>
However, we generate the callback delegate field with the algorithm
`$"cb_{JavaName}{IDSignature}"`, so this identifier is still used
twice, preventing C# compilation:
// C# binding
public partial class ObjectDetectorOptions {
public partial class Builder {
static Delegate cb_setDetectorMode_I;
static Delegate cb_setDetectorMode_I;
}
}
The solution is to add both the _managed_ name and the return type to
the callback naming algorithm to ensure that it will always be unique
for a method: `$"cb_{JavaName}_{Name}{IDSignatureWithReturnType}"`:
// C# binding
public partial class ObjectDetectorOptions {
public partial class Builder {
static Delegate cb_setDetectorMode_SetDetectorMode_I_Lcom_google_mlkit_vision_objects_defaults_ObjectDetectorOptions$Builder_;
static Delegate cb_setDetectorMode_SetDetectorMode2_I_Lcom_google_mlkit_vision_objects_ObjectDetectorOptions$Builder_;
}
} |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes: #1155
When binding Java methods, we can come across "bridge" methods that have the same name and parameters as a "real" method. They only differ by return type.
When bound in C#, this causes an issue because 2 methods cannot have the same name and parameters.
We can use
managedNamemetadata to rename one of these methods, however we generate the callback delegate field with the algorithm$"cb_{JavaName}{IDSignature}", so this name is still used twice, preventing C# compilation.The solution is to add both the managed name and the return type to the callback naming algorithm to ensure that it will always be unique for a method. (
$"cb_{JavaName}_{Name}{IDSignatureWithReturnType}")XA test PR: dotnet/android#9172