RFR: JDK-8210031: implementation for JVM Constants API

[Mandy Chung]

On 12/3/18 11:12 AM, Vicente Romero wrote:
> Hi all,
>
> Can I have the final nod to the JVM constants API, there have been 
> some changes since the last review iteration. Thanks to the internal 
> and external developers that have taken the time to provide feedback 
> so far. The links to the last versions are:
>
> webrev: http://cr.openjdk.java.net/~vromero/8210031/webrev.10/
> javadoc: 
> http://cr.openjdk.java.net/~vromero/8210031/javadoc.18/overview-summary.html
> specdiff: 
> http://cr.openjdk.java.net/~vromero/8210031/specdiff.08/overview-summary.html
>

I reviewed webrev.10 and overall looks good to me.  A couple of minor 
comments and some of them seems to be fixed already in amber repo.  No 
need to generate a new webrev.

Nit: The javadoc of the new methods starts with "Returns", "Return", 
"Produce", "Create", "Resolve", "Compares" etc.  It'd be good to do a 
pass and fix the verb form consistently.

src/java.base/share/classes/java/lang/Class.java
    nit: Line 163 seems to have extra white-spaces, not aligned with the 
other superinterfaces.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/ClassWriterImpl.java 
74 private static final Set<String> suppressSubtypesSet
75 = Set.of("java.lang.Object",
76 "org.omg.CORBA.Object");

This is not related to your change. CORBA is no longer in JDK.
Maybe this special casing is no longer needed.  It may worth
filing a JBS issue to examine this.

ConstantDescs.java
   64     // Don't change the order of these declarations!

Is there any code depending on this order?

DirectMethodHandleDesc.java
   46  * {@link MethodHandle}.  A {@linkplain DirectMethodHandleDescImpl} corresponds to

typo: DirectMethodHandleDescImpl

   57     /**
   58      * Kinds of method handles that can be described with {@linkplain DirectMethodHandleDesc}.
   59      */
   60     enum Kind {

This needs @since 12

DynamicCallSiteDesc.java
   59      * @param bootstrapMethod a {@link DirectMethodHandleDescImpl} describing the
   60      *                        bootstrap method for the {@code invokedynamic}

typo: DirectMethodHandleDescImpl and in a few other @param

Mandy