On Fri, 26 Jun 2026 06:39:29 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-8317278](https://bugs.openjdk.org/browse/JDK-8317278): JVM 
>> 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-8317279](https://bugs.openjdk.org/browse/JDK-8317279): Standard 
>> library implementation of value classes and objects
>>   - https://github.com/openjdk/jdk/pull/31123
>> 
>> 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 2859 commits:
> 
>  - Merge branch '8317277' into jep401_sub_review_8317278
>  - Merge remote-tracking branch 'valhalla/lworld' into 8317277
>  - Merge
>    
>    Merge jdk-28+4
>  - 8386963: [lworld] Improve the exception message from Object 
> synchronization methods on value objects
>    
>    Reviewed-by: dholmes, alanb
>  - 8387300: [lworld] Minor review comments in javac
>    
>    Reviewed-by: vromero
>  - 8387192: [lworld] Review comment drop for core libs
>    
>    Reviewed-by: jvernee, vromero
>  - 8386999: [lworld] C2: assert(is_dead_loop_safe()) failed: shouldn't be 
> cleared yet
>    
>    Reviewed-by: qamai, vlivanov
>  - 8386787: [lworld] 
> compiler/valhalla/inlinetypes/TestValueConstruction.java#StressIncrementalInliningDontInlineMyAbstractInit
>  timed out
>    
>    Reviewed-by: phubner, chagedorn
>  - 8386995: [lworld] Duplicate value classes are a preview feature warning
>    
>    Reviewed-by: alanb, vromero
>  - 8383389: [lworld] Augment AOTMapLogger::print_oop_details to support flat 
> arrays with oops
>    
>    Reviewed-by: iklam, fparain
>  - ... and 2849 more: https://git.openjdk.org/jdk/compare/193de1b1...cffcfb57

src/hotspot/share/c1/c1_GraphBuilder.cpp line 1906:

> 1904:         if (field_type->is_loaded() && field_type->is_inlinetype() && 
> field_type->as_inline_klass()->is_empty() &&
> 1905:             (!method()->is_class_initializer() || field->is_flat())) {
> 1906:           // Storing to a field of an empty, null-free inline type that 
> is already initialized. Ignore.

This code pattern and comment occur twice in C1 and once in C2, but the comment 
is confusing.  It is the field type that is empty, not the field's 
holder/container.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/31122#discussion_r3494760154

Reply via email to