RFR: 8213481: [REDO] Fix incorrect copy constructors in hotspot

David Holmes david.holmes at oracle.com
Tue Nov 27 03:21:07 UTC 2018


On 27/11/2018 1:15 pm, Kim Barrett wrote:
>> On Nov 26, 2018, at 8:39 PM, David Holmes <david.holmes at oracle.com> wrote:
>>
>> Hi Kim,
>>
>> First thanks for the offline education on the nuances of copy constructors!
> 
> Happy to help.
> 
>> On 19/11/2018 4:14 pm, Kim Barrett wrote:
>>> Please review this fix of the debug-only copy constructor and
>>> assignment operator for ResourceObj, along with adding some missing
>>> uses of the copy constructor.
>>> The missing copy constructor uses were found by enabling -Wextra.
>>> The ResourceObj debug-only copy constructor should behave exactly the
>>> same as the corresponding default constructor.  That is, the setup for
>>> checking the allocation state in the destructor and operator delete
>>> should be the same whether in the normal or copy constructor.  That
>>> previously wasn't true, resulting in assert failures.
>>
>> That all seems fine to me.
> 
> Thanks.
> 
>>> The ResourceObj assignment operator also should not be assuming the
>>> target of the assignment was stack/embeded allocated; that's just not
>>> a valid assumption.  The corresponding assertion has been removed.
>>
>> Isn't that enforcing a usage requirement of ResourceObj? C-heap/resource-area/arena allocated ResourceObj should only be used via pointers - assigning one to another makes no sense to me.
> 
> I don’t see any reason for such an artificial restriction, and think it might prevent reasonable uses.
> I don’t have a specific use-case in mind, but the natural result is observed from doing the natural thing.

What is the "natural result" here? I don't see (and Vladimir seems to 
share a similar view) how it makes any sense to assign non 
stack/embedded resource objects to each other?  And mixing assignment 
across different types of ResourceObj makes no sense to me at all. I'm 
not even sure there's a usecase for stack/embedded but that at least 
seem a consistent usage.

David


More information about the hotspot-dev mailing list