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

[Kim Barrett]

On Jan 13, 2016, at 3:50 PM, Coleen Phillimore <coleen.phillimore at oracle.com> wrote:
> 
> 
> Hi, Unfortunately, you're all getting another webrev.  I decided that I should make the test more robust, in the case where java -XX:+ExecuteInternalVMTests -version loads a class with the symbol "abc", "efg", "hij", etc. in it.
> 
> open webrev at http://cr.openjdk.java.net/~coleenp/8145940.05/

------------------------------------------------------------------------------  
share/vm/classfile/symbolTable.hpp
  85   // Decrement reference counter so it can go away if it's unique

"unique" seems like the wrong term to me here.  Maybe "unused" or
"unreferenced"?

------------------------------------------------------------------------------
src/share/vm/classfile/symbolTable.cpp
 679   assert(ss->refcount() == abc->refcount(), "should match TempNewSymbol");

I think a stronger check would be

  assert(ss == abc, "...");

since we have that operator==.

------------------------------------------------------------------------------   
src/share/vm/classfile/symbolTable.cpp
 678   assert(ss->refcount() == abccount, "only one abc");
...
 688   assert(s1->refcount() == efgcount, "one efg");
 689   assert(s2->refcount() == hijcount, "one hij");
&etc

the "one" and "two" and such in the messages are misleading. This is
to protect against some of these symbols already existing for other
reasons, but I think the error messages confuse things a bit.

Just a comment about the problem of pre-existing symbols, rather than
actually changing the messages, would be sufficient for me.

------------------------------------------------------------------------------   
src/share/vm/classfile/symbolTable.cpp
 704   s3 = SymbolTable::new_symbol("klm", CATCH);
 705   // can't really assert that it's one
 706   // assert(s3->refcount() == 1, "only one klm now");

Capture new_symbol result in a Symbol*, record refcount, assign symbol
to s3, and verify refcount unchanged.

------------------------------------------------------------------------------  

There are other aspects of the TempNewSymbol that I might question,
but that's for another day.

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.