[lworld] RFR: 8373202: [lworld] ObjectReference.equals should follow == semantics for value objects

[Chris Plummer]

On Thu, 18 Dec 2025 20:00:45 GMT, Alex Menkov <amenkov at openjdk.org> wrote:

> Updated implementation of ObjectReference.equals and ObjectReference.hashCode to comply the spec for value objects.
> Added the test for value object ctor debugging, the test verifies the behaviour is expected.
> There is an issue with instance filter, it till be fixed separately (it's not yet clear how it would be better to fix it)
> 
> testing: tier1..4, hs-tier5-svc

src/java.se/share/data/jdwp/jdwp.spec line 1804:

> 1802:         )
> 1803:     )
> 1804:     (Command IsSameObject=11

Are you going to add "preview API" warnings to these two APIs?

src/jdk.jdi/share/classes/com/sun/tools/jdi/ObjectReferenceImpl.java line 171:

> 169:     public int hashCode() {
> 170:         try {
> 171:             return JDWP.ObjectReference.ObjectHashCode.process(vm, this).hashCode;

Don't we want to limit the slow path to value objects?

src/jdk.jdi/share/classes/com/sun/tools/jdi/ReferenceTypeImpl.java line 334:

> 332: 
> 333:     // Inline type is a concrete value class.
> 334:     boolean isInlined() {

Why is this called isInlined()? Shouldn't it be isValueClass()? Do we want to expose it to users through ReferenceType? Should we also consider exposing a new isIdentity() API?

src/jdk.jdi/share/classes/com/sun/tools/jdi/ReferenceTypeImpl.java line 341:

> 339:         return ((modifiers & VMModifiers.IDENTITY) == 0)
> 340:             && ((modifiers & VMModifiers.INTERFACE) == 0)
> 341:             && ((modifiers & VMModifiers.ABSTRACT) == 0);

ReferenceTypeImpl has isXXX() apis to check these modifiers. We should probably use them here and add isIdentity().

It's unclear to me why you have to check INTERFACE and ABSTRACT. Isn't IDENTITY enough, or do interfaces and abstract classes not have the IDENTITY bit set?

test/jdk/com/sun/jdi/valhalla/CtorDebuggingTest.java line 69:

> 67: import com.sun.jdi.request.EventRequestManager;
> 68: 
> 69: public class CtorDebuggingTest extends TestScaffold {

I think this test would benefit from more comments. It would be good to describe all the scenarios being tested, probably in one main comment block early on, and then for implementation of each scenario add a one line comment indicating which scenarios is being tested.

test/jdk/com/sun/jdi/valhalla/CtorDebuggingTest.java line 129:

> 127:         } catch (IncompatibleThreadStateException ex) {
> 128:             System.out.println("ERROR: cannot get 'this' object: " + ex);
> 129:             return null;

It seems this just results in the caller getting an NPE. Wouldn't it be better to just throw a RuntimeException here?

test/jdk/com/sun/jdi/valhalla/CtorDebuggingTest.java line 262:

> 260:         }
> 261: 
> 262:         // Resumes the debuggee and go through all breackpoints until the location specified is reached.

Suggestion:

        // Resumes the debuggee and goes through all breakpoints until the location specified is reached.

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

PR Review Comment: https://git.openjdk.org/valhalla/pull/1834#discussion_r2688880160
PR Review Comment: https://git.openjdk.org/valhalla/pull/1834#discussion_r2688888544
PR Review Comment: https://git.openjdk.org/valhalla/pull/1834#discussion_r2688904914
PR Review Comment: https://git.openjdk.org/valhalla/pull/1834#discussion_r2688899731
PR Review Comment: https://git.openjdk.org/valhalla/pull/1834#discussion_r2688918832
PR Review Comment: https://git.openjdk.org/valhalla/pull/1834#discussion_r2688915482
PR Review Comment: https://git.openjdk.org/valhalla/pull/1834#discussion_r2688911651