On Wed, 14 Jan 2026 04:16:31 GMT, Chris Plummer <[email protected]> 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?

I'm not sure it makes much sense.
They are quite generic, I think about adding them to mainline in jdk 27 (but 
don't use by ObjectReferenceImpl.equals/hashCode)

> src/jdk.jdi/share/classes/com/sun/tools/jdi/ObjectReferenceImpl.java line 150:
> 
>> 148:     public boolean equals(Object obj) {
>> 149:         if (obj instanceof ObjectReferenceImpl other) {
>> 150:             if (!super.equals(obj)) { // checks if the references 
>> belong to the same VM
> 
> Have you considered adding a `this == obj` check at the beginning of this 
> method. I think usually we are dealing with the same heap object, so this 
> could be a quick check done for performance reasons.

If we decide to change this, it would be better to do it in mainline

> 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?

value class can be abstract.

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

PR Review Comment: 
https://git.openjdk.org/valhalla/pull/1834#discussion_r2692311793
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/1834#discussion_r2692513236
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/1834#discussion_r2692508079

Reply via email to