RFR JDK-8181425: Reflection API defend against issues with internal VM derived value type

[David Simms]

On 22/06/17 13:27, Maurizio Cimadamore wrote:
>
>
> On 22/06/17 03:21, Mandy Chung wrote:
>>> On Jun 21, 2017, at 5:23 PM, Maurizio Cimadamore 
>>> <maurizio.cimadamore at oracle.com> wrote:
>>>
>>> I notice some asymmetry between the VCC and DVT cases. For VCC we have:
>>>
>>> * isValueCapable(Class)
>>> * getValueCapableClass(Class)
>>>
>>> For DVT we have
>>>
>>> * isValueType(Class)
>>> * getValueTypeClass(Class)
>>> * loadValueTypeClass(Class)
>>>
>>> There are two kind of inconsistencies at play here - one is a plain 
>>> naming issue - the suffix Class is not using consistently in all the 
>>> methods.
>>>
>> I believe David was the author of the MinimalValueType_1_0 class and 
>> he might want to comment here.   I haven’t read up the documents in 
>> details to determine the proper terminologies.  But I have to admit 
>> that I was initially confused with isValueType that refers to DVT. 

I'm not picky about the names, I'd probably drop "Class" from the method 
name to be honest, seems redundant when the single arg are return are 
classes. The asymmetry came when I wanted to differentiate between 
"DVT"  & "VCC" Class. "It felt weird" to write "ValueCapable" and not 
finish with Class (since we are so used to saying VCC). I'm not up on 
JDK naming conventions, feel free change it.

    Terminology in the draft JVMS
    (http://cr.openjdk.java.net/~dlsmith/values.html) work also differs
    from the Shady Values
    (http://cr.openjdk.java.net/~jrose/values/shady-values.html), i.e.
    "Derived Value Type" == "Direct Value Class Type"...so there's that.
    "Value Capable" and "Value Type" seem reasonable.

Minor Comments:

  * Copyright year ValueType.java, MinimalValueTypes_1_0.java,
    TEST.groups and MVTTest.java
  * MinimalValueTypes_1_0.java:108 "already a derived value type" ?
      o "(!isValueCapable(vcc))" message might read " not a value
        capable class" ?

Otherwise, the webrev looks good, much appreciated code-coverage in the 
added tests. Thanks for addressing the mess I left behind and lack of 
modules support.

I assume both "make test" for "hotspot_valhalla" and "jdk_valhalla" pass ?

Cheers
/David Simms