Warnings Cleanup in java.util.<various> (more from hack day)

[Stuart Marks]

On 12/2/11 4:49 PM, John Rose wrote:
> On Dec 2, 2011, at 4:30 PM, Doug Lea wrote:
>> On 12/02/11 19:16, John Rose wrote:
>>> Presently we are aiming at removing warnings with no changes to semantics
>>> (i.e., bytecodes).
>>
>> Note that this is at odds with suggestions such as:
>>
>>> java/util/Currency.java --
>>>
>>> The @SuppressWarnings covers the entire method. We're trying to use
>>> @SuppressWarnings with as narrow a scope as possible. Sometimes it's helpful to
>>> create a local variable declaration for this purpose; perhaps something like
>>> this will help:
>>>
>>> @SuppressedWarnings("unchecked")
>>> Set<Currency>  result = (Set<Currency>) available.clone();
>>> return result;
>>
>> This generates two more bytecodes and a local variable.
>> This won't matter when the code is actually JIT-compiled, but
>> the increased size might inhibit inlining or compilation.
>> Surely it doesn't matter in this class, but we are very careful
>> about these and many related issues in performance-sensitive
>> java.util.concurrent code.
>
> Good point; thanks.
>
> Conflicting requirements are the spice of engineering life.
>
> If you remove 900 warnings, there is a good chance of causing some sort of bug. ( http://blogs.oracle.com/jrose/entry/three_software_proverbs )
>
> The 900 warnings may be masking other bugs, so we take the risk.

I won't make a stand on warnings fixes not changing bytecodes. This is too 
strict a requirement. As Doug pointed out (by quoting a message from me!) we 
are indeed making changes that result in different bytecodes. It's interesting 
to know that j.u.c is sensitive to bytecode changes, but most of the rest of 
the class library isn't.

I do want to ensure that the warnings fixes don't make any semantic changes. It 
should be possible to look at the source diff and determine that its semantics 
are identical. We should avoid seemingly innocuous changes such as calling the 
recommended replacement instead of a deprecated method, or upgrading to a newer 
class such as from Date to Calendar. It's very difficult to prove anything 
about such changes. John's admonition is sound here.

s'marks