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

Maurizio Cimadamore maurizio.cimadamore at oracle.com
Wed Jun 21 16:44:36 UTC 2017



On 21/06/17 16:56, Mandy Chung wrote:
>> On Jun 21, 2017, at 1:25 AM, Maurizio Cimadamore <maurizio.cimadamore at oracle.com> wrote:
>>
>> That's great - thanks Mandy. One last thing (which can also be addressed as a followup changeset) - it seems like the:
>>
>> MinimalValueTypes_1_0.classHasValueType
>>
>> method is basically just an heavy alias for the new 'isValueCapable' predicate - which also attempts to load the class. I wonder if we wouldn't be better off simply using the predicate, which seems less heavy as it doesn't strictly require generation/loading of DVT. Although I can imagine that if a client calls that method, he'll be interested in the DVT anyway.
>>
> I had similar question and wonder if this is needed at all.  If one wants to load DVT, it can call getValueTypeClass instead.  I think we should simply remove it.
>
> 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.
>
>
>> That said, from an API perspective, having one method instead of two slightly different one is better, so we should aim for it, I think.
>>
> Agree.
>
>> [but, as I said before, the changeset you have looks good enough to be pushed - so it's up to you whether you wanna try it now or later, ok? ]
>>
> I will make this change once we agree what to do for ValueType::classHasValueType.
>
> I’ll be traveling to Hong Kong tomorrow.  I don’t want to push something and not available.  I may push next Monday.
Cool
Thanks

Maurizio
>
> Mandy
>
>> Thanks
>> Maurizio
>>
>>
>>
>> On 21/06/17 03:26, Mandy Chung wrote:
>>>> On Jun 20, 2017, at 4:19 PM, Maurizio Cimadamore <maurizio.cimadamore at oracle.com> wrote:
>>>>
>>>> The new version looks good - I wonder if we should add a dual method called isValueCapable(Class<?>) and use that for the assertion check? The method would look at the class, retrieve the @VCC annotation and return true if one such annotation exists.
>>>>
>>>> As things stand, it is possible to pass classes to the loadValueClassType that are not VCC but which fail to trigger the assertion check.
>>>>
>>>> Probably a nitpick - but since we're tweaking in this area…
>>>>
>>> I added the isValueCapable method and made further clean up that I think
>>> MinimalValueTypes_1_0 Valueclass is clearer.
>>>
>>> http://cr.openjdk.java.net/~mchung/valhalla/webrevs/8181425/webrev.01/index.html
>>>
>>> Mandy



More information about the valhalla-dev mailing list