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