RFR: 8237637: Remove dubious type conversions from oop

erik.osterlund at oracle.com erik.osterlund at oracle.com
Wed Jan 22 14:50:53 UTC 2020


Hi Stefan,

Nice! Ship it!

Thanks,
/Erik

On 1/22/20 3:33 PM, Stefan Karlsson wrote:
> On 2020-01-22 14:52, coleen.phillimore at oracle.com wrote:
>>
>> I really like this and have only started clicking.
>>
>>   174 template <class T> inline T cast_from_oop(oop o) {
>>   175   return (T)(CHECK_UNHANDLED_OOPS_ONLY((void*))o);
>>   176 }
>>
>>
>> Why did you leave void* as a conversion?   Can you define 
>> cast_from_oop<> to not need it?
>
> I thought it made sense to allow it to be converted to void*, but I 
> now see that it already works because it can be converted through 
> oopDesc: oop -> oopDesc* -> void*.
>
> I removed the following line and compiled locally:
>   operator void* () const             { return (void *)obj(); }
>
> I'll make sure that it compiles with other compilers as well.
>
> Thanks,
> StefanK
>
>>
>> Thanks,
>> Coleen
>>
>> On 1/22/20 8:33 AM, Stefan Karlsson wrote:
>>> Hi all,
>>>
>>> Please review this patch to remove some dubious type conversions 
>>> from oop.
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8237637
>>>
>>> When running with fastdebug and CHECK_UNHANDLED_OOPS oop is not a 
>>> simple oopDesc* but a class. This class has a number of type 
>>> conversions with the comment:
>>>
>>>   // Explict user conversions
>>>
>>> However, these are not only *explicit* conversions. They can be 
>>> invoked implicitly as well.
>>>
>>> I propose that we get rid of most of these and only leave these two:
>>>  operator void* () const
>>>  operator oopDesc* () const
>>>
>>> so that we get better type safety in the code.
>>>
>>> I've split this up into multiple webrevs to make it easier to review:
>>>
>>> https://cr.openjdk.java.net/~stefank/8237637/webrev.01/00.promotedObject/ 
>>>
>>> https://cr.openjdk.java.net/~stefank/8237637/webrev.01/01.oopStar/
>>> https://cr.openjdk.java.net/~stefank/8237637/webrev.01/02.jobject/
>>> https://cr.openjdk.java.net/~stefank/8237637/webrev.01/03.address/
>>> https://cr.openjdk.java.net/~stefank/8237637/webrev.01/03.address/
>>> https://cr.openjdk.java.net/~stefank/8237637/webrev.01/05.voidStarVolatiles/ 
>>>
>>> https://cr.openjdk.java.net/~stefank/8237637/webrev.01/06.heapWord/
>>>
>>> All changes combined:
>>> https://cr.openjdk.java.net/~stefank/8237637/webrev.01/all/
>>>
>>> Testing: builds pass, tier1-3 running
>>>
>>> Thanks,
>>> StefanK
>>>
>>



More information about the hotspot-dev mailing list