On 04/19/2013 05:42 AM, David Holmes wrote:
On 19/04/2013 10:14 PM, Paul Sandoz wrote:

On Apr 19, 2013, at 1:15 PM, Alan Bateman <[email protected]>
wrote:

On 18/04/2013 19:49, Akhil Arora wrote:
Looks like the stars are aligning on getting on this into TL... the
refreshed webrev is -

http://cr.openjdk.java.net/~akhil/8001647.8/webrev/

A minor comment on Collection.removeIf is "that satisifies the given
predicate" might be better than "which matches the provided
predicate". Also for completeness, you could say "RuntimeExceptions
and Errors thrown by the predicate are propagated ...".

In List.replaceAll then @throws NullPointerException is listed twice,
which is okay, but might be better to combine them. A typo in the
second NPE description "if the an element".

In the implementation then the only thing that puzzled me is checking
the modification count in legacy Vector, that seems unnecessary.


The function value could structurally modify the Vector instance.

So this came through while I was writing my similar comments ...

My reaction to this is simply "well don't do that". If the
function/predicate/comparator is mutating the Vector then the user gets
what they deserve in my opinion. Trying to account for that is somewhat
futile. As per my other email the loop check for
modCount==expectedModCount will get hoisted from the loop. Further in
removeIf you need to be a lot more defensive during the first iteration
as you haven't kept a reference to the original size and array. That
aside the second part of removeIf (the actual removal) doesn't invoke
any external code so no concurrent modification is possible then.

This seems like overkill to me.

removed the in-loop modCount checks in the second part of removeIf for ArrayList and Vector

There are some tests that verify that a CME is thrown when a lambda modifies the collection it is operating upon. I added a count of how many times a lambda is called, and except for sort (which calls through to Arrays.sort), all such attempts are detected at the very first attempt (at index 0) so the checks seem to be working (and are not getting hoisted).

Reply via email to