RFR: 8237637: Remove dubious type conversions from oop

[David Holmes]

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
>