Hi Mike, Thanks very much for the careful explanation, it's appreciated. It does look trivial to fix, so I'll see about raising a ticket over there.
On 4 February 2014 17:35, Mike Duigou <mike.dui...@oracle.com> wrote: > The condition that is causing the problem is not a collection containing > null, which is allowed, but that the provided collection "c" is itself null. > > The problem was is a consequence of the following implementation: > > Iterator<E> iterator = iterator(); > > while(iterator.hasNext()) { > if(c.contains(iterator.next()) { > iterator.remove(); > } > } > > This implementation had an inconsistent behaviour. If a collection > contains elements then the first iteration of the while loops will cause a > NullPointerException if "c" is null. If, however, the collection is empty > then c is never referenced and no NPE is thrown. The change in java 8 which > adds an explicit Objects.requireNonNull(c) ensures the behaviour is > consistent whether the collection is empty or not. Passing null as the > parameter for removeAll and retainAll has never been valid. What's changed > is that it's now consistently checked. > > It's unfortunate that this particular fix tickled a bug in Antlr 3. It > appears that it's simply a bug that Antlr doesn't check the result of it's > method which returns null. ( > https://github.com/antlr/antlr3/blob/master/tool/src/main/java/org/antlr/tool/CompositeGrammar.java#L226) > The fix would seem to be relatively straightforward to either have > getDirectDelegates() return Collections.<Grammar>emptyList() or add a check > for null. My personal preference would be for the former as it allows for > uniform handling of the result wherever the method is called without having > to remember that the result might be null. > > Mike > > On Feb 4 2014, at 09:00 , Benedict Elliott Smith < > belliottsm...@datastax.com> wrote: > > > Hi, > > > > I notice this (or a related issue) has been mentioned > > before< > http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-June/017663.html > >on > > this list, but I'm not convinced the correct resolution was reached. > > We > > are seeing this problem thrown by antlr, but rather than a bug in antlr, > as > > surmised on the previous exchange, it looks to me that ArrayList is > > imposing a new constraint that is neither declared by itself nor > > Collection, and is unnecessary. ArrayList happily supports null elements, > > so requiring that the provided collection has no null elements is surely > a > > bug? > > > > I've pasted the two declarations below for ease of reference. Neither > > javadocs describe the constraint that is imposed. > > > > ArrayList: > > /** > > * Removes from this list all of its elements that are contained in > the > > * specified collection. > > * > > * @param c collection containing elements to be removed from this > list > > * @return {@code true} if this list changed as a result of the call > > * @throws ClassCastException if the class of an element of this list > > * is incompatible with the specified collection > > * (<a href="Collection.html#optional-restrictions">optional</a>) > > * @throws NullPointerException if this list contains a null element > > and the > > * specified collection does not permit null elements > > * (<a href="Collection.html#optional-restrictions">optional</a>), > > * or if the specified collection is null > > * @see Collection#contains(Object) > > */ > > public boolean removeAll(Collection<?> c) { > > Objects.requireNonNull(c); > > return batchRemove(c, false); > > } > > > > Collection: > > /** > > * Removes all of this collection's elements that are also contained > in > > the > > * specified collection (optional operation). After this call > returns, > > * this collection will contain no elements in common with the > specified > > * collection. > > * > > * @param c collection containing elements to be removed from this > > collection > > * @return <tt>true</tt> if this collection changed as a result of the > > * call > > * @throws UnsupportedOperationException if the <tt>removeAll</tt> > > method > > * is not supported by this collection > > * @throws ClassCastException if the types of one or more elements > > * in this collection are incompatible with the specified > > * collection > > * (<a href="#optional-restrictions">optional</a>) > > * @throws NullPointerException if this collection contains one or > more > > * null elements and the specified collection does not support > > * null elements > > * (<a href="#optional-restrictions">optional</a>), > > * or if the specified collection is null > > * @see #remove(Object) > > * @see #contains(Object) > > */ > > boolean removeAll(Collection<?> c); > >