Review request for java.text.** warning cleanup (original) (raw)

Deepak Bhole dbhole at redhat.com
Fri Dec 2 11:32:43 PST 2011

>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)(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[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 newRunAttributes[] = new Vector<?>[newArraySize]; >>+ @SuppressWarnings("unchecked") // generic array creation >>+ Vector 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 newRunAttributes[] = new Vector<?>[ARRAYSIZEINCREMENT]; > ^ > required: Vector[] > found: Vector<?>[]

Yes, you need to add a cast, Vector[] newRunAttributes = (Vector[])new Vector<?>[ARRAYSIZEINCREMENT]; 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

More information about the jdk8-dev mailing list