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
>
