Review Request: 8238358: Implementation of JEP 371: Hidden Classes

Peter Levart peter.levart at gmail.com
Sat Apr 4 16:16:15 UTC 2020


Hi Mandy,

Just another observation of the code in InnerClassLambdaMetafactory...

For the useImplMethodHandle case you generate the protectedImplMethod 
static field to hold the MH and a static setter method:

             mv = cw.visitMethod(ACC_PRIVATE + ACC_STATIC,
                                 "setImplMethod", DESCR_SET_IMPL_METHOD,
                                 null, null);

...but then later after you define the class you inject the MH via a 
"staticSetter" method handle:

                 MethodHandle mh = 
lookup.findStaticSetter(lookup.lookupClass(), NAME_FIELD_IMPL_METHOD, 
MethodHandle.class);
                 mh.invokeExact(implMethod);

So you don't invoke the generated setter method but set the field via 
special kind of method handle (equivalent to putstatic bytecode).
You can remove the setImplMethod method generation code.

Regards, Peter

On 4/4/20 12:58 PM, Peter Levart wrote:
> Hi Mandy,
>
> On 4/3/20 11:32 PM, Mandy Chung wrote:
>> Hi Peter,
>>
>> I added a JBS comment [1] to describe this special case trying to put 
>> the story together (let me know if it needs more explanation).   More 
>> comment inline below.
>
> Thanks, this helps in establishing the historical context.
>
>>
>> On 4/3/20 4:40 AM, Peter Levart wrote:
>>> Ok, I think I found one such use-case. In the following example:
>>>
>>> package test;
>>> public class LambdaTest {
>>>     protected void m() {
>>>     }
>>> }
>>>
>>> package test.sub;
>>> public class LambdaTestSub extends test.LambdaTest {
>>>     public void test() {
>>>         Runnable r = this::m;
>>>         r.run();
>>>     }
>>> }
>>
>> Yes.
>>
>> This is specific for binary compatibility.   the invocation of a 
>> protected method inherited from its supertype in a different package.
>>
>> The lambda proxy is in the same package as the target class 
>> (`test.sub` in the example above) but it has no access to 
>> `test.LambdaTest::m`.
>>
>>>
>>> ...when compiled with JDK 14 javac. In this case the implClass is 
>>> test.sub.LambdaTestSub while implInfo is "invokeVirtual 
>>> test.LambdaTest.m:()void" and the method is not public.
>>>
>>
>> In JDK 14, a lambda proxy `test.sub.LambdaTestSub$Lambda$$1234` is VM 
>> anonymous class which has a special powerful access as if the host 
>> class.   This proxy class, even though it's not an instance of 
>> `test.LambdaTest`, can invoke  the 
>> protected`test.LambdaTest.m:()void` member.
>
> Right, the VM anonymous class "inherits" all access rights from the 
> host class, which in above example is the subclass of test.LambdaTes.
>
>>
>>> Anyway, the name of the proxy class is derived from the targetClass 
>>> (and therefore shares the same package with targetClass) which is 
>>> caller's lookup class. Is targetClass always the same as implClass 
>>> in the invokeVirtual/invokeInterface case?
>>>
>>
>> implMethod is the direct method handle describing the implementation 
>> method resolved by the VM.   So it depends.
>>
>> In the above example, it's `test.LambdaTest.m:()void` and implClass 
>> is test.LambdaTest.
>
> Here I think, you are not quite right. First I need to clarify that we 
> are talking about the case where the method reference in above example 
> is not converted to lambda by javac, so the proxy class needs to 
> invoke the superclass method directly (without the help of lambda 
> bridge). I did an experiment and compiled the example with JDK 13 
> javac, where the patch for (JDK-8234729) is not applied yet. What I 
> get from this compilation is the following metafactory bootstrap 
> method invocation:
>
>   0: #35 REF_invokeStatic 
> java/lang/invoke/LambdaMetafactory.metafactory:(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;Ljava/lang/invoke/MethodType;Ljava/lang/invoke/MethodHandle;Ljava/lang/invoke/MethodType;)Ljava/lang/invoke/CallSite;
>     Method arguments:
>       #42 ()V
>       #43 REF_invokeVirtual test/LambdaTest.m:()V
>       #42 ()V
>
> The #43 is the implMethod method handle and it is the following:
>
>   #43 = MethodHandle       5:#44          // REF_invokeVirtual 
> test/LambdaTest.m:()V
>   #44 = Methodref          #2.#45         // test/LambdaTest.m:()V
>   #45 = NameAndType        #46:#6         // m:()V
>   #46 = Utf8               m
>    #2 = Class              #4             // test/LambdaTest
>    #4 = Utf8               test/LambdaTest
>
> *BUT* the class that looks up this MH from the constant pool is the 
> subclass (test.sub.LambdaTestSub) which is equivalent to the following 
> programmatic lookup:
>
>         var mh = MethodHandles.lookup().findVirtual(LambdaTest.class, 
> "m", MethodType.methodType(void.class));
>         System.out.println(mh.type());
>
> and this results in a method handle of the following type: 
> (LambdaTestSub)void
>
> which is correct since the method handle *MUST* check that the passed 
> in instance (this) is of type LambdaTestSub or subtype or else the 
> "protected" access would be violated.
>
> And since the ref type is REF_invokeVirtual, the 
> AbstractValidatingLambdaMetafactory assigns the following to the 
> implClass:
>
>         this.implMethodType = implMethod.type();
>         this.implInfo = caller.revealDirect(implMethod);
>         switch (implInfo.getReferenceKind()) {
>             case REF_invokeVirtual:
>             case REF_invokeInterface:
>                 this.implClass = implMethodType.parameterType(0);
>
> ...which makes the implClass be LambdaTestSub in this case. Which is 
> correct again since we want InnerClassLambdaMetafactory to decide that 
> this is the special case for proxy to invoke the method via method 
> handle:
>
>         useImplMethodHandle = 
> !implClass.getPackageName().equals(implInfo.getDeclaringClass().getPackageName())
>                                 && 
> !Modifier.isPublic(implInfo.getModifiers());
>
> If implClass was test.LambdaTest as you said above, this condition 
> would evaluate to false, since implInfo is "invokeVirtual 
> test.LambdaTest.m:()void" in above case.
>
> So everything is OK, but my original question was the following: The 
> name of the generated proxy class is derived from the targetClass 
> which is the caller's lookup class. In this example the caller is 
> LambdaTestSub and this is the same as implClass in this case. Would 
> those two classes always be the same in the case where the evaluation 
> of the above `useImplMethodHandle` boolean matters? I mean, the 
> decision about whether to base the proxy invocation strategy on method 
> handle or direct bytecode invocation is based on one class 
> (implClass), but the actual package of the proxy class which 
> effectively influences the bytecode invocation rights is taken from 
> another class (targetClass).
>
> On one hand the package of the proxy class has to be the same as 
> targetClass if the proxy wants to be the nestmate of the targetClass 
> (for example to have private access to the members of the nest). But 
> OTOH it needs to be the same package also with implClass so that the 
> above decision of the proxy invocation strategy is correct. I have a 
> feeling that for protected virtual methods, this is true, because the 
> type of the 0-argument of such method handle is always the lookup 
> class. But am I right?
>
> What do you think of another alternative to invoking the super 
> protected method in other package: What if the LMF would decide to 
> base the name of the proxy class on the implInfo.getDeclaringClass() 
> in such case? It would not have to be a nestmate of any class, just in 
> the same package with the method's declaring class. Consequently it 
> would be in the same module as the declaring class and loaded by the 
> same class loader. Therefore it would have to be WEAKLY referenced 
> from the class loader. And the Lookup instance passed to bootstrap LMF 
> method would not be enough for defining such class. But LMF could 
> obtain special powers since it is JDK internal class...
>
> Well, I don't know for myself. Is this situation rare enough so that 
> invoking via method handle is not a drawback? It only happens when 
> running JDK 13- compiled classes with JDK 15+ and in addition the 
> method reference must point to protected method in a distant class.
>
>> The targetClass is test.sub.LambdaTestSub which is *NOT* the same as 
>> implClass.  That's why we change if they are in the same package.
>>
>> It can be invoking a method in targetClass or a method in another 
>> class in the same package with package access, then implClass may or 
>> may not be the same as targetClass.
>>
>>> I also noticed that JDK 15 patched javac transforms method reference 
>>> in above code into a lambda method. But looking at the javac changes 
>>> in the patch, I don't quite see where this distinction between JDK 
>>> 14 and patched JDK 15 javac comes from. 
>>
>> javac has been changed in JDK 14 to synthesize a bridge method if 
>> it's a method reference to access a protected member in a remote 
>> supertype  (JDK-8234729).
>
> Ah, that was the reason I haven't seen the change in your patch. I 
> used the JDK 13 javac and thought it would be the same as JDK 14 
> javac. My mistake.
>
> So this answers the question below...
>
>
> Regards, Peter
>
>>
>> BTW, the new tests relevant to this scenario are under 
>> test/jdk/java/lang/invoke/lambda/superProtectedMethod.
>>
>>> From the changes to method 
>>> com.sun.tools.javac.comp.LambdaToMethod.LambdaAnalyzerPreprocessor.ReferenceTranslationContext#needsConversionToLambda:
>>>
>>>             final boolean needsConversionToLambda() {
>>>                 return interfaceParameterIsIntersectionOrUnionType() ||
>>>                         isSuper ||
>>>                         needsVarArgsConversion() ||
>>>                         isArrayOp() ||
>>> #                        isPrivateInOtherClass() ||
>>> isProtectedInSuperClassOfEnclosingClassInOtherPackage() ||
>>>                         !receiverAccessible() ||
>>>                         (tree.getMode() == ReferenceMode.NEW &&
>>>                           tree.kind != ReferenceKind.ARRAY_CTOR &&
>>>                           (tree.sym.owner.isLocal() || 
>>> tree.sym.owner.isInner()));
>>>             }
>>>
>>> ...I would draw the conclusion that conversion to lambda is 
>>> performed in less cases not in more. 
>>
>> Jan and Srikanath may be able to explain this further.
>>
>>> Hm.
>>>
>>> Regards, Peter
>>>
>>> On 4/3/20 11:11 AM, Peter Levart wrote:
>>>> Hi Mandy,
>>>>
>>>> Good work.
>>>>
>>>> I'm trying to find out which language use-case is covered by the 
>>>> InnerClassLambdaMetafactory needing to inject method handle into 
>>>> the generated proxy class to be invoked instead of proxy class 
>>>> directly invoking the method:
>>>>
>>>>         useImplMethodHandle = 
>>>> !implClass.getPackageName().equals(implInfo.getDeclaringClass().getPackageName())
>>>>                                 && 
>>>> !Modifier.isPublic(implInfo.getModifiers());
>>>>
>>>> If I check what implClass and implInfo get resolved to in 
>>>> AbstractValidatingLambdaMetafactory, there are several cases:
>>>>
>>>>         this.implInfo = caller.revealDirect(implMethod);
>>>>         switch (implInfo.getReferenceKind()) {
>>>>             case REF_invokeVirtual:
>>>>             case REF_invokeInterface:
>>>>                 this.implClass = implMethodType.parameterType(0);
>>>>                 // reference kind reported by implInfo may not 
>>>> match implMethodType's first param
>>>>                 // Example: implMethodType is (Cloneable)String, 
>>>> implInfo is for Object.toString
>>>>                 this.implKind = implClass.isInterface() ? 
>>>> REF_invokeInterface : REF_invokeVirtual;
>>>>                 this.implIsInstanceMethod = true;
>>>>                 break;
>>>>             case REF_invokeSpecial:
>>>>                 // JDK-8172817: should use referenced class here, 
>>>> but we don't know what it was
>>>>                 this.implClass = implInfo.getDeclaringClass();
>>>>                 this.implIsInstanceMethod = true;
>>>>
>>>>                 // Classes compiled prior to dynamic nestmate 
>>>> support invokes a private instance
>>>>                 // method with REF_invokeSpecial.
>>>>                 //
>>>>                 // invokespecial should only be used to invoke 
>>>> private nestmate constructors.
>>>>                 // The lambda proxy class will be defined as a 
>>>> nestmate of targetClass.
>>>>                 // If the method to be invoked is an instance 
>>>> method of targetClass, then
>>>>                 // convert to use invokevirtual or invokeinterface.
>>>>                 if (targetClass == implClass && 
>>>> !implInfo.getName().equals("<init>")) {
>>>>                     this.implKind = implClass.isInterface() ? 
>>>> REF_invokeInterface : REF_invokeVirtual;
>>>>                 } else {
>>>>                     this.implKind = REF_invokeSpecial;
>>>>                 }
>>>>                 break;
>>>>             case REF_invokeStatic:
>>>>             case REF_newInvokeSpecial:
>>>>                 // JDK-8172817: should use referenced class here 
>>>> for invokestatic, but we don't know what it was
>>>>                 this.implClass = implInfo.getDeclaringClass();
>>>>                 this.implKind = implInfo.getReferenceKind();
>>>>                 this.implIsInstanceMethod = false;
>>>>                 break;
>>>>             default:
>>>>                 throw new 
>>>> LambdaConversionException(String.format("Unsupported MethodHandle 
>>>> kind: %s", implInfo));
>>>>         }
>>>>
>>>>
>>>> For majority of cases (REF_invokeSpecial, REF_invokeStatic, 
>>>> REF_newInvokeSpecial) the this.implClass = 
>>>> implInfo.getDeclaringClass();
>>>>
>>>> Only REF_invokeVirtual and REF_invokeInterface are possible 
>>>> kandidates, right?
>>>>
>>>> So when does the implMethod type of parameter 0 differ from the 
>>>> declaring class of the MethodHandleInfo cracked from that same 
>>>> implMethod and in addition those two types leave in different 
>>>> packages?
>>>>
>>
>> This is concerning the instance method and so parameter 0 is what it 
>> wants to look at.
>>
>>>> Regards, Peter
>>>>
>>
>> Hope this helps.
>> Mandy
>> [1] 
>> https://bugs.openjdk.java.net/browse/JDK-8239384?focusedCommentId=14328369&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14328369
>>
>>
>



More information about the valhalla-dev mailing list