Re: RFR: 6378384 (reflect) subclass can’t access superclass’s protected fields and methods by reflection

[Peter Levart]

Hi Mandy,

Thanks for looking into this patch.

On 10/15/2016 11:31 PM, Mandy Chung wrote:
> Hi Peter,
>
>> On Oct 2, 2016, at 2:51 PM, Peter Levart <peter.levart at gmail.com> wrote:
>>
>> http://cr.openjdk.java.net/~plevart/jdk9-dev/6378384_Reflection.ensureAccess/webrev.02/
> This change looks good to me.  Thanks for adding the test enumerating exhaustive combinations.  I ran JCK tests and verified that no new failure.
>
> With this change, sun.reflect.misc.ReflectUtil.ensureMemberAccess could be removed when Atomic*FieldUpdater are updated to use Reflection::ensureMemberAccess directly. As of now, we will keep ReflectUtil::ensureMemberAccess.
>
> AccessControlTest.java is a great test.  I wonder if AccessControlTest.java could be made less verbose and make it easier to read.  For example, can we add a builder class that takes either the list of allowed or denied MemberFactory  (but not both) to reduce the verbosity.

I think specifying both is more verbose on one hand, but OTOH it allows 
one to visually inspect all the cases and think of each and every one in 
isolation to see if it is valid in a particular caller/member/target 
arrangement. The test infrastructure verifies that the test case covers 
all the MemberFactory(s) so one must only verify each individual allowed 
/ denied MemberFactory.

>     Builder::allowAll and Builder::denyAll would be useful.  allowAccessMember of a specific modifier can imply both field and method.

This is a good idea. Here's a modified test (no changes to patched 
files, just tests):

http://cr.openjdk.java.net/~plevart/jdk9-dev/6378384_Reflection.ensureAccess/webrev.03/


Is it easier on the eye now?

I also added Copyright headers to all the test sources.


Regards, Peter