RFR 8223351 [lworld/lw2] : Primary mirror and nullable mirror for inline type

[Roger Riggs]

Hi Mandy,

Class.java: 261,  Shouldn't "value" -> "inline"  as does toString().

Line 535:
I'm a bit confused by the Class.inlineType field.
It seems to be the secondary mirror (javaClasses.cpp:990) but here is 
returned as the primary.
Is this backwards?  If it is an inline class, then it is its own primary.
Alternatively, inlineType would/should have been initialized to the 
primary and a runtime check would not be needed.

1329:  Can't asPrimaryType() be used without qualification?
AsPrimaryType should always return the primary and the signers should 
always be on the primary.

Everyplace that uses asPrimaryType() would be clearer if it did not 
double test for inline.

Class.java: 511-516 should not need to be there; the verifier should 
have already confirmed that.
Add a comment to remove extra tests.

Line 593:  no need for ?: here; return isIndirectType();

Line 1718:  Use private toTypeName().

I didn't look too closely at the j.l.invoke parts.

Roger


On 06/04/2019 08:27 PM, Mandy Chung wrote:
> I have updated the patch to add the indirect projection type. Updated 
> webrev:
> http://cr.openjdk.java.net/~mchung/valhalla/webrevs/lw2/8223351/webrev.01/ 
>
>
> javadoc:
> http://cr.openjdk.java.net/~mchung/valhalla/webrevs/lw2/api/java.base/java/lang/Class.html 
>
>
> Summary of the changes:
>
> 1. For an inline class V, V.class is the primary mirror.   The 
> secondary mirror
>     is the indirect projection type which is also the nullable 
> projection type.
>
>     Class.forName returns the primary mirror.  The projection types are
>     used for reflection APIs to find a field of type V? or a method 
> with signature
>     with V? (i.e. L-type descriptor).
>
> 2. New APIs are added for core reflection to project an inline class 
> its indirect
>     or nullable projection type as well as query if a Class object is 
> indirect or inline
>     or nullable.
>
>     Class::asPrimaryType
>     Class::asIndirectType
>     Class::asNullableType
>
>     Class::isInlineClass  (was Class::isValue)
>     Class::isIndirectType
>     Class::isNullableType
>
> 3. Class::getName returns the same name for inline class and the 
> projection type
>     as that's the name of the class in the source
>
> 4. Class::isAssignableFrom checks properly the subtype relationship
>     Point <: Point? and Point? <: Object
>
> Notes:
> - I didn't rename ValueKlass::value_mirror as ValueKlass and other 
> classes will
>   be renamed together in the future.
> - I think AccessFlags::set_is_flattenable should be re-examined. 
> JVM_ACC_FLATTENABLE
>   no longer set in lworld?
> - Intrinsification of Class::asPrimaryType will need update 
> (JDK-8225317).
>   I have excluded 
> MyValue1.class.isAssignableFrom(MyValue1.class.asIndirectType())
>   test case in compiler/valhalla/valuetypes/TestIntrinsifics.java. I'd 
> need the compiler
>   expert to help investigating it while I think it might be related to 
> JDK-8225317.  Hence
>   I capture that failing case in JDK-8225317.
> - I took out the unnecessary asPrimaryType call in the tests since 
> V.class now returns
>   the primary mirror.  I left several asPrimaryType calls in 
> TestIntrinsics as they look
>   like those calls are intended.
>
> Mandy
>
> On 5/15/19 11:38 AM, Roger Riggs wrote:
>> Please review Mandy's additions and changes to reflection and 
>> java.lang.invoke APIs
>> for inline and nullable types.
>> The changes go a bit deep because of the support for the Java APIs 
>> provided by the VM.
>>
>> This initial prototype reflects discussions about terminology and 
>> orthogonality
>> of concepts for inline vs nullable as described in the comments of 
>> 8223351.
>>
>> Issue:
>>   https://bugs.openjdk.java.net/browse/JDK-8223351
>>
>> Webrev:
>>    http://cr.openjdk.java.net/~rriggs/webrev-lworld-dev-8223351/
>>
>> Thanks for any comments and suggestions, Roger
>>
>