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
>>
>
>
>
