On Mon, 29 Jun 2026 15:04:12 GMT, Chen Liang <[email protected]> wrote:

>> Reflective preview APIs can be used out of preview. Thus it is meaningful 
>> for us to note how they behave when preview features are disabled.
>> 
>> I hope this can clarify some common misconceptions and help onboarding to 
>> the JEP 401 reflection model.
>> 
>> ---------
>> - [x] I confirm that I make this contribution in accordance with the 
>> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai).
>
> Chen Liang has updated the pull request with a new target base due to a merge 
> or a rebase. The incremental webrev excludes the unrelated changes brought in 
> by the merge/rebase. The pull request contains six additional commits since 
> the last revision:
> 
>  - Merge branch 'lworld' of https://github.com/openjdk/valhalla into 
> doc/lw-reflect-preview-api-0
>  - Merge branch 'lworld' of https://github.com/openjdk/valhalla into 
> doc/lw-reflect-preview-api-0
>  - Merge branch 'lworld' of https://github.com/openjdk/valhalla into 
> doc/lw-reflect-preview-api-0
>  - Reword recommendations
>  - Make requireIdentity reflective
>  - Reflective preview API behavior out of preview

src/java.base/share/classes/java/lang/Class.java line 639:

> 637:      * The {@code ACC_IDENTITY} flag is considered always set for a 
> class that
> 638:      * does not use preview features; consequently, this method always 
> returns
> 639:      * {@code false} when preview features are disabled.

Good.

src/java.base/share/classes/java/lang/Class.java line 1380:

> 1378:      *      the component type is an interface
> 1379:      * <li> when preview features are enabled, its {@link
> 1380:      *      AccessFlag#IDENTITY identity} modifier is always true

As it's now a link then maybe "identity" should be "IDENTITY".  I realize the 
other non-link usages are "identity".

src/java.base/share/classes/java/lang/Class.java line 1392:

> 1390:      *      <div class="preview-comment">
> 1391:      *          When preview features are enabled, if this {@code 
> Class} object
> 1392:      *          represents a class whose {@code class} file does not 
> use preview

It might be a bit smooth to start with "When preview features are enabled and 
this Class object ..."

src/java.base/share/classes/java/lang/Class.java line 1411:

> 1409:      *          enabled, a status not accessible to programs.  The 
> preferred way
> 1410:      *          to check for the identity status is to use {@link 
> #isValue()
> 1411:      *          Class.isValue()}.

I think it would be better to drop "a status not accessible to programs", it 
will just anger developers.

Can the second sentence just use "Use the isValue method to test if a value 
class or identity class"?

src/java.base/share/classes/java/lang/Class.java line 1418:

> 1416:      *          whether preview features are enabled.
> 1417:      *          {@snippet lang=java :
> 1418:      *          !clazz.isPrimitive() && !clazz.isValue() && 
> !clazz.isInterface()

Good idea to include this snippet as it is useful to have.

src/java.base/share/classes/java/lang/classfile/attribute/StackMapFrameInfo.java
 line 89:

> 87:      * unset fields, the {@code class} file must declare it uses preview
> 88:      * features.  The list of unset fields is always empty for a stack 
> map frame
> 89:      * declared in a {@code class} file that does not use preview 
> features.

It might be a bit more readable to swap the sentences so that it's initially 
clear that the list is always empty when they class does not use preview 
features. It may be non-empty when the class uses preview features. Just a 
suggestion as I had to read the paragraph a few times to digest it.

src/java.base/share/classes/java/lang/reflect/Field.java line 293:

> 291:      * ACC_STRICT_INIT} flag is considered not set for a field declared 
> in a
> 292:      * class or interface that does not use preview features; 
> consequently,
> 293:      * this method always returns {@code false} when preview features 
> are disabled.

Good, this is an improvement on what we had previously.

src/java.base/share/classes/java/util/Objects.java line 203:

> 201:      * features are enabled.  Value objects do not exist when preview 
> features
> 202:      * are disabled; consequently, this method behaves the same as {@link
> 203:      * #nonNull Objects.nonNull} when preview features are disabled.

"Value objects do not exist when preview features are disabled".  What would 
you think about just saying that "All objects are identity objects when preview 
features are disabled"?

src/java.base/share/classes/java/util/Objects.java line 238:

> 236:      * @since 28
> 237:      */
> 238:     @PreviewFeature(feature = PreviewFeature.Feature.VALUE_OBJECTS, 
> reflective = true)

Changing the requireIdentity methods to be reflective preview APIs looks okay, 
as is the update to their docs.  My guess is that the libraries that want to 
get a head start will access the preview APIs with core reflection so they 
won't care if the methods are just normal preview APIs.

-------------

PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2578#discussion_r3505435606
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2578#discussion_r3505387829
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2578#discussion_r3505447073
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2578#discussion_r3505489339
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2578#discussion_r3505493335
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2578#discussion_r3505411564
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2578#discussion_r3505414383
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2578#discussion_r3505431788
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2578#discussion_r3505398043

Reply via email to