Breaking Changeset: 4802647: Throw required NPEs from removeAll()/retainAll() (original) (raw)
Ali Ebrahimi ali.ebrahimi1781 at gmail.com
Tue Jun 4 10:08:11 UTC 2013
- Previous message: Breaking Changeset: 4802647: Throw required NPEs from removeAll()/retainAll()
- Next message: Breaking Changeset: 4802647: Throw required NPEs from removeAll()/retainAll()
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
What we this:
previous result: before changeset collection.removeAll(null) do nothing if collection is empty collection.removeAll(null) NullPointerException if collection is not empty
current result: after changeset collection.removeAll(null) NullPointerException if collection is empty collection.removeAll(null) NullPointerException if collection is not empty
solutions: solution1: backout/revert changeset
solution2: patch changeset
public boolean removeAll(Collection<?> c) {
if(!isEmpty()) Objects.requireNonNull(c);
boolean modified = false;
Iterator<?> it = iterator();
while (it.hasNext()) {
if (c.contains(it.next())) {
it.remove();
modified = true;
}
}
return modified;
}
public boolean retainAll(Collection<?> c) {
if(!isEmpty()) Objects.requireNonNull(c);
boolean modified = false;
Iterator<E> it = iterator();
while (it.hasNext()) {
if (!c.contains(it.next())) {
it.remove();
modified = true;
}
}
return modified;
}this is behaviorally compatible change.
On Tue, Jun 4, 2013 at 2:08 PM, Alan Bateman <Alan.Bateman at oracle.com>wrote:
On 04/06/2013 10:14, Ali Ebrahimi wrote:
:
the cause of this error is this new changeset: 4802647: Throw required NPEs from removeAll()/retainAll() current code assume that collection.removeAll(null) doesn't do anything. but with this changeset produces NullPointerException that doesn't handled. following is part of source code org/antlr/tool/**CompositeGrammar.java (see******** ) 217 /** Get delegates below direct delegates of g */ 218 public List getIndirectDelegates(Grammar g) { 219 List direct = getDirectDelegates(g); 220 List delegates = getDelegates(g); 221 delegates.removeAll(direct);********** 222 return delegates; 223 } 224 225 /** Return list of delegate grammars from root down to g. 226 * Order is root, ..., g.parent. (g not included). 227 */ 228 public List getDelegators(Grammar g) { 229 if ( g==delegateGrammarTreeRoot.**grammar ) { 230 return null;********** 231 } 232 List grammars = new ArrayList(); 233 CompositeGrammarTree t = delegateGrammarTreeRoot. **findNode(g); 234 // walk backwards to root, collecting grammars 235 CompositeGrammarTree p = t.parent; 236 while ( p!=null ) { 237 grammars.add(0, p.grammar); // add to head so in order later 238 p = p.parent; 239 } 240 return grammars; 241 } So this changeset at least breaks 'antlr' third-party library and any apps depends on. Thanks for the bug report. It does seem to have exposed a bug in antlr. Kevin Rushforth (FX) and Mike Duigou have been looking into the same thing via 8015656. -Alan.
- Previous message: Breaking Changeset: 4802647: Throw required NPEs from removeAll()/retainAll()
- Next message: Breaking Changeset: 4802647: Throw required NPEs from removeAll()/retainAll()
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]