RFR: 8021591 : (s) Additional explicit null checks

Mike Duigou mike.duigou at oracle.com
Fri Aug 2 17:20:46 UTC 2013

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.


More information about the core-libs-dev mailing list