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

Array type is getting generated in the .class where java.lang.Object should have been generated #147

Closed
sravanlakkimsetti opened this issue Jun 17, 2022 · 6 comments · Fixed by #153
Milestone

Comments

@sravanlakkimsetti
Copy link
Member

Based on eclipse-equinox/p2#92

for the code return methodHandle.invoke(self);, We are generating

INVOKEVIRTUAL java/lang/invoke/MethodHandle.invoke ([Ljava/lang/Object;)Ljava/lang/Object;
@sravanlakkimsetti
Copy link
Member Author

Notify @jarthana

@jarthana
Copy link
Member

This is turns out to be a result of consuming "java/lang/invoke/MethodHandle" with --release option. This particular class file, when --release is used is having wrong content. I.e., the RuntimeVisibleAnnotations attribute is missing. As a result of this, the @PolymorphicSignature from the MethodHandle#invoke() is ignored, which further affects how we generate code for a variable arity method. As far JDT is concerned, the relevant code is in Statement#generateArguments() where binding.isVarargs() returns false since the method is not created as polymorphic. If we have a version of JDK with correct .class files, we should take them and close this as not eclipse.

@sravanlakkimsetti
Copy link
Member Author

sravanlakkimsetti commented Jun 20, 2022

I went ahead with looking into ct.sym file changes between java 17.0.1 and 17.0.3. There are no changes between them. I am really not confident on calling this as fixed in 17.0.3 yet.

Will continue investigation on this one.

@jarthana
Copy link
Member

I went ahead with looking into ct.sym file changes between java 17.0.1 and 17.0.3. There are no changes between them. I am really not confident on calling this as fixed in 17.0.3 yet.

Will continue investigation on this one.

Actually, I find the produced .class file having the same issue with 17.0.3.1 as well.

@sravanlakkimsetti
Copy link
Member Author

We are depending on PolymorphicSignature annotation to determine whether the method is polymorphic or not. But according to https://docs.oracle.com/javase/specs/jls/se18/html/jls-15.html#jls-15.12.3

A method is signature polymorphic if all of the following are true:

  • It is declared in the java.lang.invoke.MethodHandle class or the java.lang.invoke.VarHandle class.
  • It has a single variable arity parameter (§8.4.1) whose declared type is Object[].
  • It is native.

I guess we should deduce PolymorphicSignature annotation based on above definition

@sravanlakkimsetti
Copy link
Member Author

Here is the sample code

import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
import java.lang.reflect.Method;


public class Cl {
	public final Object invoke(Object self) throws Throwable {

			Method method = null;
			try {
				MethodHandle methodHandle = MethodHandles.lookup().unreflect(method );
				return methodHandle.invoke(self);
			} catch (IllegalArgumentException e) {
				throw e;
			} 
		}
}

@sravanlakkimsetti sravanlakkimsetti changed the title Array type is getting generated in the .class where a single object should have been generated Array type is getting generated in the .class where java.lang.Object should have been generated Jun 21, 2022
sravanlakkimsetti added a commit that referenced this issue Jul 5, 2022
* Adding Polymorphic method deduction

We are depending on PolymorphicSignature annotation to determine whether
the method is polymorphic or not. But according to
https://docs.oracle.com/javase/specs/jls/se11/html/jls-15.html#jls-15.12.3

A method is signature polymorphic if all of the following are true:

It is declared in the java.lang.invoke.MethodHandle class or the
java.lang.invoke.VarHandle class.
It has a single variable arity parameter (§8.4.1) whose declared type is
Object[].
It is native.
I guess we should deduce PolymorphicSignature annotation based on above
definition

Fixes #147

* Review Discussion
It seems like Java invented an interesting notion of "internal":

The annotation type PolymorphicSignature is package private in MethodHandle, which renders its use outside java.lang.invoke illegal. They could've stopped here.

As the annotation has retention RUNTIME it must be present in MethodHandle.class.

I was surprised to see even RUNTIME as the annotation is intended (as per javadoc) to aid "the Java compiler", but fair enough, likely the VM needs it, too.

But then, in --release mode, MethodHandle.class isn't even used, but some MethodHandle.sig.

Seeing lack of file PolymorphicSignature.sig in ct.sym could be understood as making use of this annotation in user code doubly illegal (why?).

But seeing methods like invokeExact() defined in MethodHandle.sig without any annotation (source code has two), is - weird.

As a result, the tool (java compiler) that is the main client of the annotation cannot see the annotation in --release mode.

If only PolymorphicSignature.sig was missing from ct.sym I would recommend to interpret the name even if it cannot be resolved, but as even the annotation reference in invokeExact() and friends is missing, we don't seem to be allowed to do what the spec requires us to do.

In this light, the find in jls-15.12.3 is precious! :) That's the best possible kludge we can clutch at.

I only see one downside in the deduction strategy: ecj gets more fragile vis-a-vis changes in JLS (that may or may not be announced), whereas reading a manifest annotation would be future proof.

Co-authored-by: Jay Arthanareeswaran <jarthana@in.ibm.com>
@jarthana jarthana added this to the 4.25 M2 milestone Jul 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants