RFR: JDK-8210031: implementation for JVM Constants API
Mandy Chung
mandy.chung at oracle.com
Wed Dec 5 18:12:13 UTC 2018
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
More information about the amber-dev
mailing list