Skip to content

8363846: [lworld] Make Class.isIdentityClass() non-native #1514

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 9 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/hotspot/share/oops/instanceKlass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3593,7 +3593,7 @@ bool InstanceKlass::find_inner_classes_attr(int* ooff, int* noff, TRAPS) const {

void InstanceKlass::check_can_be_annotated_with_NullRestricted(InstanceKlass* type, Symbol* container_klass_name, TRAPS) {
assert(type->is_instance_klass(), "Sanity check");
if (type->access_flags().is_identity_class()) {
if (type->is_identity_class()) {
ResourceMark rm(THREAD);
THROW_MSG(vmSymbols::java_lang_IncompatibleClassChangeError(),
err_msg("Class %s expects class %s to be a value class, but it is an identity class",
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/oops/klass.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -729,7 +729,7 @@ class Klass : public Metadata {
bool is_interface() const { return _access_flags.is_interface(); }
bool is_abstract() const { return _access_flags.is_abstract(); }
bool is_synthetic() const { return _access_flags.is_synthetic(); }
bool is_identity_class() const { return _access_flags.is_identity_class(); }
bool is_identity_class() const { assert(is_instance_klass(), "only for instanceKlass"); return _access_flags.is_identity_class(); }
void set_is_synthetic() { _access_flags.set_is_synthetic(); }
bool has_finalizer() const { return _misc_flags.has_finalizer(); }
void set_has_finalizer() { _misc_flags.set_has_finalizer(true); }
Expand Down
15 changes: 1 addition & 14 deletions src/hotspot/share/prims/jvm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ static void validate_array_arguments(Klass* elmClass, jint len, TRAPS) {
THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(), "Array length is negative");
}
elmClass->initialize(CHECK);
if (elmClass->is_identity_class()) {
if (elmClass->is_array_klass() || elmClass->is_identity_class()) {
THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(), "Element class is not a value class");
}
if (elmClass->is_abstract()) {
Expand Down Expand Up @@ -1389,19 +1389,6 @@ JVM_ENTRY(jboolean, JVM_IsHiddenClass(JNIEnv *env, jclass cls))
return k->is_hidden();
JVM_END

JVM_ENTRY(jboolean, JVM_IsIdentityClass(JNIEnv *env, jclass cls))
oop mirror = JNIHandles::resolve_non_null(cls);
if (java_lang_Class::is_primitive(mirror)) {
return JNI_FALSE;
}
Klass* k = java_lang_Class::as_Klass(mirror);
if (EnableValhalla) {
return k->is_array_klass() || k->is_identity_class();
} else {
return k->is_interface() ? JNI_FALSE : JNI_TRUE;
}
JVM_END

class ScopedValueBindingsResolver {
public:
InstanceKlass* Carrier_klass;
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/prims/unsafe.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,7 @@ UNSAFE_ENTRY(jarray, Unsafe_NewSpecialArray(JNIEnv *env, jobject unsafe, jclass
if (len < 0) {
THROW_MSG_NULL(vmSymbols::java_lang_IllegalArgumentException(), "Array length is negative");
}
if (klass->is_identity_class()) {
if (klass->is_array_klass() || klass->is_identity_class()) {
THROW_MSG_NULL(vmSymbols::java_lang_IllegalArgumentException(), "Element class is not a value class");
}
if (klass->is_abstract()) {
Expand Down
10 changes: 9 additions & 1 deletion src/java.base/share/classes/java/lang/Class.java
Original file line number Diff line number Diff line change
Expand Up @@ -624,7 +624,15 @@ public static Class<?> forName(Module module, String name) {
* @since Valhalla
*/
@PreviewFeature(feature = PreviewFeature.Feature.VALUE_OBJECTS, reflective=true)
public native boolean isIdentity();
public boolean isIdentity() {
if (isPrimitive()) {
return false;
} else if (PreviewFeatures.isEnabled()) {
return isArray() || Modifier.isIdentity(modifiers);
} else {
return !isInterface();
}
}

/**
* {@return {@code true} if this {@code Class} object represents a value
Expand Down
1 change: 0 additions & 1 deletion src/java.base/share/native/libjava/Class.c
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ static JNINativeMethod methods[] = {
{"getSuperclass", "()" CLS, NULL},
{"getInterfaces0", "()[" CLS, (void *)&JVM_GetClassInterfaces},
{"isHidden", "()Z", (void *)&JVM_IsHiddenClass},
{"isIdentity", "()Z", (void *)&JVM_IsIdentityClass},
{"getDeclaredFields0","(Z)[" FLD, (void *)&JVM_GetClassDeclaredFields},
{"getDeclaredMethods0","(Z)[" MHD, (void *)&JVM_GetClassDeclaredMethods},
{"getDeclaredConstructors0","(Z)[" CTR, (void *)&JVM_GetClassDeclaredConstructors},
Expand Down
79 changes: 79 additions & 0 deletions test/jdk/valhalla/valuetypes/IsIdentityClassTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
/*
* Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/

/*
* @test
* @summary Test that IsIdentityClass and modifiers return true for arrays that can be flattened.
* @library /test/lib
* @enablePreview false
* @modules java.base/jdk.internal.misc
* java.base/jdk.internal.value
Copy link
Member

Choose a reason for hiding this comment

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

TEST.properties is updated; we might remove these directives.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I can't merge right now with the latest to test.

* @run junit/othervm IsIdentityClassTest
* @run junit/othervm --enable-preview IsIdentityClassTest
*/

import org.junit.jupiter.api.Test;

import java.lang.reflect.AccessFlag;
import java.lang.reflect.Modifier;
import java.util.Set;

import jdk.internal.misc.PreviewFeatures;

import static jdk.test.lib.Asserts.*;

public class IsIdentityClassTest {

@Test
void testIsIdentityClass() {
assertEquals(!PreviewFeatures.isEnabled(), Integer.class.isIdentity(), "Integer is not an IDENTITY type");
assertTrue(Integer[].class.isIdentity(), "Arrays of inline types are IDENTITY types");
}

@Test
void testModifiers() {
// Without --enable-preview (before Valhalla), there was no IDENTITY modifier.
// With --enable-preview, Integer still should not have the IDENTITY modifier.
// So only verify this in preview mode.
if (PreviewFeatures.isEnabled()) {
int imod = Integer.class.getModifiers();
assertFalse(Modifier.isIdentity(imod), "Modifier of Integer should not have IDENTITY set");
}
int amod = Integer[].class.getModifiers();
assertEquals(PreviewFeatures.isEnabled(), Modifier.isIdentity(amod), "Modifier of array should have IDENTITY set");
}

@Test
void testAccessFlags() {
// Without --enable-preview (before Valhalla), there was no IDENTITY accessflag.
// With --enable-preview, Integer still should not have the IDENTITY accessflag.
// So only verify this in preview mode.
if (PreviewFeatures.isEnabled()) {
Set<AccessFlag> iacc = Integer.class.accessFlags();
assertFalse(iacc.contains(AccessFlag.IDENTITY), "Access flags should not contain IDENTITY");
}
// AccessFlags for arrays set the IDENTITY accessflag.
Set<AccessFlag> aacc = Integer[].class.accessFlags();
assertEquals(PreviewFeatures.isEnabled(), aacc.contains(AccessFlag.IDENTITY), "Access flags of array of inline types should contain IDENTITY");
}
}