RFR 7118066: Warnings in java.util.concurrent package

[David Holmes]

Chris, Doug,

A few nits see below.

Cheers,
David
-----

As a matter of style can we ensure annotations are on separate lines. I 
find this:

   @SuppressWarnings("unchecked") E x = (E) items[takeIndex];

hard to read. (I hate seeing local variable annotations in the first 
place - way too much clutter in my opinion.)

Is the reason for constructs like this:

    HashEntry<K,V>[] tab = (HashEntry<K,V>[])new HashEntry<?,?>[cap];

that we can't utilize diamond? Otherwise it would nicely reduce to:

    HashEntry<K,V>[] tab = new HashEntry<>[cap];


In ConcurrentSkipListMap:
- doesn't the class-level unchecked annotation make the local-variable 
annotation redundant in clone()
- you added:
      * @param s the stream
   for readObject, but not for writeObject. Seems unnecessary for either.
- in the KeySet and Values classes why the changes from <E,Object> to 
<E,?> ? Seems superfluous. Did it affect any warnings?

In Exchanger why suppress the serial warning rather than add the 
serialVersionId? Elsewhere the id was added. (I prefer to suppress but 
am querying consistency here)

In LinkedTransferQueue we changed:

this.<E>cast(item);

to

LinkedTransferQueue.<E>cast(item);

but in ArrayBlockingQueue we simply changed to: (E) item. Were these 
cast methods introduced simply to localize the unchecked suppression?

Also in LinkedTransferQueue we went from:

       E e;
       while ( (e = poll()) != null) {

to

       for (E e; (e = poll()) != null;) {

the side-effect on the loop condition is ugly in a for-loop. Why change 
from the while? Why not change to a proper for-loop:

  for (E e = poll(); e != null; e = poll()) {


In SynchronousQueue:

-  the Transferer, TransferStack and TransferQueue classes have been 
generified, but not the internal SNode and QNode classes. Generifying 
nodes would take a bit of work, but I guess I don't see the point of 
generifying the others.

- why stop using Collections.emptyIterator()?
- same comments on while-loop to for-loop conversions as for LTQ



On 6/12/2011 1:36 AM, Chris Hegarty wrote:
>
> Cleanup warnings in the j.u.c. package.
>
> This is a sync up with the warning fixes in Doug's CVS. There are also a
> few style cleanups, import fixes, trivial local variable renaming,
> typos, etc. But nothing too surprising!
>
> http://cr.openjdk.java.net/~chegar/7118066/webrev.00/webrev/
>
> -Chris.
>
> P.S. I have already reviewed this, and the contribution is of course
> from Doug.