review comments for JEP 12: Preview Language and VM Features JDK-8195734

[Alex Buckley]

On 12/10/2018 7:54 AM, Karen Kinnear wrote:
> 1. As discussed in  https://bugs.openjdk.java.net/browse/JDK-8198908
> <https://bugs.openjdk.java.net/browse/JDK-8198908> the following
> paragraph is not-implementable: "If a class file's use of a preview VM
> feature of Java SE $N causes an exception during loading, linking, or
> execution, then the JVM implementation must indicate that the
> exception is due to an preview VM feature of Java SE $N."
>
> As discussed in the bug, we have added off-by-default logging when
> loading a class that supports a preview version. I don’t believe we
> want to mandate the details of diagnostic information. In particular,
> the JVM does not have always have the information to detect that the
> root cause of an exception lies in using a preview VM feature. So I
> would recommend removing this.

Fair comment. The paragraph quoted above was intended to align with this 
earlier paragraph in JEP 12:

"If a compile-time error occurs due to the incorrect use of a preview 
language feature of Java SE $N, then javac may indicate that the error 
is associated specifically with a preview language feature."

The intent was to be noisy anywhere and everywhere that a preview 
feature was used or misused. That said, if javac gets a "may indicate", 
then HotSpot should not get a "must indicate". For consistency, and to 
clarify that an implementation is welcome to provide preview-related 
diagnostics, I will say:

"If a class file's use of a preview VM feature of Java SE $N causes an 
exception during loading, linking, or execution, then the JVM 
implementation ***may*** indicate that the exception is due to an 
preview VM feature of Java SE $N."

Few things:

1. I see in JDK-8198908 that `-Xlog:class+preview` is supported so I 
will add the following:

"The JVM implementation in JDK $N has an off-by-default ability to show 
which loaded classes depend on the preview features of Java SE $N. To 
enable the ability, pass `-Xlog:class+preview` to the `java` launcher."

2. Is  the [class,preview] output predicated on --enable-preview being 
given in addition to -Xlog:class+preview ?

3. The wording of the logging output is "Loading preview feature type 
$NAME". That sounds like the type is part of the preview feature, i.e., 
involved in supporting the preview feature, e.g., the 
java.lang.AutoCloseable type if the try-with-resources statement had 
been a preview feature. The word "type" also seems unusually abstract 
for logging output. Recommend "Loading class file that depends on 
preview features: $NAME", or, if it's redundant to say "Loading class 
file" in a log about class loading, then "$NAME depends on preview 
features of Java SE $RELEASE_THAT_CORRESPONDS_TO_MAJOR_VERSION"

> 2. Relationship to Java SE APIs 2nd paragraph: A preview language or
> VM feature must not rely on an incubating API, since the API is not
> part of the Java SE Platform.
>
> 4th paragraph: The implementation of a preview language or VM feature
> may rely on incubating APIs. (and similarly 5th paragraph).
>
> Unless I misread this, this is inconsistent, unless there is a subtle
> distinction between the feature and a specific implementation.

This is in "Relationship to Incubating APIs", not "Relationship to Java 
SE APIs". The intent was to distinguish the design of a preview feature 
from its implementation. Most times when the JEP speaks of a preview 
feature, it means the design. That said, the JEP is usefully careful to 
say "Java compiler" or "JVM implementation" when speaking of the 
implementation, and I will clarify the fourth paragraph to be careful.

Alex