On Mon, 1 Jun 2026 09:11:30 GMT, David Simms <[email protected]> wrote:

>> This is a "*sub-review pull request*" for the first 
>> [preview](https://openjdk.org/jeps/12) of [JEP 401: Value Classes and 
>> Objects](https://openjdk.org/jeps/401), specifically 
>> [JDK-8317279](https://bugs.openjdk.org/browse/JDK-8317279): Standard library 
>> implementation of value classes and objects 
>> 
>>> [!NOTE]
>>> This pull request and the other sub-review pull requests listed below are 
>>> based on the "*master pull request*" 
>>> (https://github.com/openjdk/jdk/pull/31120). It contains the same full set 
>>> of code changes as the "*master pull request*" to preserve the full 
>>> implementation context; the language compiler, JVM, and standard library 
>>> changes are intertwined. This separate pull requests exist only to 
>>> subdivide the review and related discussion by area.
>> 
>> Other areas for review:
>> 
>> - [JDK-8317277](https://bugs.openjdk.org/browse/JDK-8317277): Java language 
>> implementation of value classes and objects
>>   - https://github.com/openjdk/jdk/pull/31121
>> - [JDK-8317278](https://bugs.openjdk.org/browse/JDK-8317278): JVM 
>> implementation of value classes and objects
>>   - https://github.com/openjdk/jdk/pull/31122
>> 
>> Code changes resulting from the review process should be made in 
>> [`valhalla/lworld`](https://github.com/openjdk/valhalla/).
>> 
>> `valhalla/lworld` is currently updated from `jdk/master` whenever a weekly 
>> [`jdk` tag](https://github.com/openjdk/jdk/tags) is created. At that time, 
>> code changes from `valhalla/lworld` will be propagated to the master pull 
>> request and to all sub-review pull requests, including this one.
>> 
>> Ultimately, review sign-off will be recorded on the "*master pull request*", 
>> and this pull request will be closed without integration.
>> 
>> This pull request has a large code surface area and often conflicts with 
>> `jdk/master` on a daily basis. Refer to 
>> [`valhalla/lworld`](https://github.com/openjdk/valhalla/) for the latest 
>> state of the project code, keeping in mind that it may lag several days 
>> behind `jdk/master`. Both repositories may be needed as references during 
>> review.
>> 
>> ---------
>> - [x] I confirm that I make this contribution in accordance with the 
>> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai).
>
> David Simms has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains 2754 commits:
> 
>  - Merge remote-tracking branch 'valhalla/lworld' into 
> jep401_sub_review_8317279
>  - Merge
>    
>    Merge jdk-27+24
>  - 8385674: [lworld] TestNullableInlineTypes.java fails after JDK-8325632
>    
>    Reviewed-by: mchevalier
>  - 8385652: [lworld] RedefineClasses should use stack map frame name
>    
>    Reviewed-by: fparain
>  - 8384107: [lworld] Update runtime/contended tests to run the same testing 
> for value classes
>    
>    Reviewed-by: fparain, lmesnik
>  - 8385600: [lworld] DA/DU issues with strict fields
>    
>    Reviewed-by: vromero
>  - 8384897: [lworld] this.staticField should be restricted in early 
> construction context
>    
>    Reviewed-by: liach, vromero
>  - 8385601: [lworld] Update testing documentation for the ValueClassPlugin 
> jtreg option
>    
>    Reviewed-by: lmesnik
>  - 8385569: [lworld] Apply JDK-8343767 to Valhalla specific StubRoutines
>    
>    Reviewed-by: fparain, vlivanov
>  - 8385581: [lworld] Remove the experimental JVMCI feature
>    8382708: [lworld] JVMCI support for Value Objects
>    8372605: [lworld] 
> compiler/jvmci/jdk.vm.ci.runtime.test/src/jdk/vm/ci/runtime/test/TestResolvedJava*.java
>  fail with --enable-preview
>    
>    Reviewed-by: thartmann
>  - ... and 2744 more: https://git.openjdk.org/jdk/compare/2c7efc08...9e804255

src/java.base/share/classes/java/util/WeakHashMap.java line 575:

> 573:      * an {@linkplain IdentityException} is thrown when the first value 
> object
> 574:      * key is encountered. Zero or more mappings may have already been 
> copied to
> 575:      * this map.

API notes are for "commentary, rationale, or examples pertaining to the API". 
It might be better to make the first sentence be an implNote instead.

The second sentence is an allowed side effect when IdentityException is thrown 
so I think it needs to move into normative spec. Maybe the throws description 
could be expanded to say that it may be thrown after some mappings have been 
copied?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/31123#discussion_r3347819491

Reply via email to