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

[Maurizio Cimadamore]

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.

The second inconsistency is that for a DVT we distinguish between 
get/load, while for VCC we do not. But it's a moot distinction, given 
that getValueTypeClass will always call loadValueTypeClass (and recheck 
same assertion). Maybe we should fuse those two methods?

Cheers
Maurizio



On 21/06/17 21:21, Mandy Chung wrote:
>> On Jun 21, 2017, at 9:44 AM, Maurizio Cimadamore <maurizio.cimadamore at oracle.com> wrote:
>>
>>
>> On 21/06/17 16:56, Mandy Chung wrote:
>>> ValueType::classHasValueType is not used.  Should this be replaced with isValueCapable?  Is there a case no DVT defined for VCC (which would be the case classHasValueType would return false for VCC) but I suspect not?
>> I believe ValueType is supposed to be a public API, while MinimalValueType_1_0 is not. So, maybe we should keep that one as a sort of reflective predicate.
> I keep ValueType::classHasValueType the same behavior that loads DVT before
> returning.  I suspect it may not be necessesary but we can re-examine that
> in the future.
>
> http://cr.openjdk.java.net/~mchung/valhalla/webrevs/8181425/webrev.02/
>
> Changes w.r.t. webrev.01
> 1. Removed classHasValueType(Class<?>) and getValueTypeClass(Class<?>) which
> is only used by classHasValueType.
> 2. getValueCapableClass and getValueTypeClass no longer throws CNFE
>     (it throws IAE if the given type is not DVT or VCC).  MethodHandles is
>     updated due to the removal of this checked exception.
> 3. I added valhalla/mvt to jdk_lang test group that makes it handy to do verification.
>
> thanks
> Mandy