Code Review Request: 7157893: Warnings Cleanup in java.util.*

[Mike Duigou]

Looks good for commit. Just a few notes

- The inconsistent use of <K,V> by WeakHashMap and <?,?> by the other Hash Map classes for their tables is a bit annoying but not a new problem. We should either make all of the maps use <K,V> (my preference) or all use <?,?>. There were lots of cases of introduced <?> and <?,?> that are clearly correct (equals(), contains() etc.). I understand that <K,V> doesn't really add anything the compiler or JVM can currently use but it would seem to make the source intent more obvious.

ComparableTimSort.java:

"UnnecessaryLocalVariable" is a non-standard warning annotation. I seem to recall that there was a previous discussion of how these would be handled. Just a reminder in-case some action needs to be taken on this.

EnumMap.java:

maskNull seemed to have the unchecked cast well bottlenecked. Why move the cast outside of unmaskNull() and thus require @SuppressWarnings("unchecked") in many other places. 

Mike


On Apr 10 2012, at 16:15 , Kurchi Hazra wrote:

> Hi Stuart,
> 
>   Please find the updated webrev here: http://cr.openjdk.java.net/~khazra/7157893/webrev.01/
> 
> I hope to have included all the suggestions correctly. Also, note that I made some new changes in Hashtable.java at lines 185, 355 and 910 to get rid of some additional warnings.
> 
> Thanks,
> Kurchi
> 
> 
> On 4/6/2012 5:35 PM, Stuart Marks wrote:
>> Hi Kurchi, I think we've converged on the code changes. Please prepare and post another webrev for a final cross-check before pushing.
>> 
>> What follows is I think merely residual disagreement over the philosophy of how to handle generic casts vs reification. :-)
>> 
>> On 4/6/12 3:06 AM, Rémi Forax wrote:
>>> On 04/05/2012 11:04 PM, Stuart Marks wrote:
>>>> I'm somewhat skeptical of making code changes now based on potential future
>>>> benefits when/if generics become reified. This was discussed before; see
>>>> 
>>>> http://mail.openjdk.java.net/pipermail/jdk8-dev/2011-December/000454.html 
>>>> 
>>>> In that message, John Rose said "If the best practices have to change, then
>>>> we'll have to change that code again. Or maybe the retrofit strategy will
>>>> have to take account of the existing code idioms. In any case, we'll cross
>>>> that bridge when we get to it. (Coping with reification in this case is a
>>>> decision to make tomorrow, not today.)"
>>> 
>>> I disagree with John. The main issue with generics nowadays is that
>>> most of the people doesn't care about a cast to a type variable because
>>> everybody knows about erasure. So codes are written with an implementation
>>> glitch in mind.
>>> Frankly, I don't know if reification will appear (yes it's a kind of magical)
>>> or not
>>> but I think it's a sloppy path to not consider all casts as equals.
>> 
>> In order to program effectively with generics, I think you have to understand erasure and its implications. It may have been an unfortunate choice, but erasure is part of the language and we have to deal with it and in some cases rely on it. I don't think it's merely an "implementation glitch."
>> 
>> The difficulty I have with reification is that while there are proposals floating around for how it could be done, nobody really knows how it will eventually turn out, nor whether it will actually be done. If it is eventually done, there will legal and illegal constructs, constructs that generate warnings, and perhaps a style guide for how to use reified generics properly.
>> 
>> Right now, we can *imagine* what these future rules might be, but it seems untenable to me to try to make today's code conform to those imaginary future rules, especially in the absence of tools to help support those rules.
>> 
>>> If unmaskNull return a V, the code of equals will upcast the value from Object
>>> to V
>>> to just after downcast it from V to Object,
>>> I think it's better that unmask to return Object and upcast it to V when it's
>>> necessary.
>> 
>> Certainly there are cases where there's a redundant downcast and upcast. In a reified world, will this be a significant expense? Really, I have no idea.
>> 
>> s'marks
> 
> -- 
> -Kurchi
>