On Oct 17, 2013, at 1:52 AM, Mike Duigou <mike.dui...@oracle.com> wrote:
> > On Oct 16 2013, at 05:34 , Paul Sandoz <paul.san...@oracle.com> wrote: > >> On Oct 16, 2013, at 1:52 PM, David Holmes <david.hol...@oracle.com> wrote: >>>>> Perhaps HashMap's implementations should throw CME? >>>>> >>>> >>>> Perhaps, seems to be going beyond the call of duty. My inclination is not >>>> to bother. It becomes most relevant with forEach since the consumer will >>>> have side-effects that might make it easier to unintentionally slip in a >>>> modification to the map itself. >>> >>> I think there is a lot to be said for consistency. >> >> Yes, i was proposing to consistently not support it for non-traversal >> methods :-) > > > I have prepared an updated webrev removing all of the non-traversal CME > throwing. > > http://cr.openjdk.java.net/~mduigou/JDK-8024688/2/webrev/ > Code looks good. Agree with David there is still some work to do cleaning up the doc. e.g.: On concurrent map this: * @implNote This implementation assumes that the ConcurrentMap cannot * contain null values and {@code get()} returning null unambiguously means * the key is absent. Implementations which support null values * <strong>must</strong> override this default implementation. and this variant of the same thing: * The default implementation assumes that the ConcurrentMap cannot * contain null values and get() returning null unambiguously means the key * is absent. Implementations which support null values * <strong>must</strong> override this default implementation. should be impl specs not notes. (It could also be pulled up into the class rather than repeating for each default method.) May get reformulated if you state "The default implementation is equivalent to...". For: /** * {@inheritDoc} * * @implNote The default implementation may retry these steps when multiple * threads attempt updates. * * The default implementation assumes that the ConcurrentMap cannot * contain null values and get() returning null unambiguously means the key * is absent. Implementations which support null values * <strong>must</strong> override this default implementation. * * @throws UnsupportedOperationException {@inheritDoc} * @throws ClassCastException {@inheritDoc} * @throws NullPointerException {@inheritDoc} * @since 1.8 */ @Override default V computeIfAbsent(K key, Function<? super K, ? extends V> mappingFunction) { Objects.requireNonNull(mappingFunction); V v, newValue; return ((v = get(key)) == null && (newValue = mappingFunction.apply(key)) != null && (v = putIfAbsent(key, newValue)) == null) ? newValue : v; } there is no retry. > It does bother me to be throwing out "good information" by not throwing the > CMEs but I'm willing to go with the flow. As a practical matter later > reintroduction of even valid error detection would almost certainly be > difficult. (https://bugs.openjdk.java.net/browse/JDK-5045147 for one example). > I still think we can solve that with a statement on Map (my preference is to be DRY). An impl supporting CME should override the defaults, which is not that different conceptually to what is said about synchronization/atomicity on Map or null values on ConcurrentMap. Paul. > The patch also fixes up missing @throws and @since from the ConcurrentMap > implementations. > > Mike >
