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

[Kurchi Hazra]

I don't think you need the space before "unchecked" and the one after 
"rawtypes" in lines 128 and 147 in
  http://cr.openjdk.java.net/~dxu/7193406/webrev.02/src/share/classes/java/util/PropertyResourceBundle.java.sdiff.html.

- Kurchi

On 8/28/2012 4:57 PM, Dan Xu wrote:
> 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
>

-- 
-Kurchi