On May 5, 2015, at 7:54 AM, Martin Buchholz <marti...@google.com> wrote:
> 
> One query in ConcurrentSkipListMap, we have:
> 
> 2500             // else use iterator
> 2501             @SuppressWarnings("unchecked") Iterator<Map.Entry<Object,E>> 
> it =
> 2502                     ((SubMap<Object,E>)m).entryIterator();
> 
> and then
> 
> 2578             // else use iterator
> 2579             Iterator<Map.Entry<K1,V1>> it = 
> ((SubMap<K1,V1>)m).entryIterator();
> 
> why does only the former require the "unchecked" warning suppression?
> 
> Good question.

Yes, for values the key type parameter is "thrown" away and reconstituting as 
Object is not type safe.


>   I think it's a small but clear improvement to consistently use K,V on view 
> subclasses, allowing a few @SuppressWarnings to be removed:
> 

AFAICT all but the above related suppress warning annotations could be removed 
if spliterator returning methods were on SubMap, rather than casting the result 
of the iterator based methods (see end of email for patch). I dunno if the 9 
compiler got smarter or code was updated and the annotation was not removed.

Your patch (plus addition of SubMap spliterator returning methods) seems like a 
good change to bring in bulk-wise, or otherwise earlier on if fixing another 
bug.

Paul.

diff -r b029e73f9cbb 
src/java.base/share/classes/java/util/concurrent/ConcurrentSkipListMap.java
--- 
a/src/java.base/share/classes/java/util/concurrent/ConcurrentSkipListMap.java   
    Tue May 05 09:43:27 2015 +0200
+++ 
b/src/java.base/share/classes/java/util/concurrent/ConcurrentSkipListMap.java   
    Tue May 05 11:10:41 2015 +0200
@@ -2400,12 +2400,12 @@
             Map.Entry<E,?> e = m.pollLastEntry();
             return (e == null) ? null : e.getKey();
         }
-        @SuppressWarnings("unchecked")
+//        @SuppressWarnings("unchecked")
         public Iterator<E> iterator() {
             if (m instanceof ConcurrentSkipListMap)
-                return ((ConcurrentSkipListMap<E,Object>)m).keyIterator();
+                return ((ConcurrentSkipListMap<E,>)m).keyIterator();
             else
-                return 
((ConcurrentSkipListMap.SubMap<E,Object>)m).keyIterator();
+                return ((ConcurrentSkipListMap.SubMap<E,?>)m).keyIterator();
         }
         public boolean equals(Object o) {
             if (o == this)
@@ -2451,12 +2451,12 @@
         public NavigableSet<E> descendingSet() {
             return new KeySet<E>(m.descendingMap());
         }
-        @SuppressWarnings("unchecked")
+//        @SuppressWarnings("unchecked")
         public Spliterator<E> spliterator() {
             if (m instanceof ConcurrentSkipListMap)
                 return ((ConcurrentSkipListMap<E,?>)m).keySpliterator();
             else
-                return (Spliterator<E>)((SubMap<E,?>)m).keyIterator();
+                return ((SubMap<E,?>)m).keySpliterator();
         }
     }
 
@@ -2465,7 +2465,7 @@
         Values(ConcurrentNavigableMap<?, E> map) {
             m = map;
         }
-        @SuppressWarnings("unchecked")
+//        @SuppressWarnings("unchecked")
         public Iterator<E> iterator() {
             if (m instanceof ConcurrentSkipListMap)
                 return ((ConcurrentSkipListMap<?,E>)m).valueIterator();
@@ -2486,12 +2486,12 @@
         }
         public Object[] toArray()     { return toList(this).toArray();  }
         public <T> T[] toArray(T[] a) { return toList(this).toArray(a); }
-        @SuppressWarnings("unchecked")
+//        @SuppressWarnings("unchecked")
         public Spliterator<E> spliterator() {
             if (m instanceof ConcurrentSkipListMap)
                 return ((ConcurrentSkipListMap<?,E>)m).valueSpliterator();
             else
-                return (Spliterator<E>)((SubMap<?,E>)m).valueIterator();
+                return ((SubMap<?,E>)m).valueSpliterator();
         }
         public boolean removeIf(Predicate<? super E> filter) {
             if (filter == null) throw new NullPointerException();
@@ -2516,7 +2516,7 @@
         EntrySet(ConcurrentNavigableMap<K1, V1> map) {
             m = map;
         }
-        @SuppressWarnings("unchecked")
+//        @SuppressWarnings("unchecked")
         public Iterator<Map.Entry<K1,V1>> iterator() {
             if (m instanceof ConcurrentSkipListMap)
                 return ((ConcurrentSkipListMap<K1,V1>)m).entryIterator();
@@ -2563,13 +2563,12 @@
         }
         public Object[] toArray()     { return toList(this).toArray();  }
         public <T> T[] toArray(T[] a) { return toList(this).toArray(a); }
-        @SuppressWarnings("unchecked")
+//        @SuppressWarnings("unchecked")
         public Spliterator<Map.Entry<K1,V1>> spliterator() {
             if (m instanceof ConcurrentSkipListMap)
                 return ((ConcurrentSkipListMap<K1,V1>)m).entrySpliterator();
             else
-                return (Spliterator<Map.Entry<K1,V1>>)
-                    ((SubMap<K1,V1>)m).entryIterator();
+                return ((SubMap<K1,V1>)m).entrySpliterator();
         }
         public boolean removeIf(Predicate<? super Entry<K1, V1>> filter) {
             if (filter == null) throw new NullPointerException();
@@ -3112,14 +3111,26 @@
             return new SubMapKeyIterator();
         }
 
+        Spliterator<K> keySpliterator() {
+            return new SubMapKeyIterator();
+        }
+
         Iterator<V> valueIterator() {
             return new SubMapValueIterator();
         }
 
+        Spliterator<V> valueSpliterator() {
+            return new SubMapValueIterator();
+        }
+
         Iterator<Map.Entry<K,V>> entryIterator() {
             return new SubMapEntryIterator();
         }
 
+        Spliterator<Entry<K,V>> entrySpliterator() {
+            return new SubMapEntryIterator();
+        }
+
         /**
          * Variant of main Iter class to traverse through submaps.
          * Also serves as back-up Spliterator for views

Reply via email to