[lworld] RFR: JDK-8247795 allow factory methods for inline types to return j.l.Obje…

[Harold Seigel]

On Fri, 19 Jun 2020 20:08:04 GMT, Mandy Chung <mchung at openjdk.org> wrote:

>> Expect the return type for the <init> factory methods of hidden inline classes to return type java.lang.Object.  Also,
>> fix issue so that Reflection::new_constructor(), not new_method(), is called for <init> factory methods.
>
> src/hotspot/share/classfile/classFileParser.cpp line 2482:
> 
>> 2481:         const Symbol* required = class_name();
>> 2482:         if (is_hidden() || is_unsafe_anonymous()) {
>> 2483:           // The original class name in hidden classes and in the UAC byte stream gets
> 
> JDK no longer uses VM anonymous class.   This check (when parsing the static factory method of an inline type) should
> be updated to `if (is_hidden())` and drop `is_unsafe_anonymous()`.

VM anonymous classes need to be supported until they are obsoleted.  Removing this call to is_unsafe_anonymous() can
easily be done once that happens.

> src/hotspot/share/runtime/reflection.cpp line 1226:
> 
>> 1225:     BasicType rtype;
>> 1226:     if (klass->is_hidden() || klass->is_unsafe_anonymous()) {
>> 1227:       rtype = T_OBJECT;
> 
> Drop `is_unsafe_anonymous` check.   We should not support VM anonymous class be an inline type.
> 
> I suggest to add a comment to explain this workarounds the JVM spec issue for hidden classes.

See above comment about is_unsafe_anonymous().  I can add a comment about the JVM Spec issue in a follow-on fix.

> test/hotspot/jtreg/runtime/valhalla/valuetypes/HiddenInlineClassTest.java line 48:
> 
>> 47:     static byte[] readClassFile(String classFileName) throws Exception {
>> 48:         File classFile = new File(CLASSES_DIR + File.separator + classFileName);
>> 49:         try (FileInputStream in = new FileInputStream(classFile);
> 
> A simpler implementation is to use NIO:
>     static final Path CLASSES_DIR = Paths.get(System.getProperty("test.classes", "."));
> 
>     static byte[] readClassFile(String cn) throws IOException {
>         Path path = CLASSES_DIR.resolve(cn.replace('.', File.separatorChar) + ".class");
>         return Files.readAllBytes(path);
>     }

Multiple tests could benefit by using the NIO change that you suggest.  I'll enter an RFE for that.

> test/hotspot/jtreg/runtime/valhalla/valuetypes/HiddenInlineClassTest.java line 65:
> 
>> 64:         Class<?> c = lookup.defineHiddenClass(bytes, true, NESTMATE).lookupClass();
>> 65:         Object hp = c.newInstance();
>> 66:         String s = (String)c.getMethod("getValue").invoke(hp);
> 
> Nit: No need to be a nestmate.

Is it a problem that it's a nestmate?

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

PR: https://git.openjdk.java.net/valhalla/pull/94