Looks reasonable to me. Could BlockingDeque::push() just use {@inheritDoc} for
main body doc?
Mike
On Apr 29 2013, at 18:05 , Martin Buchholz wrote:
> Below is my proposed alternative fix, that also adds some missing @throws,
> reuses some existing wording, and makes small improvements to existing
> @throws specs (maintaining these specs is very tedious...)
>
> Index: src/main/java/util/Deque.java
> ===================================================================
> RCS file: /export/home/jsr166/jsr166/jsr166/src/main/java/util/Deque.java,v
> retrieving revision 1.24
> diff -u -U 15 -r1.24 Deque.java
> --- src/main/java/util/Deque.java 11 Feb 2013 17:27:45 -0000 1.24
> +++ src/main/java/util/Deque.java 30 Apr 2013 01:01:56 -0000
> @@ -155,51 +155,53 @@
> * methods, but instead inherit the identity-based versions from class
> * {@code Object}.
> *
> * <p>This interface is a member of the <a
> * href="{@docRoot}/../technotes/guides/collections/index.html"> Java
> Collections
> * Framework</a>.
> *
> * @author Doug Lea
> * @author Josh Bloch
> * @since 1.6
> * @param <E> the type of elements held in this collection
> */
> public interface Deque<E> extends Queue<E> {
> /**
> * Inserts the specified element at the front of this deque if it is
> - * possible to do so immediately without violating capacity restrictions.
> - * When using a capacity-restricted deque, it is generally preferable to
> - * use method {@link #offerFirst}.
> + * possible to do so immediately without violating capacity restrictions,
> + * throwing an {@code IllegalStateException} if no space is currently
> + * available. When using a capacity-restricted deque, it is generally
> + * preferable to use method {@link #offerFirst}.
> *
> * @param e the element to add
> * @throws IllegalStateException if the element cannot be added at this
> * time due to capacity restrictions
> * @throws ClassCastException if the class of the specified element
> * prevents it from being added to this deque
> * @throws NullPointerException if the specified element is null and this
> * deque does not permit null elements
> * @throws IllegalArgumentException if some property of the specified
> * element prevents it from being added to this deque
> */
> void addFirst(E e);
>
> /**
> * Inserts the specified element at the end of this deque if it is
> - * possible to do so immediately without violating capacity restrictions.
> - * When using a capacity-restricted deque, it is generally preferable to
> - * use method {@link #offerLast}.
> + * possible to do so immediately without violating capacity restrictions,
> + * throwing an {@code IllegalStateException} if no space is currently
> + * available. When using a capacity-restricted deque, it is generally
> + * preferable to use method {@link #offerLast}.
> *
> * <p>This method is equivalent to {@link #add}.
> *
> * @param e the element to add
> * @throws IllegalStateException if the element cannot be added at this
> * time due to capacity restrictions
> * @throws ClassCastException if the class of the specified element
> * prevents it from being added to this deque
> * @throws NullPointerException if the specified element is null and this
> * deque does not permit null elements
> * @throws IllegalArgumentException if some property of the specified
> * element prevents it from being added to this deque
> */
> void addLast(E e);
>
> @@ -441,32 +443,31 @@
> * returns {@code null} if this deque is empty.
> *
> * <p>This method is equivalent to {@link #peekFirst()}.
> *
> * @return the head of the queue represented by this deque, or
> * {@code null} if this deque is empty
> */
> E peek();
>
>
> // *** Stack methods ***
>
> /**
> * Pushes an element onto the stack represented by this deque (in other
> * words, at the head of this deque) if it is possible to do so
> - * immediately without violating capacity restrictions, returning
> - * {@code true} upon success and throwing an
> + * immediately without violating capacity restrictions, throwing an
> * {@code IllegalStateException} if no space is currently available.
> *
> * <p>This method is equivalent to {@link #addFirst}.
> *
> * @param e the element to push
> * @throws IllegalStateException if the element cannot be added at this
> * time due to capacity restrictions
> * @throws ClassCastException if the class of the specified element
> * prevents it from being added to this deque
> * @throws NullPointerException if the specified element is null and this
> * deque does not permit null elements
> * @throws IllegalArgumentException if some property of the specified
> * element prevents it from being added to this deque
> */
> void push(E e);
> Index: src/main/java/util/concurrent/BlockingDeque.java
> ===================================================================
> RCS file:
> /export/home/jsr166/jsr166/jsr166/src/main/java/util/concurrent/BlockingDeque.java,v
> retrieving revision 1.25
> diff -u -U 15 -r1.25 BlockingDeque.java
> --- src/main/java/util/concurrent/BlockingDeque.java 11 Feb 2013 17:27:45
> -0000 1.25
> +++ src/main/java/util/concurrent/BlockingDeque.java 30 Apr 2013 01:01:57
> -0000
> @@ -596,28 +596,29 @@
> * @return the number of elements in this deque
> */
> public int size();
>
> /**
> * Returns an iterator over the elements in this deque in proper
> sequence.
> * The elements will be returned in order from first (head) to last
> (tail).
> *
> * @return an iterator over the elements in this deque in proper sequence
> */
> Iterator<E> iterator();
>
> // *** Stack methods ***
>
> /**
> - * Pushes an element onto the stack represented by this deque. In other
> - * words, inserts the element at the front of this deque unless it would
> - * violate capacity restrictions.
> + * Pushes an element onto the stack represented by this deque (in other
> + * words, at the head of this deque) if it is possible to do so
> + * immediately without violating capacity restrictions, throwing an
> + * {@code IllegalStateException} if no space is currently available.
> *
> * <p>This method is equivalent to {@link #addFirst(Object) addFirst}.
> *
> * @throws IllegalStateException {@inheritDoc}
> * @throws ClassCastException {@inheritDoc}
> * @throws NullPointerException if the specified element is null
> * @throws IllegalArgumentException {@inheritDoc}
> */
> void push(E e);
> }
> Index: src/main/java/util/concurrent/ConcurrentLinkedDeque.java
> ===================================================================
> RCS file:
> /export/home/jsr166/jsr166/jsr166/src/main/java/util/concurrent/ConcurrentLinkedDeque.java,v
> retrieving revision 1.43
> diff -u -U 15 -r1.43 ConcurrentLinkedDeque.java
> --- src/main/java/util/concurrent/ConcurrentLinkedDeque.java 27 Mar 2013
> 19:46:34 -0000 1.43
> +++ src/main/java/util/concurrent/ConcurrentLinkedDeque.java 30 Apr 2013
> 01:01:57 -0000
> @@ -989,35 +989,51 @@
> }
>
> /**
> * Inserts the specified element at the tail of this deque.
> * As the deque is unbounded, this method will never throw
> * {@link IllegalStateException} or return {@code false}.
> *
> * @return {@code true} (as specified by {@link Collection#add})
> * @throws NullPointerException if the specified element is null
> */
> public boolean add(E e) {
> return offerLast(e);
> }
>
> public E poll() { return pollFirst(); }
> - public E remove() { return removeFirst(); }
> public E peek() { return peekFirst(); }
> +
> + /**
> + * @throws NoSuchElementException {@inheritDoc}
> + */
> + public E remove() { return removeFirst(); }
> +
> + /**
> + * @throws NoSuchElementException {@inheritDoc}
> + */
> + public E pop() { return removeFirst(); }
> +
> + /**
> + * @throws NoSuchElementException {@inheritDoc}
> + */
> public E element() { return getFirst(); }
> +
> + /**
> + * @throws NullPointerException {@inheritDoc}
> + */
> public void push(E e) { addFirst(e); }
> - public E pop() { return removeFirst(); }
>
> /**
> * Removes the first element {@code e} such that
> * {@code o.equals(e)}, if such an element exists in this deque.
> * If the deque does not contain the element, it is unchanged.
> *
> * @param o element to be removed from this deque, if present
> * @return {@code true} if the deque contained the specified element
> * @throws NullPointerException if the specified element is null
> */
> public boolean removeFirstOccurrence(Object o) {
> checkNotNull(o);
> for (Node<E> p = first(); p != null; p = succ(p)) {
> E item = p.item;
> if (item != null && o.equals(item) && p.casItem(item, null)) {
> Index: src/main/java/util/concurrent/LinkedBlockingDeque.java
> ===================================================================
> RCS file:
> /export/home/jsr166/jsr166/jsr166/src/main/java/util/concurrent/LinkedBlockingDeque.java,v
> retrieving revision 1.44
> diff -u -U 15 -r1.44 LinkedBlockingDeque.java
> --- src/main/java/util/concurrent/LinkedBlockingDeque.java 27 Mar 2013
> 19:46:34 -0000 1.44
> +++ src/main/java/util/concurrent/LinkedBlockingDeque.java 30 Apr 2013
> 01:01:57 -0000
> @@ -280,40 +280,40 @@
> unlinkLast();
> } else {
> p.next = n;
> n.prev = p;
> x.item = null;
> // Don't mess with x's links. They may still be in use by
> // an iterator.
> --count;
> notFull.signal();
> }
> }
>
> // BlockingDeque methods
>
> /**
> - * @throws IllegalStateException {@inheritDoc}
> - * @throws NullPointerException {@inheritDoc}
> + * @throws IllegalStateException if this deque is full
> + * @throws NullPointerException {@inheritDoc}
> */
> public void addFirst(E e) {
> if (!offerFirst(e))
> throw new IllegalStateException("Deque full");
> }
>
> /**
> - * @throws IllegalStateException {@inheritDoc}
> + * @throws IllegalStateException if this deque is full
> * @throws NullPointerException {@inheritDoc}
> */
> public void addLast(E e) {
> if (!offerLast(e))
> throw new IllegalStateException("Deque full");
> }
>
> /**
> * @throws NullPointerException {@inheritDoc}
> */
> public boolean offerFirst(E e) {
> if (e == null) throw new NullPointerException();
> Node<E> node = new Node<E>(e);
> final ReentrantLock lock = this.lock;
> lock.lock();
> @@ -588,32 +588,31 @@
> return false;
> } finally {
> lock.unlock();
> }
> }
>
> // BlockingQueue methods
>
> /**
> * Inserts the specified element at the end of this deque unless it would
> * violate capacity restrictions. When using a capacity-restricted
> deque,
> * it is generally preferable to use method {@link #offer(Object) offer}.
> *
> * <p>This method is equivalent to {@link #addLast}.
> *
> - * @throws IllegalStateException if the element cannot be added at this
> - * time due to capacity restrictions
> + * @throws IllegalStateException if this deque is full
> * @throws NullPointerException if the specified element is null
> */
> public boolean add(E e) {
> addLast(e);
> return true;
> }
>
> /**
> * @throws NullPointerException if the specified element is null
> */
> public boolean offer(E e) {
> return offerLast(e);
> }
>
> /**
> @@ -726,32 +725,32 @@
> try {
> int n = Math.min(maxElements, count);
> for (int i = 0; i < n; i++) {
> c.add(first.item); // In this order, in case add() throws.
> unlinkFirst();
> }
> return n;
> } finally {
> lock.unlock();
> }
> }
>
> // Stack methods
>
> /**
> - * @throws IllegalStateException {@inheritDoc}
> - * @throws NullPointerException {@inheritDoc}
> + * @throws IllegalStateException if this deque is full
> + * @throws NullPointerException {@inheritDoc}
> */
> public void push(E e) {
> addFirst(e);
> }
>
> /**
> * @throws NoSuchElementException {@inheritDoc}
> */
> public E pop() {
> return removeFirst();
> }
>
> // Collection methods
>
> /**
> @@ -817,31 +816,31 @@
> * collection, especially when count is close to capacity.
> */
>
> // /**
> // * Adds all of the elements in the specified collection to this
> // * queue. Attempts to addAll of a queue to itself result in
> // * {@code IllegalArgumentException}. Further, the behavior of
> // * this operation is undefined if the specified collection is
> // * modified while the operation is in progress.
> // *
> // * @param c collection containing elements to be added to this queue
> // * @return {@code true} if this queue changed as a result of the call
> // * @throws ClassCastException {@inheritDoc}
> // * @throws NullPointerException {@inheritDoc}
> // * @throws IllegalArgumentException {@inheritDoc}
> -// * @throws IllegalStateException {@inheritDoc}
> +// * @throws IllegalStateException if this deque is full
> // * @see #add(Object)
> // */
> // public boolean addAll(Collection<? extends E> c) {
> // if (c == null)
> // throw new NullPointerException();
> // if (c == this)
> // throw new IllegalArgumentException();
> // final ReentrantLock lock = this.lock;
> // lock.lock();
> // try {
> // boolean modified = false;
> // for (E e : c)
> // if (linkLast(e))
> // modified = true;
> // return modified;
>
>
>
> On Mon, Apr 29, 2013 at 4:56 PM, Mike Duigou <mike.dui...@oracle.com> wrote:
> OK, I will wait on that and hopefully Chris can pick it up on the next jsr166
> sync.
>
> Mike
>
> On Apr 29 2013, at 16:43 , Martin Buchholz wrote:
>
>> As always for changes to files maintained in jsr166 CVS, we'd like changes
>> to flow through jsr166 CVS into openjdk proper.
>>
>> In this case, there are some issues with missing exception specs in
>> implementing classes. I'll come up with a diff.
>>
>> Martin
>>
>>
>> On Fri, Apr 26, 2013 at 5:12 PM, Mike Duigou <mike.dui...@oracle.com> wrote:
>> Hello all;
>>
>> A very small change to review.
>>
>> http://cr.openjdk.java.net/~mduigou/JDK-7178639/0/webrev/
>>
>> The change removes some erroneous documentation from the Deque push() method.
>>
>> Mike
>>
>
>
