RFR: 8237637: Remove dubious type conversions from oop

[Kim Barrett]

> On Jan 23, 2020, at 3:59 AM, Stefan Karlsson <stefan.karlsson at oracle.com> wrote:
> 
> On 2020-01-23 08:06, Kim Barrett wrote:
>>> On Jan 22, 2020, at 8:33 AM, Stefan Karlsson <stefan.karlsson at oracle.com> wrote:
>>> […]
>>> All changes combined:
>>> https://cr.openjdk.java.net/~stefank/8237637/webrev.01/all/
>>> 
>>> Testing: builds pass, tier1-3 running
>>> 
>>> Thanks,
>>> StefanK
>> src/hotspot/share/compiler/oopMap.cpp
>>  795     *derived_loc = (oop)(cast_from_oop<address>(base) + offset);
>> Seems like that should also be using cast_to_oop.
> 
> This patch is about fixing conversions *from* oop to other types. There are numerous places in the VM where we convert *to* oop with casts instead of cast_to_oop. I don't want to do those kind of changes in this RFE.

OK.

>> Similarly for is_in_cset.  And there might be other functions like
>> this that I didn't notice.
> 
> I can't find what you are referring to.

Neither can I now.  Sorry for the noise.

>> Note that there are a lot of implicit conversions (mostly to void* I
>> think) that prevent us from removing (or making explicit with C++11)
>> the oopDesc* conversion operator.
> 
> It's not obvious to me that we want to remove the implicit conversions to oopDesc*.

I was looking at that possibility on the basis that implicit
conversions are often problematic, so wondering if we really need this
one.  It turns out we do use it quite a bit.  That's not surprising,
since it makes the CHECK_UNHANDLED_OOPS case more like the
!CHECK_UNHANDLED_OOPS case, where oop is just a typedef for oopDesc*.
But it seemed worth the check.  I should have mentioned the behavioral
consistency as another reason to keep it though.

> I'm going to run this (and David's suggestions) through tier1,

I'm somewhat ambivalent about David's suggestions.  One of my not yet
written up suggestions for followup involves putting some constraints
on the types that can be used in cast_to/from_oop, rather than allow
arbitrary types (some of which are probably at least questionable,
including some that are currently used).  But any issues there can be
dealt with as part of that followup.

> and then I'll push the patch.

OK.