RFR (xs) 8145940: TempNewSymbol should have correct copy and assignment functions

[Coleen Phillimore]

On 1/13/16 6:29 PM, John Rose wrote:
> Count me in as one who is very glad to see TempNewSymbol
> getting regularized.  You can also count me as a reviewer.

I forgot to say, thank you for the code review.  Also, thank you for 
pointing out the problem in the first place.
It was lost in my mailbox for a while.

Coleen

>
> Like Kim and Markus, I dislike the fact that TempNewSymbol
> can decrease a refcount that it did not increase in the first place,
> because new_symbol or lookup pre-incremented the refcount.
> (FWIW, this has always bothered me about TempNewSymbol.)
> This is the sort of convention that works best when the type
> system enforces it.  Quick question:  Exactly how many of
> the Symbol::* API points which yield Symbol* values include
> the pre-increment?  If the answer is not "all of them" or "none
> of them", we have a manually-enforced distinction which may
> turn out to be fragile.
>
> Markus, is there a tracking bug yet for addressing this?
> There should be.
>
> The "copy and swap idiom" is nice, and a new C++ trick to me.
> It is a clever way of cleanly encapsulating keeping the lifecycle
> invariants in the ctor/dtor pairs.  (Except for the case of TNS(S*)
> which Kim, Markus, and I are uncomfortable about.)
>
> Here's a thought, FWIW, about an incremental way forward on
> the pre-increment problem.  It might be useful to split the "magic"
> functions:
>
>   TempNewSymbol lookup(int index, const char* name, int len, unsigned 
> int hash);
>   TempNewSymbol new_symbol(const char* name, TRAPS) { … }
>   Symbol* lookup_increment_refcount(int index, const char* name, int 
> len, unsigned int hash);
>   Symbol* new_symbol_increment_refcount(const char* name, TRAPS) { … }
>
> There would be an initial wave of renaming (adding _increment_refcount),
> followed (perhaps) by gradual removal of the _increment_refcount versions
> in favor of more consistent use of TempNewSymbol.  Arrays are tricky, 
> alas,
> so maybe the endpoint is still messy, but elsewhere we work with growable
> arrays of type Handle, so maybe there is a way forward.
>
> — John
>
>
> On Jan 13, 2016, at 2:35 PM, Kim Barrett <kim.barrett at oracle.com 
> <mailto:kim.barrett at oracle.com>> wrote:
>>
>> I also like Markus's suggestion of SymbolTable symbol access returning
>> TempNewSymbol (perhaps under a better name!), but that is likely a
>> bigger change; I think this is still an improvement.
>>
>> Looks good, other than those minor things above.  Unless one of these
>> leads to a substantive change, I don't need another webrev.
>