Review Request: 7193406 - Clean-up JDK Build Warnings in java.util, java.io

[Dan Xu]

Thanks for all your good suggestions!

I have updated my changes, which revoke changes to makefiles and put 
@SuppressWarnings outside methods instead of introducing local variables 
for small methods.

The webrev is at http://cr.openjdk.java.net/~dxu/7193406/webrev.02/. Thanks!

-Dan

On 08/27/2012 04:33 PM, Stuart Marks wrote:
> On 8/27/12 3:55 AM, Doug Lea wrote:
>> The underlying issue is that code size is one of the criteria
>> that JITs use to decide to compile/inline etc. So long as they do
>> so, there will be cases here and there where it critically
>> important to keep sizes small in bottleneck code. Not many,
>> but still enough for me to object to efforts that might
>> blindly increase code size for the sake of warnings cleanup.
>
> I'm pleased that warnings cleanup has attracted this much attention. :-)
>
> I was wondering where rule about @SuppressWarnings and local variables 
> originally came from, and I tracked this back to Effective Java, Item 
> 24, which says (as part of a fairly long discussion)
>
>     If you find yourself using the SuppressWarnings annotation
>     on a method or constructor that's more than one line long,
>     you may be able to move it onto a local variable declaration.
>     You may have to declare a new local variable, but it's worth it.
>
> Aha! So it's all Josh Bloch's fault! :-)
>
> In the warnings cleanup thus far, and in Dan's webrev, we've followed 
> this rule fairly strictly. But since we seem to have evidence that 
> this change isn't worth it, we should consider relaxing the rule for 
> performance-critical code. How about adding a local variable for 
> @SuppressWarnings only if the method is, say, longer than ten lines? 
> (Or some other suitable threshold.) For short methods the annotation 
> should be placed on the method itself.
>
> The risk of suppressing other warnings inadvertently is pretty small 
> in a short method, and short methods are the ones most likely to be 
> impacted by the addition of a local variable affecting 
> compilation/inlining decisions. Right?
>
> (Also, while I've recommended that people follow the local variable 
> rule fairly strictly, I think it tends to garbage up short methods. So 
> I wouldn't mind seeing the rule relaxed on readability grounds as well.)
>
> s'marks