Hi, Mandy
On 9/5/19 6:03 PM, Mandy Chung wrote:
On 9/5/19 5:09 PM, Brent Christian wrote:
jvm.h
349 * Link the 'arg' class (unless ClassForNameDeferLinking is set)
I suggest to drop "(unless ...)" phrase because the flag is temporary.
Sure, good idea.
jni.cpp
The implementation of JNI FindClass has the same behavior as 1-arg
Class.forName(name). The JNI spec needs fixing but it's a separate
issue.
Right - looks like David filed JDK-8230685.
find_class_from_class_loader
I like David's suggestion to assert that if init == true, link
must be true. Alternatively, pass an enum instead of two booleans
clearly tell linking or initializing.
Yup, assert added.
Lookup::findClass:
"In particular, the method first attempts to load the requested class"
I think this sentence is no longer needed together with your change. What
about:
/**
- * Looks up a class by name from the lookup context defined by this
{@code Lookup} object. The static
- * initializer of the class is not run.
+ * Looks up a class by name from the lookup context defined by this
{@code Lookup} object.
+ * This method attempts to locate, load, and link the class, and then
determines whether
+ * the class is accessible to this {@code Lookup} object.
+ * The static initializer of the class is not run.
* <p>
* The lookup context here is determined by the {@linkplain
#lookupClass() lookup class}, its class
- * loader, and the {@linkplain #lookupModes() lookup modes}. In
particular, the method first attempts to
- * load the requested class, and then determines whether the class is
accessible to this lookup object.
+ * loader, and the {@linkplain #lookupModes() lookup modes}.
Looks good to me.
The note you added in this method should also be added to 2-arg
Class::forName for consistency.
OK, sure.
Update webrev is here:
http://cr.openjdk.java.net/~bchristi/8212117/webrev11/
with changes to jvm.h, MethodHandles.java, and Class.java.
I went ahead and put together a specdiff[1] for the changed methods
([2][3][4]). I've updated and finalized the CSR.
Thanks for the review,
-Brent
1.
http://cr.openjdk.java.net/~bchristi/8212117/webrev11-specdiff/specdiff-summary.html
2.
http://cr.openjdk.java.net/~bchristi/8212117/webrev11-specdiff/Class-report.html#forName(java.lang.String,boolean,java.lang.ClassLoader)
3.
http://cr.openjdk.java.net/~bchristi/8212117/webrev11-specdiff/Class-report.html#forName(java.lang.Module,java.lang.String)
4.
http://cr.openjdk.java.net/~bchristi/8212117/webrev11-specdiff/invoke/MethodHandles.Lookup-report.html#findClass(java.lang.String)
