RFR: 8214363: HeapWord should not be a fake class

[coleen.phillimore at oracle.com]

I don't think heap_word_value() is necesary.  I was just missing a "*" 
in that expression.  It's not dereferencing an opaque type.
LGTM!

thanks,
Coleen

On 2/15/19 4:36 PM, Mikael Vidstedt wrote:
> I wouldn’t be against that. :)
>
> Cheers,
> Mikael
>
>> On Feb 15, 2019, at 1:34 PM, Kim Barrett <kim.barrett at oracle.com> wrote:
>>
>>> On Feb 15, 2019, at 4:13 PM, Mikael Vidstedt <mikael.vidstedt at oracle.com> wrote:
>>>
>>>
>>>
>>>> On Feb 15, 2019, at 12:44 PM, Kim Barrett <kim.barrett at oracle.com> wrote:
>>>>
>>>>> On Feb 15, 2019, at 3:27 PM, coleen.phillimore at oracle.com wrote:
>>>>>
>>>>>
>>>>> moving to hotspot-dev.
>>>>> Coleen
>>>>>
>>>>> On 2/15/19 8:07 AM, coleen.phillimore at oracle.com wrote:
>>>>>> This is nice but what does this mean?
>>>>>>
>>>>>> http://cr.openjdk.java.net/~kbarrett/8214363/open.02/src/hotspot/share/gc/shared/spaceDecorator.cpp.udiff.html
>>>>>>
>>>>>> + return badHeapWord == (max_juint & reinterpret_cast<uintptr_t>(*q));
>>>>>>
>>>>>>
>>>>>> I thought since it has no implementation class, you can't use *q ?
>>>> The type of q is HeapWord*, which is (with this change) HeapWordImpl**.
>>>> Dereferencing q => HeapWordImpl*, which is then cast to a uintptr_t.
>>>>
>>>> A pretty similar effect could have been achieved by changing HeapWord to
>>>> struct HeapWord { HeapWordImpl* value; };
>>>> e.g. making the member public (to eliminate the new gcc warning) and changing
>>>> the pointee type of the member to an incomplete type (to prevent dereference).
>>> I was also confused by that. Not enough to have a strong opinion though.
>>>
>>> Looks good to me. For completeness I too tried your change with gcc 8.2 and the warnings are indeed gone.
>>>
>>> Cheers,
>>> Mikael
>> I thought about adding “uintptr_t heap_word_value(HeapWord*)”, but it didn’t seem worth
>> the trouble for a single caller.  If that would have helped…
>>