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

Mandy Chung mchung at openjdk.java.net
Tue Jun 23 20:03:58 UTC 2020


On Tue, 23 Jun 2020 19:55:33 GMT, Harold Seigel <hseigel at openjdk.org> wrote:

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

This is a copy-n-paste issue.  I expect a test knows what it does and it should use it with a clear intention.  I
prefer to take it out or add a comment explaining why NESTMATE is used for this hidden inline test.

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

I do not agree that we need to make VM anonymous classes to support inline types.    This block is for inline types
only.  Please remove it.

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

Please do not make VM anonymous classes to support inline type.

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

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


More information about the valhalla-dev mailing list