RFR: 8237637: Remove dubious type conversions from oop

[Stefan Karlsson]

Hi David,

Thanks for looking at this. I've incorporated your suggestions.

StefanK

On 2020-01-23 00:56, David Holmes wrote:
> Hi Stefan,
> 
> I don't have any concerns with this, but if cast_to_oop is for 
> "numerical conversions" then can't we get rid of the secondary casts to 
> jlong and u8 where used i.e can
> 
> 
> --- 
> old/src/hotspot/share/jfr/leakprofiler/checkpoint/objectSampleWriter.cpp 
> 2020-01-22 14:24:21.194074776 +0100
> +++ 
> new/src/hotspot/share/jfr/leakprofiler/checkpoint/objectSampleWriter.cpp 
> 2020-01-22 14:24:20.714066939 +0100
> @@ -206,7 +206,7 @@
>     oop object = oosi->_data._object;
>     assert(object != NULL, "invariant");
>     writer->write(oosi->_id);
> -  writer->write((u8)(const HeapWord*)object);
> +  writer->write((u8)cast_from_oop<HeapWord*>(object));
> 
> not use:
> 
> writer->write(cast_from_oop<u8>(object));
> 
> and:
> 
> --- old/src/hotspot/share/jvmci/jvmciCompilerToVM.cpp    2020-01-22 
> 14:24:22.674098943 +0100
> +++ new/src/hotspot/share/jvmci/jvmciCompilerToVM.cpp    2020-01-22 
> 14:24:22.142090256 +0100
> @@ -431,7 +431,7 @@
>         } else if (JVMCIENV->isa_HotSpotObjectConstantImpl(base_object)) {
>           Handle base_oop = JVMCIENV->asConstant(base_object, 
> JVMCI_CHECK_NULL);
>           if (base_oop->is_a(SystemDictionary::Class_klass())) {
> -          base_address = (jlong) (address) base_oop();
> +          base_address = (jlong) cast_from_oop<address>(base_oop());
> 
> not use
> 
>            base_address = cast_from_oop<jlong>(base_oop());
> 
> ?
> 
> Thanks,
> David
> 
> On 22/01/2020 11:33 pm, 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
>>