Hi Peter,

> On Oct 2, 2016, at 2:51 PM, Peter Levart <peter.lev...@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.   Builder::allowAll and 
Builder::denyAll would be useful.  allowAccessMember of a specific modifier can 
imply both field and method.

The builder will generate a test case that allowed and denied sets are distinct 
and cover the full set of MemberFactory.

Mandy

Reply via email to