[core-libs] RFR (L): 8010319: Implementation of JEP 181: Nest-Based Access Control

[mandy chung]

Hi David,

I reviewed:
http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.corelibs.v1/

Looks good in general.  I have a couple other comments.  I will review 
the new version that includes new tests in test/jdk/java/lang/reflect 
when it's ready.

Class.java

3988 Class<?>[] members = getNestMembers0();

If the above fails with any LinkageError, what is the expected behavior 
if Class::getNestMembers() is called the second time?  will it throw the 
same LinkageError?


On 5/20/18 10:57 PM, David Holmes wrote:
>
>> I think it's okay in VerifyAccess too but the "FIX ME" suggests there 
>> are doubts so I assume this needs more eyes to triple check.
>
> The "FIX ME" is not about the access check but the form of the 
> assertion. The assert is verification that resolution of a private 
> method always leads to selection of that private method: refc == defc. 
> I used "myassert" so that this check was always enabled during 
> testing. The "FIX ME" was to either convert to language assert 
> statement or else remove it (having validated that it never fires). 
> Obviously "myassert" has not fired in all the testing that I have done 
> so either choice seems fine. Do you have a preference? 

Since you have the assert, I would convert it to language assert that 
would help the readers to understand the result of such resolution.

Regarding Alan's and Peter's comment on the spec clarification on 
primitive type and array type, it is reasonable to take a pass on all 
reflection APIs and make the text explicit consistently if this class is 
a primitive type and array type.   I will file an issue to track this.   
Class::getModifiers does return the modifier of the component type 
whereas Class::getEnclosingClass returns null if this class is an array 
type even if the component type has an enclosing class.   Clarification 
would help developers.

Mandy