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

[Karen Kinnear]

> On Dec 10, 2018, at 3:00 PM, Alex Buckley <alex.buckley at oracle.com> wrote:
> 
> 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.”
Thank you.
> 
> 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.”
Your call what you want in the JEP. A note - this is hotspot-specific JVM flag, not a launcher flag, so other
implementations are unlikely to choose this approach.
> 
> 2. Is  the [class,preview] output predicated on --enable-preview being given in addition to -Xlog:class+preview ?
Yes. Otherwise the class would fail to load and you would get an UnsupportedClassVersionError and in hotspot an error message mentioned Preview features are not enabled, ...
> 
> 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”
Agree with the need for improvement - Harold will respond with a specific textual proposal.
You do not always have a context of already knowing you are loading a class file.
> 
>> 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.
Thank you both for the correction to my note and the clarification. That is a valuable distinction.

thanks,
Karen
> 
> Alex