Using @implSpec in AbstractMap, AbstractCollection, AbstractList, etc

[Chris Hegarty]

Hi Doug,

I agree with your changes to AbstractMap. I’ve taken your patch, removed the superfluous paragraph tags, and generated a webrev. 
   http://cr.openjdk.java.net/~chegar/AbstractMap_implSpec/webrev/

I also ran specdiff to see what else may be impacted by this. It shows that the only spec changes are to AbstractMap itself, and to ConcurrentHashMap size and isEmpty. What we want.
   http://cr.openjdk.java.net/~chegar/AbstractMap_implSpec/specdiff/overview-summary.html

I know it is late in the JDK 8 game, but I see this as a serious bug in the documentation, and it needs to be addressed.

Conservatively, one could argue that minimally pasting the appropriate specs into CHM size and isEmpty would be sufficient to resolve the problem, but I think your first proposal is a more complete solution. Given the above specdiff, I would be confident to request this change for inclusion in JDK 8.

I filed the following bug to track this issue:
   https://bugs.openjdk.java.net/browse/JDK-8031133

-Chris. 

On 28 Dec 2013, at 13:47, Doug Lea <dl at cs.oswego.edu> wrote:

> 
> There might have been some mis-communication about whether the
> new @implSpec tag would be used in existing java.util.AbstractX
> classes. I had assumed that it would be, and pulled out a few
> javadoc overrides in java.util.concurrent classes that would
> not be needed if @implSpec were used. With @implSpec, you do
> not need to paste in the interface-level spec in an overridden
> method just to suppress inclusion of (incorrect) doc comments
> about the default implementation.
> 
> Is it too late to do this for JDK8? It is easy -- just add a line
> with @implSpec for each defined method. AbstractMap diffs
> below. (They could be tweaked to get rid of now-unnecessary
> paragraph markup, but I don't think this would improve display.)
> 
> If this isn't done, then minimally we'd need to paste in
> interface-level specs in ConcurrentHashMap.{size, isEmpty}
> since the JDK8 javadocs as they stand are wrong. There
> may be other cases as well.
> 
> ...
> 
> 
> diff -r 8bc258c021a3 src/share/classes/java/util/AbstractMap.java
> --- a/src/share/classes/java/util/AbstractMap.java	Thu Nov 21 09:23:03 2013 -0800
> +++ b/src/share/classes/java/util/AbstractMap.java	Sat Dec 28 08:33:42 2013 -0500
> @@ -78,6 +78,7 @@
>     /**
>      * {@inheritDoc}
>      *
> +     * @implSpec
>      * <p>This implementation returns <tt>entrySet().size()</tt>.
>      */
>     public int size() {
> @@ -87,6 +88,7 @@
>     /**
>      * {@inheritDoc}
>      *
> +     * @implSpec
>      * <p>This implementation returns <tt>size() == 0</tt>.
>      */
>     public boolean isEmpty() {
> @@ -96,6 +98,7 @@
>     /**
>      * {@inheritDoc}
>      *
> +     * @implSpec
>      * <p>This implementation iterates over <tt>entrySet()</tt> searching
>      * for an entry with the specified value.  If such an entry is found,
>      * <tt>true</tt> is returned.  If the iteration terminates without
> @@ -126,6 +129,7 @@
>     /**
>      * {@inheritDoc}
>      *
> +     * @implSpec
>      * <p>This implementation iterates over <tt>entrySet()</tt> searching
>      * for an entry with the specified key.  If such an entry is found,
>      * <tt>true</tt> is returned.  If the iteration terminates without
> @@ -157,6 +161,7 @@
>     /**
>      * {@inheritDoc}
>      *
> +     * @implSpec
>      * <p>This implementation iterates over <tt>entrySet()</tt> searching
>      * for an entry with the specified key.  If such an entry is found,
>      * the entry's value is returned.  If the iteration terminates without
> @@ -191,6 +196,7 @@
>     /**
>      * {@inheritDoc}
>      *
> +     * @implSpec
>      * <p>This implementation always throws an
>      * <tt>UnsupportedOperationException</tt>.
>      *
> @@ -206,6 +212,7 @@
>     /**
>      * {@inheritDoc}
>      *
> +     * @implSpec
>      * <p>This implementation iterates over <tt>entrySet()</tt> searching for an
>      * entry with the specified key.  If such an entry is found, its value is
>      * obtained with its <tt>getValue</tt> operation, the entry is removed
> @@ -255,6 +262,7 @@
>     /**
>      * {@inheritDoc}
>      *
> +     * @implSpec
>      * <p>This implementation iterates over the specified map's
>      * <tt>entrySet()</tt> collection, and calls this map's <tt>put</tt>
>      * operation once for each entry returned by the iteration.
> @@ -276,6 +284,7 @@
>     /**
>      * {@inheritDoc}
>      *
> +     * @implSpec
>      * <p>This implementation calls <tt>entrySet().clear()</tt>.
>      *
>      * <p>Note that this implementation throws an
> @@ -302,6 +311,7 @@
>     /**
>      * {@inheritDoc}
>      *
> +     * @implSpec
>      * <p>This implementation returns a set that subclasses {@link AbstractSet}.
>      * The subclass's iterator method returns a "wrapper object" over this
>      * map's <tt>entrySet()</tt> iterator.  The <tt>size</tt> method
> @@ -358,6 +368,7 @@
>     /**
>      * {@inheritDoc}
>      *
> +     * @implSpec
>      * <p>This implementation returns a collection that subclasses {@link
>      * AbstractCollection}.  The subclass's iterator method returns a
>      * "wrapper object" over this map's <tt>entrySet()</tt> iterator.
> @@ -425,6 +436,7 @@
>      * <tt>equals</tt> method works properly across different implementations
>      * of the <tt>Map</tt> interface.
>      *
> +     * @implSpec
>      * <p>This implementation first checks if the specified object is this map;
>      * if so it returns <tt>true</tt>.  Then, it checks if the specified
>      * object is a map whose size is identical to the size of this map; if
> @@ -478,6 +490,7 @@
>      * <tt>m1</tt> and <tt>m2</tt>, as required by the general contract of
>      * {@link Object#hashCode}.
>      *
> +     * @implSpec
>      * <p>This implementation iterates over <tt>entrySet()</tt>, calling
>      * {@link Map.Entry#hashCode hashCode()} on each element (entry) in the
>      * set, and adding up the results.