> On 2 Jan 2014, at 22:02, Martin Buchholz <marti...@google.com> wrote: > > As the subject says, these changes should be applied to all of the so-called > "skeletal implementations" in java.util.
You may have noticed that I used a different subject line in the bug ;-) > There is a *lot* of (previously unavoidable (painful memories)) javadoc > duplication in java.util, some of which could now be undone. If you are agreeable, then I'd like to push just the AbstractMap changes as they are to JDK 9 and JDK 8. We can then immediately follow up with the additional skeletal types, under a different bug number for JDK 9, and evaluate the feasibility of putting it into JDK 8. This is just a matter of timing, it is getting very late in the JDK 8 release cycle. There is a specific problem in CHM stemming from AbstractMap, which I would like to resolve without growing the scope of the changes and risk. -Chris. > > >> On Thu, Jan 2, 2014 at 8:03 AM, Chris Hegarty <chris.hega...@oracle.com> >> wrote: >> 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 <d...@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. >
