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

Dan Xu dan.xu at
Wed Aug 29 05:22:08 UTC 2012

It is funny. :) I have searched all source codes under jdk and removed 
spaces for the similar cases.

Please review the new version of change at,

Thanks for your comment!


On 08/28/2012 05:32 PM, Kurchi Hazra wrote:
> Irony of the day - those changes were done by me! 
> ( :D
> They were probably a mistake/oversight. I guess the better way is 
> without those extra spaces. See 
> If you have time, maybe you can remove those spaces I put in as a part 
> of this CR.
> Thanks!
> Kurchi
> On 8/28/2012 5:23 PM, Dan Xu wrote:
>> I also thought the space was not needed. But when I made the changes, 
>> I found that many similar codes had the space when two or more 
>> warning types need to be suppressed. For example, 
>> java/util/, java/util/, 
>> java/util/, and etc. If only one warning type 
>> needs to be suppressed, I don't see the space in our codes. Thanks!
>> -Dan
>> On 08/28/2012 05:08 PM, Kurchi Hazra wrote:
>>> I don't think you need the space before "unchecked" and the one 
>>> after "rawtypes" in lines 128 and 147 in
>>> - 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 
>>>> 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

More information about the core-libs-dev mailing list