[lworld] RFR: 8357373: [lworld] jdk/valhalla/valuetypes/ObjectMethods.java fails with -XX:+UseAtomicValueFlattening [v2]

[Roger Riggs]

On Fri, 5 Dec 2025 08:43:40 GMT, Volkan Yazici <vyazici at openjdk.org> wrote:

>> Roger Riggs has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Correct check for identityHashCode
>
> test/jdk/valhalla/valuetypes/ValueObjectMethodsTest.java line 253:
> 
>> 251:         Line l3 = new Line(0, 9, 2, 3);
>> 252:         Line l4 = new Line(0, 1, 9, 3);
>> 253:         Line l5 = new Line(0, 1, 2, 9);
> 
> Random thought: Would it be a good idea to randomize the content to increase coverage over time? For example,
> 
>     var r = new java.util.Random();
>     var coordinates = Stream
>             .generate(() -> List.of(r.nextInt(), r.nextInt()))
>             .distinct()
>             .limit(4)
>             .toList();
>     int i = 0;
>     var p1 = new Point(coordinates.get(i).get(0), coordinates.get(i).get(1)); i++;
>     var p2 = new Point(coordinates.get(i).get(0), coordinates.get(i).get(1)); i++;
>     var p3 = new Point(coordinates.get(i).get(0), coordinates.get(i).get(1)); i++;
>     var p4 = new Point(coordinates.get(i).get(0), coordinates.get(i).get(1));

There is very little incremental value in that and it raises the complexity of the test, also needed more setup to document the random seed so it can be reproduced.
The test is intended to ensure that a change to each value individually causes a change in the hashCode.
Additional random values would do the same.

> test/jdk/valhalla/valuetypes/ValueObjectMethodsTest.java line 280:
> 
>> 278:     @MethodSource("hashcodeTests")
>> 279:     public void testHashCode(List<Object> objects) {
>> 280:         assertTrue(objects.size() > 1, "More than one object is required: " + objects);
> 
> _Nit:_ I doubt if displaying `[]` will contribute much to the diagnostics context.
> 
> Suggestion:
> 
>         assertFalse(objects.isEmpty(), "More than one object is required");

Every little bit helps.  
(`isEmpty` is not equivalent to `>1`)

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

PR Review Comment: https://git.openjdk.org/valhalla/pull/1771#discussion_r2592882965
PR Review Comment: https://git.openjdk.org/valhalla/pull/1771#discussion_r2592887565