Review request for java.text.** warning cleanup
Deepak Bhole
dbhole at redhat.com
Fri Dec 2 11:32:43 PST 2011
* 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
More information about the jdk8-dev
mailing list