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