Skip to content

Conversation

@AditiS11
Copy link

@AditiS11 AditiS11 commented Nov 25, 2025

Set the J9ClassHasIdentity flag only if the class is
not a value class and not an interface.

Fixes the following failure in test/jdk/valhalla/valuetypes/ValueClassTest.java

[14:01:06.476] STARTED    ValueClassTest::testIsValueObjectCompatible 'testIsValueObjectCompatible()'
org.opentest4j.AssertionFailedError: interface: java.lang.Comparable ==> expected: <true> but was: <false>
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
	at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:188)
	at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:1167)
	at ValueClassTest.isValueObjectCompatibleCase(ValueClassTest.java:63)
	at ValueClassTest.testIsValueObjectCompatible(ValueClassTest.java:51)
	at java.base/java.lang.reflect.Method.invoke(Method.java:571)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1604)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1604)

@AditiS11 AditiS11 changed the title Fix J9HasIdentity flag assignment Fix J9ClassHasIdentity flag assignment Nov 25, 2025
Set the J9ClassHasIdentity flag only if the class is
not a value class and not an interface.
@theresa-m theresa-m self-requested a review November 25, 2025 13:10
@theresa-m theresa-m added comp:vm project:valhalla Used to track Project Valhalla related work labels Nov 25, 2025
#endif /* defined(J9VM_OPT_VALHALLA_VALUE_TYPES) */
} else if (!(J9_IS_CLASSFILE_OR_ROMCLASS_VALUETYPE_VERSION(romClass) && J9ROMCLASS_IS_INTERFACE(romClass))) {
/* All classes before value type version are identity. */
} else if (!J9ROMCLASS_IS_VALUE(romClass) && !J9ROMCLASS_IS_INTERFACE(romClass)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can remove the interface check here. J9ROMCLASS_IS_VALUE is only true when an interface flag is not set.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

J9ROMCLASS_IS_VALUE is false for interfaces. If I remove the interface check, J9ROMCLASS_IS_VALUE will return false for interfaces and enters the else if block and sets J9ClassHasIdentity flag for interfaces too.

@theresa-m theresa-m requested a review from hangshao0 November 26, 2025 14:41
#endif /* defined(J9VM_OPT_VALHALLA_VALUE_TYPES) */
} else if (!(J9_IS_CLASSFILE_OR_ROMCLASS_VALUETYPE_VERSION(romClass) && J9ROMCLASS_IS_INTERFACE(romClass))) {
/* All classes before value type version are identity. */
} else if (!J9ROMCLASS_IS_VALUE(romClass) && !J9ROMCLASS_IS_INTERFACE(romClass)) {
Copy link
Contributor

@hangshao0 hangshao0 Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the original code, if Valhalla JDK is running on an older version of class file (e.g. Java 21), J9ClassHasIdentity is set on interface. With this change, J9ClassHasIdentity won't be set.

Have you checked the return value of java.lang.Class.isIdentity() with OpenJDK Valhalla build if given a interface compiled by Java 21 ?

@AditiS11 AditiS11 marked this pull request as draft November 28, 2025 11:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp:vm project:valhalla Used to track Project Valhalla related work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants