[Nestmates] RFR: 8196320: [Nestmates] Restore the old enclosing-class based isSamePackageMember check in sun/invoke/util/VerifyAccess.java

[mandy chung]

On 1/31/18 10:47 PM, David Holmes wrote:
>
>> OK (I was confused with the semantics of JVM_AreNestMates).  It does 
>> return true on the same class.   It would help if you can add the 
>> comment describing this function in jvm.h.
>
> Most methods in jvm.h are not documented. The assumption seems to be 
> that the semantics are defined as per the JDK method that calls them - 
> in this case Reflection.areNestMates. In this case, like many 
> Reflection methods, that wasn't documented either (oops!) so I've 
> added some basic docs:
>
>  /**
>    * Returns true if {@code currentClass} and {@code memberClass}
>    * are nestmates - that is, if they have the same nesthost as
>    * determined by the VM.
>    */
>

Thanks.
>>>
>>> Callers to Reflection.areNestmates (which calls JVM_AreNestMates) 
>>> can do their own "optimizations" if they wish to pre-check if we are 
>>> dealing with the same class or different packages - as the original 
>>> isSameMemberPackage check does.
>>
>> These optimization in VerifyAccess::areNestMates can be moved to 
>> Reflection::areNestMates so that it's clearer the difference is the 
>> top-level enclosing class check.
>
> Reflection.areNestMates is a native method. Any fast-paths belong in 
> the callers ie VerifyAccess.areNestMates, or 
> Reflection.verifyMemberAccess - and indeed they do exist there.
>
> If you are concerned about confusion between VerifyAccess.areNestMates 
> and Reflection.areNestMates I can return to using 
> VerifyAccess.isSamePackageMember - though I find that a very poor name 
> to begin with and keep writing it the wrong way round. 
> (isInSamePackageMember would be more accurate.)

It's okay to leave it as is.   Two same method names doing different 
check does cause some confusion but I think the real confusion to me is 
that core reflection does not match the language access check in this case.

>
> I've reverted to -source/-target and hope that will not induce an API 
> check (though will probably produce a warning). If it does do the API 
> check then I'll have to factor the test classes into a separate file 
> as you say.
>
> Updated webrev: http://cr.openjdk.java.net/~dholmes/8196320/webrev.v2/
>
Looks okay to me,

Mandy