> On Oct 17, 2016, at 4:30 PM, Peter Levart <peter.lev...@gmail.com> wrote:
>
> ...you would review the members that are allowed and then you should ask
> yourself: "Which are the ones that are denied? All the rest. What are they?",
> or: "Could there be any other that should be allowed? Which one?”
I’m happy with your revised patch that group field/method together and adding
allowAll/denyAll that makes it easier to understand while it does not lose any
information.
> It's much easier if they are explicitly listed:
>
> ok &= new Test()
>
> .current("b.PublicSub").member("a.PublicSuper").target("b.PublicSub")
> .allowed(PROTECTED_INSTANCE_F_M, PUBLIC_INSTANCE_F_M,
> PROTECTED_STATIC_F_M,
> PUBLIC_STATIC_F_M, PUBLIC_C)
> .denied (PRIVATE_INSTANCE_F_M, PACKAGE_INSTANCE_F_M,
> PRIVATE_STATIC_F_M,
> PACKAGE_STATIC_F_M, PRIVATE_C, PACKAGE_C,
> PROTECTED_C)
> .perform();
>
>
> And besides, you don't really have to review them all.
I do review them as this test is one important part of this patch :)
> The fact that running the test on unpatched JDK 9 finds just two differences:
>
> - access to protected static method from subclass in another package
> - access to protected static field from subclass in another package
>
> ...is a reassurance that the patch does exactly what it should. No less,
> no more.
>
Indeed.
>
> Then I would have to have a mapping from class name -> constant name for the
> generator.
This is one time thing.
> Besides, constant names would not be any prettier than class name string
> literals. At least now it is obvious to anyone what package a particular
> class belongs to:
>
> "a.Package" vs. A_PACKAGE ?
>
PACKAGE_CLASS_IN_PKG_A
PUBLIC_SUPERCLASS_IN_PKG_A
PUBLIC_SUBCLASS_IN_PKG_A
> Note that I can't use a.Package.class literal(s) here (thought I would like
> to) as they don't compile if they refer to a package-private class from
> another package.
>
> I would like to keep those things unchanged, If you don't mind.
Do the suggested variable names help?
Mandy
