On 4/18/20 3:28 PM, Mandy Chung wrote:
Hi Chris, Serguei,

On 4/18/20 12:18 AM, Chris Plummer wrote:

Yes. I'd like to see all this as part of the Class/Classloading spec in some sort of section that gives an overview of all these topics, but mostly clarifies what an initiating loader is, and the (non) relationship to hidden classes.

We should refer initiating loader and class loading from JVMS 5.3.   JVM TI needs the clarification w.r.t.  GetClassLoaderClasses that does not include hidden classes because the initiating class loader cannot find it.

GetLoadedClasses is about class creation.   While it seems tempted to put the descriptions in some sort of overview section, I found the clarification is specific for each function and hence inlining them in each function helps the readers directly see the difference.

I made a few changes that should ease your concern of duplication:
http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.06-svc-spec-changes.01/

- The class creation description added in GetLoadedClasses.  Not in JDWP, JDI and Instrumentation. - The description in GetClassLoaderClasses, Instrumentation::getInitiatedClasses, ClassLoaderReference::visibleClasses is revised to take out the details.   Add links from JDWP and JDI to GetClassLoaderClasses. - The details about hidden classes can be found from `Class::isHidden` and      `Lookup::defineHiddenClass`.

Mandy
Hi Mandy,

This looks much better. Thank you for the consolidating and the cross references. I think that's a big improvement.

One minor thing I noticed is a few "JNI-style signature" references. In LocalVariable.java you renamed "JNI signature" to "JNI-style signature", but in JDI Mirror.java you renamed "JNI-style signature" to "type signature". And then in JDI TypeComponent.java you left the "JNI-style signature" reference unchanged. Why the inconsistency?

In VirtualMachine.java:

 145      * @see Class or interface creation specified in
 146      * <a href="{@docRoot}/../specs/jvmti/jvmti.html#GetLoadedClasses">JVM TI GetLoadedClasses</a>

How about:

 145      * @a description of how Class and interface creation can be triggered is specified in  146      * <a href="{@docRoot}/../specs/jvmti/jvmti.html#GetLoadedClasses">JVM TI GetLoadedClasses</a>

I read the following incorrectly a few times before I finally understood the meaning:

  44     /**
  45      * Returns the {@linkplain com.sun.jdi.Type#name() name of the class}
  46      * that has been unloaded. The returned string may not be a
  47      * <a href="${docRoot}/java.base/java/lang/ClassLoader.html#binary-name">binary name</a>.
  48      *
  49      * @see Class#getName()
  50      */
  51     public String className();
  52
  53     /**
  54      * Returns the {@linkplain com.sun.jdi.Type#signature() type signature of the class}   55      * that has been unloaded.  The returned string may not be a type descriptor
  56      * specified in JVMS {@jvms 4.3.2}.
  57      *
  58      * @see java.lang.invoke.TypeDescriptor#descriptorString()
  59      */
  60     public String classSignature();

"may not be" through me off. I read it as "is not allowed not be". Your intent was "might not be", which might be a better choice of words.

thanks,

Chris

Reply via email to