JDK 9 RFR - 8031142 [was: Re: Using @implSpec in AbstractMap, AbstractCollection, AbstractList, etc]
Chris Hegarty
chris.hegarty at oracle.com
Mon Jan 6 16:47:08 UTC 2014
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.hegarty at oracle.com> wrote:
>
>> On 3 Jan 2014, at 02:14, Martin Buchholz <martinrb at 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.hegarty at oracle.com> wrote:
>>>
>>> On 2 Jan 2014, at 22:02, Martin Buchholz <martinrb at 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.hegarty at 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 <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.
>>>>
>>>>
>>>
>>
>
More information about the core-libs-dev
mailing list