On 3/08/2013 3:20 AM, Mike Duigou wrote:
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.
It was merely an observation that we have already trodden this path. But
if it were a vote I would vote to throw and add a note to the
compatibility docs.
A spec that is loosened to say may throw NPE is useless because anyone
who relies on an implementation not throwing will be bitten by an
implementation that does.
Cheers,
David
-----
Mike
