Hi Mandy,
Don't count me as reviewer; this is only editorial:
In MethodHandles.java:
============
165 * A caller, specified as a {@code Lookup} object, is in
module {@code M1} is
166 * allowed to do deep reflection on module {@code M2} and
package of the target class
167 * if and only if all of the following conditions are {@code
true}:
There are two 'is' in this sentence, which makes it a bit difficult
to parse. Should the first be removed?
* A caller, specified as a {@code Lookup} object, in module {@code M1}
is ...
============
169 * <li>If there is a security manager, its {@code
checkPermission} method is
170 * called to check {@code
ReflectPermission("suppressAccessChecks")} and
171 * that must return normally.
172 * <li>
173 * If there is a security manager, its {@code checkPermission}
method is
174 * called to check {@code
ReflectPermission("suppressAccessChecks")}.
175 * This method must return normally.
Probably one of these two bullets should be removed: they look
the same to me.
=============
585 * A {@code Lookup} with {@link #PUBLIC} mode and a lookup
class in {@code M1}
586 * can access public types in a module {@code M2} when {@code
M2} is readable to
587 * {@code M1} when the type is in a package of {@code M2} that
is exported to at least {@code M1}.
^^^^
I believe there is "and" missing here:
<<... when M2 is readable to M1 *and* when the type ....>>
===========
Is the specdiff report incomplete? I was hoping to see the table
added to the Lookup class level API under
582 * <h1><a id="cross-module-lookup"></a>Cross-module lookups</h1>
but it seems to be missing from the specdiff?
BTW: is <h1> the right header? I thought <h1> was reserved for the
class name. John Gibbon will know more ;-)
best regards,
-- daniel
On 02/07/2019 03:47, Mandy Chung wrote:
On 7/1/19 12:51 PM, Mandy Chung wrote:
This is an enhancement to |`Lookup::in`| and
|`MethodHandles::privateLookupIn`| API
for cross module teleporting. A `Lookup` object will record the previous
lookup class from which this |Lookup| object was teleported such that
the access check will use both the previous lookup class and the current
|lookup| context (current lookup class and allowed modes) to determine if
a type is accessible to this `Lookup` object or not.
In a nutshell, `T` in M2 is accessible to a `Lookup` object on `C`
(lookup class in M1) and `PLC` (previous lookup class in M0) if and
only if
1. both M0 and M1 can read M2
2. T is in a package that is exported from M2 at least to both M0 and M1
Detailed specification is in Lookup class spec and `accessClass` javadoc.
The relevant spec about cross-module teleporting is in the Lookup class
spec and `Lookup::in` and `MethodHandles::privateLookupIn`.
CSR: https://bugs.openjdk.java.net/browse/JDK-8226916
webrev:
http://cr.openjdk.java.net/~mchung/jdk14/8173978/webrev.00
javadoc:
http://cr.openjdk.java.net/~mchung/jdk14/8173978/javadoc/java.base/java/lang/invoke/MethodHandles.Lookup.html
http://cr.openjdk.java.net/~mchung/jdk14/8173978/javadoc/java.base/java/lang/invoke/MethodHandles.html#privateLookupIn(java.lang.Class,java.lang.invoke.MethodHandles.Lookup)
I have yet to generate the spec diff. The tool is currently broken
due to javadoc change. I'll try to workaround it and post the
spec diff soon.
specdiff:
http://cr.openjdk.java.net/~mchung/jdk14/8173978/specdiff/overview-summary.html
Mandy