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

[Kim Barrett]

> On Nov 27, 2018, at 4:34 PM, David Holmes <david.holmes at oracle.com> wrote:
> 
> On 28/11/2018 4:12 am, Kim Barrett wrote:
>> 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

I don't understand why you might think that, and don't agree.  Maybe
there are additional unstated and unenforced restrictions?

But part of my rejection of such a position is that a commonly
understood idiomatic operation like assignment should either just
work, or it shouldn't be allowed at all. Having a strange restriction
like this makes code much harder to understand. Make a new operation
for this restricted assignment.

Unfortunately, there are already many uses of ResourceObj assignment.
I spent a bit of time exploring the idea of removing or replacing it,
but that seems way too hard for the intended scope of this cleanup.
(Note that for some of the existing uses it is not obvious whether the
restriction applies. Presumably it doesn't, since we're not hitting
the assert.) While I like the idea of prohibiting assignment (and
possibly copy too, which has many of the same issues), I'm not going
to pursue that beyond perhaps an RFE for future work.

BTW, as part of that exploration I found such gems as this, from
opto/cfgnode.cpp:962:

  Node_Array node_map = new Node_Array(a);

Anyway, even though I think the assert prevents some things that would
work (but should perhaps be avoided anyway), and doesn't prevent some
things that won't work, I'm willing to reinstate it in order to make
progress.

New webrevs:
full: http://cr.openjdk.java.net/~kbarrett/8213481/open.01/
incr: http://cr.openjdk.java.net/~kbarrett/8213481/open.01.inc/