Mike Duigou wrote but nobody heard:
This is a review request for an issue which was previously committed in 2006
but was quickly withdrawn because it was believed to cause a regression in
other software. That removal was mistaken and this fix appears to be bona-fide
beneficial.
http://cr.openjdk.java.net/~mduigou/5045147/0/webrev/
Note that this fix impacts both TreeMap and TreeSet. Prior to this fix both have allowed
"null" to be added to the collection when the map/set is empty. I've personally
run across this issue in usage. Diagnosing and fixing the broken application wasn't
initially obvious because of this bug in TreeMap/TreeSet. Only after some frustrating
sleuthing were we able to conclude that the problem was in TreeMap.
It would be great to get this one fixed, thanks for going through the
history.
The change looks good to me. I guess there isn't really any need to
explicitly check if the comparator is null as it will NPE anyway, and
probably there isn't a need to explicitly check key either as
compare(key,key) will give us the NPE. If you are keeping the explicit
null check then should the braces go?
-Alan.