RFR: 8021591 : (s) Additional explicit null checks

[Mike Duigou]

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