Review request for java.text.** warning cleanup

[Deepak Bhole]

* Rémi Forax <forax at univ-mlv.fr> [2011-12-02 14:23]:
> Hi Deepak,
> 
> On 12/02/2011 07:47 PM, Deepak Bhole wrote:
> >* John Rose<john.r.rose at oracle.com>  [2011-12-01 19:49]:
> >>On Dec 1, 2011, at 3:19 PM, Deepak Bhole wrote:
> >>
[ ... ]

> >Ah, I wasn't sure if I should have used 7-specific features so I stayed
> >away from them (when bootstrapping when ecj, we found diamond to be
> >problematic as older ecj versions cannot understand it). I will change
> >these then.
> 
> You can use them everywhere but in langtools because
> the compiler need to bootstrap itself.

Ah okay, thanks.

[ ... ]
> >
> >>For createRunAttributeDataVectors, ensureRunBreak, etc. consider pushing the @SuppressWarnings down onto the individual declarations.  It is best to put @SuppressWarnings on the smallest program unit possible, and a local variable declaration is often the right spot.  I see that createRunAttributeDataVectors could be viewed as simple enough to mark as a whole, but ensureRunBreak is probably too complex to mark as a whole.
> >>
> >Doh! Good point.
> >
> >For DictionaryBasedBreakIterator.divideUpDictionaryRange, I cannot add
> >it above the
> >"bestBreakPositions = (Stack<Integer>)(currentBreakPositions.clone());" line
> >without refactoring. I done done a very minor refactor (use copy
> >constructor instead of clone) -- please let me know if it is
> >satisfactory.
> 
> It seems you still use clone,
> you can't replace a call to clone with a call to a copy constructor because
> if Stack is subclassed, clone will work but not the copy constructor.
> 

Doh. Sorry, I forgot to update my reply before sending. Using copy constructor
was my intended approach, but then I realized that Stack itself doesn't have
one (for the exact reason you pointed out :) -- Stack's parent Vector has one).
What I did was create a temp copy object in the local scope (thus allowing
suppression) and then assign the outer scope stack to it.

Sorry for the unnecessary noise.

[ ... ]

> >>For generic array creation, I suggest that you get rid of the "rawtypes" warning on the array element type by adding<?>, before suppressing the "unchecked" warning.  Basically, when you find yourself writing new Foo<Bar>[len], change it to new Foo<?>[len].  Then, if you still get an unchecked warning, add SuppressWarnings.
> >>
> >>-            Vector newRunAttributes[] = new Vector[newArraySize];
> >>-            Vector newRunAttributeValues[] = new Vector[newArraySize];
> >>+            @SuppressWarnings("unchecked")  // generic array creation
> >>+            Vector<Attribute>  newRunAttributes[] = new Vector<?>[newArraySize];
> >>+            @SuppressWarnings("unchecked")  // generic array creation
> >>+            Vector<Object>  newRunAttributeValues[] = new Vector<?>[newArraySize];
> >>
> >I tried that yesterday, but the compiler threw errors:
> >
> >../../../src/share/classes/java/text/AttributedString.java:420: error: incompatible types
> >         Vector<Attribute>  newRunAttributes[] = new Vector<?>[ARRAY_SIZE_INCREMENT];
> >                                                ^
> >   required: Vector<Attribute>[]
> >   found:    Vector<?>[]
> 
> Yes, you need to add a cast,
> 
> Vector<Attribute>[] newRunAttributes = (Vector<Attribute>[])new Vector<?>[ARRAY_SIZE_INCREMENT];
> 
> but if Java is reified in 1.9 as state the current plan, we will be
> in trouble.
> 

I see. Then perhaps it is just better to leave the code as it and keep
the suppressions in rev 01? Or should I change it regardless and then
let it be fixed again for 9?

Thanks for taking a look!

Cheers,
Deepak