Re: JDK 9 RFR - 8031142 [was: Re: Using @implSpec in AbstractMap, AbstractCollection, AbstractList, etc]
On 6 Jan 2014, at 21:19, Martin Buchholz marti...@google.com wrote: Your change looks good, except there's one more trailing p to remove. Thanks for the review. I found the trailing p and removed it. -Chris.
Re: JDK 9 RFR - 8031142 [was: Re: Using @implSpec in AbstractMap, AbstractCollection, AbstractList, etc]
On 6 Jan 2014, at 17:25, Martin Buchholz marti...@google.com wrote: On Mon, Jan 6, 2014 at 9:19 AM, Mike Duigou mike.dui...@oracle.com wrote: If you navigate through http://cr.openjdk.java.net/~chegar/8031142/specdiff/java/util/package-summary.html it shows just the relevant diffs. It appears that the specdiff was generated without an explicit --includes option which results in all the extra chaff. Thanks. I see a java/ subdirectory now, but I swear it wasn't there 17 minutes ago. It was possibly still uploading when I sent the mail. Sorry for the confusion. I created a large spec diff, all java.** and javax.**, to ensure that this change didn’t inadvertently affect anything else. -Chris.
Re: JDK 9 RFR - 8031142 [was: Re: Using @implSpec in AbstractMap, AbstractCollection, AbstractList, etc]
On 6 Jan 2014, at 17:05, Martin Buchholz marti...@google.com wrote:
On Mon, Jan 6, 2014 at 8:47 AM, Chris Hegarty chris.hega...@oracle.com
wrote:
Part 2...
JDK 9 RFR - 8031142: AbstractCollection and AbstractList should specify their
default implementation using @implSpec
http://cr.openjdk.java.net/~chegar/8031142/webrev/
http://cr.openjdk.java.net/~chegar/8031142/specdiff/
Is that specdiff link what you want? I just get a giant tree with javax
files in it...
---
In a sane language, the AbstractFoo classes would be traits - they should
never contribute to the *specification* of a concrete class, only to its
implementation. Therefore, in Java, all of the methods should have
precisely an {@inheritDoc} followed by an @implSpec. In particular, for
AbstractCollection.toArray I see:
114 /**
115 * {@inheritDoc}
116 *
117 * pThis method is equivalent to:
118 *
119 * pre {@code
120 * ListE list = new ArrayListE(size());
121 * for (E e : this)
122 * list.add(e);
123 * return list.toArray();
124 * }/pre
125 *
126 * @implSpec
127 * This implementation returns an array containing all the elements
128 * returned by this collection's iterator, in the same order, stored
in
129 * consecutive elements of the array, starting with index {@code 0}.
130 * The length of the returned array is equal to the number of
elements
131 * returned by the iterator, even if the size of this collection
changes
132 * during iteration, as might happen if the collection permits
133 * concurrent modification during iteration. The {@code size}
method is
134 * called only as an optimization hint; the correct result is
returned
135 * even if the iterator returns a different number of elements.
136 */
137 public Object[] toArray() {
which must be wrong. Either the sample code should be moved into the
@implSpec or promoted to Collection.java.toArray. The introduction of
default methods (not quite traits) makes the situation murkier. Looking
more closely, the exact wording cannot be promoted to Collection.java because
the behavior may in fact differ from the code sample. size() may or may not
be called. toArray implementation is more likely to be atomic, etc... So
move it into the @implSpec somehow…
I wasn’t quite sure about this. “This method is equivalent to, or, as if the
following was invoked …” without actually specifying the actual implementation.
But I agree, AbstractFoo should only have @inheritDoc or @implSpec. Let me see
what happens when I move it into @implSpec.
-Chris.
Re: JDK 9 RFR - 8031142 [was: Re: Using @implSpec in AbstractMap, AbstractCollection, AbstractList, etc]
If you navigate through
http://cr.openjdk.java.net/~chegar/8031142/specdiff/java/util/package-summary.html
it shows just the relevant diffs. It appears that the specdiff was generated
without an explicit --includes option which results in all the extra chaff.
Mike
On Jan 6 2014, at 09:05 , Martin Buchholz marti...@google.com wrote:
On Mon, Jan 6, 2014 at 8:47 AM, Chris Hegarty chris.hega...@oracle.com
wrote:
Part 2...
JDK 9 RFR - 8031142: AbstractCollection and AbstractList should specify their
default implementation using @implSpec
http://cr.openjdk.java.net/~chegar/8031142/webrev/
http://cr.openjdk.java.net/~chegar/8031142/specdiff/
Is that specdiff link what you want? I just get a giant tree with javax
files in it...
---
In a sane language, the AbstractFoo classes would be traits - they should
never contribute to the *specification* of a concrete class, only to its
implementation. Therefore, in Java, all of the methods should have
precisely an {@inheritDoc} followed by an @implSpec. In particular, for
AbstractCollection.toArray I see:
114 /**
115 * {@inheritDoc}
116 *
117 * pThis method is equivalent to:
118 *
119 * pre {@code
120 * ListE list = new ArrayListE(size());
121 * for (E e : this)
122 * list.add(e);
123 * return list.toArray();
124 * }/pre
125 *
126 * @implSpec
127 * This implementation returns an array containing all the elements
128 * returned by this collection's iterator, in the same order, stored
in
129 * consecutive elements of the array, starting with index {@code 0}.
130 * The length of the returned array is equal to the number of
elements
131 * returned by the iterator, even if the size of this collection
changes
132 * during iteration, as might happen if the collection permits
133 * concurrent modification during iteration. The {@code size}
method is
134 * called only as an optimization hint; the correct result is
returned
135 * even if the iterator returns a different number of elements.
136 */
137 public Object[] toArray() {
which must be wrong. Either the sample code should be moved into the
@implSpec or promoted to Collection.java.toArray. The introduction of
default methods (not quite traits) makes the situation murkier. Looking
more closely, the exact wording cannot be promoted to Collection.java because
the behavior may in fact differ from the code sample. size() may or may not
be called. toArray implementation is more likely to be atomic, etc... So
move it into the @implSpec somehow...
JDK 9 RFR - 8031142 [was: Re: Using @implSpec in AbstractMap, AbstractCollection, AbstractList, etc]
Part 2...
JDK 9 RFR - 8031142: AbstractCollection and AbstractList should specify
their default implementation using @implSpec
http://cr.openjdk.java.net/~chegar/8031142/webrev/
http://cr.openjdk.java.net/~chegar/8031142/specdiff/
-Chris.
On 06/01/14 15:37, Mike Duigou wrote:
Coming in late but this looks like a very good change to me as well.
Mike
On Jan 2 2014, at 23:06 , Chris Hegarty chris.hega...@oracle.com wrote:
On 3 Jan 2014, at 02:14, Martin Buchholz marti...@google.com wrote:
OK, as you wish - this change is clear progress!
Thanks Martin.
I pushed the AbstractMap changes to JDK 8 and JDK 9.
8031142 now tracks the changes to AbstractCollection and AbstractList
https://bugs.openjdk.java.net/browse/JDK-8031142
-Chris.
On Thu, Jan 2, 2014 at 3:48 PM, Chris Hegarty chris.hega...@oracle.com wrote:
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.javaThu Nov 21 09:23:03 2013
-0800
+++ b/src/share/classes/java/util/AbstractMap.javaSat Dec 28 08:33:42 2013
-0500
@@ -78,6 +78,7 @@
/**
* {@inheritDoc}
*
+ * @implSpec
* pThis implementation returns ttentrySet().size()/tt.
*/
public int size() {
@@ -87,6 +88,7 @@
/**
* {@inheritDoc}
*
+ * @implSpec
* pThis implementation returns ttsize() == 0/tt.
*/
public boolean isEmpty() {
@@ -96,6 +98,7 @@
/**
* {@inheritDoc}
*
+ * @implSpec
* pThis implementation iterates over ttentrySet()/tt searching
* for an entry with the specified value. If such an entry is found,
* tttrue/tt is returned. If the iteration terminates without
@@ -126,6 +129,7 @@
/**
* {@inheritDoc}
*
+ * @implSpec
* pThis implementation iterates over ttentrySet()/tt searching
* for an entry with the specified key. If such an entry is found,
* tttrue/tt is returned. If the iteration terminates without
@@ -157,6 +161,7 @@
Re: JDK 9 RFR - 8031142 [was: Re: Using @implSpec in AbstractMap, AbstractCollection, AbstractList, etc]
On 6 Jan 2014, at 18:11, Chris Hegarty chris.hega...@oracle.com wrote:
On 6 Jan 2014, at 17:05, Martin Buchholz marti...@google.com wrote:
……..
In a sane language, the AbstractFoo classes would be traits - they should
never contribute to the *specification* of a concrete class, only to its
implementation. Therefore, in Java, all of the methods should have
precisely an {@inheritDoc} followed by an @implSpec. In particular, for
AbstractCollection.toArray I see:
114 /**
115 * {@inheritDoc}
116 *
117 * pThis method is equivalent to:
118 *
119 * pre {@code
120 * ListE list = new ArrayListE(size());
121 * for (E e : this)
122 * list.add(e);
123 * return list.toArray();
124 * }/pre
125 *
126 * @implSpec
127 * This implementation returns an array containing all the elements
128 * returned by this collection's iterator, in the same order, stored
in
129 * consecutive elements of the array, starting with index {@code 0}.
130 * The length of the returned array is equal to the number of
elements
131 * returned by the iterator, even if the size of this collection
changes
132 * during iteration, as might happen if the collection permits
133 * concurrent modification during iteration. The {@code size}
method is
134 * called only as an optimization hint; the correct result is
returned
135 * even if the iterator returns a different number of elements.
136 */
137 public Object[] toArray() {
which must be wrong. Either the sample code should be moved into the
@implSpec or promoted to Collection.java.toArray. The introduction of
default methods (not quite traits) makes the situation murkier. Looking
more closely, the exact wording cannot be promoted to Collection.java
because the behavior may in fact differ from the code sample. size() may or
may not be called. toArray implementation is more likely to be atomic,
etc... So move it into the @implSpec somehow…
Done.
I wasn’t quite sure about this. “This method is equivalent to, or, as if the
following was invoked …” without actually specifying the actual
implementation. But I agree, AbstractFoo should only have @inheritDoc or
@implSpec. Let me see what happens when I move it into @implSpec.
Updated webrev and spec diff:
http://cr.openjdk.java.net/~chegar/8031142/specdiff.01/overview-summary.html
http://cr.openjdk.java.net/~chegar/8031142/webrev.01/webrev/
AbstractList retains its copy of the List add(E), clear(), and iterator()
method specifications. When I replaced them with {@inheritdoc}, the
Collection/AbstractCollection docs were copied in ( but we want the List docs
). I think this is a javadoc bug. I’ll look into this separately.
-Chris.
-Chris.
