On Thu, 26 Jan 2023 21:46:04 GMT, Paul Sandoz <[email protected]> wrote:
>> Mandy Chung has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> review feedback
>
> src/java.base/share/classes/java/lang/Module.java line 607:
>
>> 605: * {@link java.lang.invoke.MethodHandles.Lookup Lookup} object that
>> can be used to
>> 606: * {@link java.lang.invoke.MethodHandles.Lookup#defineClass(byte[])
>> inject classes}
>> 607: * in package {@code p}. </p>
>
> Suggestion:
>
> * <p> A package {@code p} opened to module {@code M} means that code in
> * {@code M} is allowed to do deep reflection on all types in the package.
> * Further, if {@code M} reads this module it can obtain a
> * {@link java.lang.invoke.MethodHandles.Lookup Lookup} object that is
> allowed to
> * {@link java.lang.invoke.MethodHandles.Lookup#defineClass(byte[])
> define classes}
> * in package {@code p}. </p>
>
> Trying to reuse existing terms. I am presuming deep reflection implies on all
> members and setAccessible so there is no need to mention it?
>
> Also i don't see "inject" used in existing text, so just reuse "define"?
Using "define" is fine too.
This method links to `AccessibleObject.setAccessible` and
`MethodHandles.privateLookupIn`. It may be adequate and no need to mention
more.
> src/java.base/share/classes/java/lang/invoke/MethodHandles.java line 883:
>
>> 881: * of {@code T}. Extreme caution should be taken when opening a
>> package
>> 882: * to another module. The injected classes have the same full
>> privilege
>> 883: * access as other members in the target module.
>
> Suggestion:
>
> * <p>
> * The {@code Lookup} object returned by {@code privateLookupIn} is
> allowed to
> * {@linkplain Lookup#defineClass(byte[]) define classes} in the runtime
> package
> * of {@code T}. Extreme caution should be taken when opening a package
> * to another module as such defined classes have the same full privilege
> * access as other members in the target module.
>
>
> You mention "target module" but i don't think i it is defined for the Lookup
> class JavaDoc. In this case are we referring to module M2?
yes, M2. Updated.
-------------
PR: https://git.openjdk.org/jdk/pull/12236
