On Aug 1 2013, at 16:05 , David Holmes wrote: > On 2/08/2013 1:57 AM, Alan Bateman wrote: >> On 26/07/2013 16:31, Mike Duigou wrote: >>> Hello all; >>> >>> This patch adds some missing checks for null that, according to >>> interface contract, should be throwing NPE. It also improves the >>> existing tests to check for these cases. >>> >>> http://cr.openjdk.java.net/~mduigou/JDK-8021591/0/webrev/ >>> >>> The changes to >>> src/share/classes/java/util/concurrent/ConcurrentHashMap.java will be >>> synchronized separately with the jsr166 workspace. They are part of >>> this review to avoid test failures. >>> >>> Mike >> As retainAll and removeAll are long standing methods, are there are >> cases where we might now throw NPE when we didn't previously? I'm just >> wondering if any of these need to be looked at more closely, minimally >> to get into release/compatibility notes.
Yes, this would definitely be something we want to mention for the release notes. > I get a sense of deja-vu here. For retainAll/removeAll this fixes the case > where you would not get NPE if the target collection is empty. We already > dealt with this for some j.u.c collections - see 7123424 and then 8001575. I am not sure if this is a vote for or against continuing to add the missing NPE checks. If we decide against adding the checks I would actually push for revising the spec (make the NPE optional) to reflect the relatively common practice and possibly even remove some of the recently added NPE. Mike
