We'll be using https://bugs.openjdk.java.net/browse/JDK-8029055 for this issue.
I've posted a webrev here: http://cr.openjdk.java.net/~mduigou/JDK-8029055/0/webrev/ There is an identical change in ConcurrentMap's merge(). Mike On Nov 22 2013, at 16:01 , Henry Jen <henry....@oracle.com> wrote: > > On 11/21/2013 06:33 PM, David Holmes wrote: >> On 22/11/2013 5:02 AM, Louis Wasserman wrote: >>> While I agree that case should be specified, I'm not certain I follow why >>> that's what's going on here. >>> >>> The weird condition is that if oldValue is null, not value; oldValue >>> is the >>> old result of map.get(key). The Javadoc seems not just unspecified, but >>> actively wrong. Here's a worked example: >>> >>> Map<String, Integer> map = new HashMap<>(); >>> map.merge("foo", 3, Integer::plus); >>> Integer fooValue = map.get("foo"); >>> >>> Going through the Javadoc-specified default implementation: >>> >>> V oldValue = map.get(key); // oldValue is null >>> V newValue = (oldValue == null) ? value : >>> remappingFunction.apply(oldValue, value); >>> // newValue is set to value, which is 3 >>> if (newValue == null) // newValue is nonnull, branch not taken >>> map.remove(key); >>> else if (oldValue == null) // oldValue is null, branch taken >>> map.remove(key); // removes the entry from the map >>> else // else if was already triggered, branch not taken >>> map.put(key, newValue); >>> > > Seems like a document bug to me, we should fix this @implSpec. > > Cheers, > Henry >
