RFR: 8213481: [REDO] Fix incorrect copy constructors in hotspot
David Holmes
david.holmes at oracle.com
Tue Nov 27 21:34:23 UTC 2018
On 28/11/2018 4:12 am, Kim Barrett wrote:
>> On Nov 26, 2018, at 10:21 PM, David Holmes <david.holmes at oracle.com> wrote:
>>
>> 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:
>>>>> 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
>
> The "natural result" is that the derived class copy-assign code does
> what it does. There are things that can go badly wrong here regarding
> lifetimes of embedded object references. But the asserted restriction
> on the allocated location of the object being copy-assigned to does
> nothing that I can see to prevent those potential problems.
>
> Can you provide a use-case where the assertion actually does something
> useful? I removed it because I don’t think such a thing exists.
AFAICS the only time it makes any sense to use assignment is with
stack/embedded ResourceObj. The assert verifies that is the case.
This seems a very clear cut use of an assert to ensure an API is not
being misused. Does it prevent all possible misuse - no. But that's not
a reason to remove what it does do.
Do we ever need the assignment semantics? Maybe we should just prohibit
it altogether?
David
More information about the hotspot-dev
mailing list