Hi Andrew, You're right about emptyList() invocation from subList. Thanks.
Updated (and rebased) commit: https://github.com/ivmai/classpath/commit/ef3af4f38970f7cea3d5b5a7d98a617f7b23da4e (branch: https://github.com/ivmai/classpath/tree/ivmai4review-v3) diff --git a/ChangeLog b/ChangeLog index f78f393..5805463 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,19 @@ +2012-10-16 Ivan Maidanski <iv...@mail.ru> + + * java/util/Collections.java: + (emptySet(), EmptySet.iterator(), emptyList(), emptyMap(), + EmptyMap.entrySet(), EmptyMap.keySet(), EmptyMap.values()): Suppress + "unchecked" warnings. + (emptySet(), emptyList(), emptyMap()): Don't create new instance (use + the corresponding immutable container instance); remove FIXME. + (EmptySet.equals(Object), EmptyList.equals(Object), + EmptyMap.entrySet(), EmptyMap.equals(Object), EmptyMap.keySet(), + EmptyMap.values()): Add generic typing. + (SingletonList.subList(int, int)): Use emptyList() instead of + EMPTY_LIST (to eliminate "unchecked" warning). + (SynchronizedCollection.toArray(T[])): Rename T type to E (to + suppress compiler warning about type hiding). + 2012-10-15 Andrew John Hughes <gnu_and...@member.fsf.org> * configure.ac: Set to 0.99.1pre, as diff --git a/java/util/Collections.java b/java/util/Collections.java index 828c6ec..b970dd8 100644 --- a/java/util/Collections.java +++ b/java/util/Collections.java @@ -120,10 +120,10 @@ public class Collections * @return an empty parameterized set. * @since 1.5 */ + @SuppressWarnings("unchecked") public static final <T> Set<T> emptySet() { - /* FIXME: Could this be optimized? */ - return new EmptySet<T>(); + return (Set<T>) EMPTY_SET; } /** @@ -161,6 +161,7 @@ public class Collections * @return A non-iterating iterator. */ // This is really cheating! I think it's perfectly valid, though. + @SuppressWarnings("unchecked") public Iterator<T> iterator() { return (Iterator<T>) EMPTY_LIST.iterator(); @@ -196,7 +197,7 @@ public class Collections */ public boolean equals(Object o) { - return o instanceof Set && ((Set) o).isEmpty(); + return o instanceof Set<?> && ((Set<?>) o).isEmpty(); } /** @@ -288,10 +289,10 @@ public class Collections * @return an empty parameterized list. * @since 1.5 */ + @SuppressWarnings("unchecked") public static final <T> List<T> emptyList() { - /* FIXME: Could this be optimized? */ - return new EmptyList<T>(); + return (List<T>) EMPTY_LIST; } /** @@ -369,7 +370,7 @@ public class Collections */ public boolean equals(Object o) { - return o instanceof List && ((List) o).isEmpty(); + return o instanceof List<?> && ((List<?>) o).isEmpty(); } /** @@ -480,10 +481,10 @@ public class Collections * @return an empty parameterized map. * @since 1.5 */ + @SuppressWarnings("unchecked") public static final <K,V> Map<K,V> emptyMap() { - /* FIXME: Could this be optimized? */ - return new EmptyMap<K,V>(); + return (Map<K,V>) EMPTY_MAP; } /** @@ -511,9 +512,10 @@ public class Collections * There are no entries. * @return The empty set. */ + @SuppressWarnings("unchecked") public Set<Map.Entry<K, V>> entrySet() { - return EMPTY_SET; + return (Set<Map.Entry<K, V>>) EMPTY_SET; } // The remaining methods are optional, but provide a performance @@ -546,7 +548,7 @@ public class Collections */ public boolean equals(Object o) { - return o instanceof Map && ((Map) o).isEmpty(); + return o instanceof Map<?,?> && ((Map<?,?>) o).isEmpty(); } /** @@ -572,9 +574,10 @@ public class Collections * No entries. * @return The empty set. */ + @SuppressWarnings("unchecked") public Set<K> keySet() { - return EMPTY_SET; + return (Set<K>) EMPTY_SET; } /** @@ -601,9 +604,10 @@ public class Collections * Collection, will work. Besides, that's what the JDK uses! * @return The empty set. */ + @SuppressWarnings("unchecked") public Collection<V> values() { - return EMPTY_SET; + return (Collection<V>) EMPTY_SET; } /** @@ -1854,7 +1858,7 @@ public class Collections public List<T> subList(int from, int to) { if (from == to && (to == 0 || to == 1)) - return EMPTY_LIST; + return emptyList(); if (from == 0 && to == 1) return this; if (from > to) @@ -2480,7 +2484,7 @@ public class Collections * @throws ArrayStoreException if the type of any element of the * collection is not a subtype of the element type of a. */ - public <T> T[] toArray(T[] a) + public <E> E[] toArray(E[] a) { synchronized (mutex) { Regards, Ivan Mon, 15 Oct 2012 06:09:57 -0400 (EDT) от Andrew Hughes <ahug...@redhat.com>: > > > > > > > ----- Original Message ----- > > From: Ivan Maidanski <iv...@mail.ru> > > > > 2011-07-20 Ivan Maidanski <iv...@mail.ru> > > > > * java/util/Collections.java: > > (emptySet(), EmptySet.iterator(), emptyList(), emptyMap(), > > EmptyMap.entrySet(), EmptyMap.keySet(), EmptyMap.values(), > > SingletonList.subList(int, int)): Suppress "unchecked" warnings. > > (emptySet(), emptyList(), emptyMap()): Don't create new instance > > (use > > the corresponding immutable container instance); remove FIXME. > > (EmptySet.equals(Object), EmptyList.equals(Object), > > EmptyMap.entrySet(), EmptyMap.equals(Object), EmptyMap.keySet(), > > EmptyMap.values(), SingletonList.subList(int, int)): Add generic > > typing. > > (SynchronizedCollection.toArray(T[])): Rename T type to E (to > > suppress compiler warning about type hiding). > > --- > > ChangeLog | 15 +++++++++++++++ > > java/util/Collections.java | 33 +++++++++++++++++++-------------- > > 2 files changed, 34 insertions(+), 14 deletions(-) > > > > diff --git a/ChangeLog b/ChangeLog > > index c0d84cd..5690754 100644 > > --- a/ChangeLog > > +++ b/ChangeLog > > @@ -1,5 +1,20 @@ > > 2011-07-20 Ivan Maidanski <iv...@mail.ru> > > > > + * java/util/Collections.java: > > + (emptySet(), EmptySet.iterator(), emptyList(), emptyMap(), > > + EmptyMap.entrySet(), EmptyMap.keySet(), EmptyMap.values(), > > + SingletonList.subList(int, int)): Suppress "unchecked" warnings. > > + (emptySet(), emptyList(), emptyMap()): Don't create new instance > > (use > > + the corresponding immutable container instance); remove FIXME. > > + (EmptySet.equals(Object), EmptyList.equals(Object), > > + EmptyMap.entrySet(), EmptyMap.equals(Object), EmptyMap.keySet(), > > + EmptyMap.values(), SingletonList.subList(int, int)): Add generic > > + typing. > > + (SynchronizedCollection.toArray(T[])): Rename T type to E (to > > + suppress compiler warning about type hiding). > > + > > +2011-07-20 Ivan Maidanski <iv...@mail.ru> > > + > > * native/jni/java-util/java_util_VMTimeZone.c: > > Include jcl.h file. > > (Java_java_util_VMTimeZone_getSystemTimeZoneId): Throw > > diff --git a/java/util/Collections.java b/java/util/Collections.java > > index 828c6ec..e7e7056 100644 > > --- a/java/util/Collections.java > > +++ b/java/util/Collections.java > > @@ -120,10 +120,10 @@ public class Collections > > * @return an empty parameterized set. > > * @since 1.5 > > */ > > + @SuppressWarnings("unchecked") > > public static final <T> Set<T> emptySet() > > { > > - /* FIXME: Could this be optimized? */ > > - return new EmptySet<T>(); > > + return (Set<T>) EMPTY_SET; > > } > > This is ugly, but it seems to be the right thing according to the Javadoc: > > "Implementation note: Implementations of this method need not create a separate Set object for each call. Using this method is likely to have comparable cost to using the like-named field. (Unlike this method, the field does not provide type safety." > > > > > /** > > @@ -161,6 +161,7 @@ public class Collections > > * @return A non-iterating iterator. > > */ > > // This is really cheating! I think it's perfectly valid, > > though. > > + @SuppressWarnings("unchecked") > > public Iterator<T> iterator() > > { > > return (Iterator<T>) EMPTY_LIST.iterator(); > > @@ -196,7 +197,7 @@ public class Collections > > */ > > public boolean equals(Object o) > > { > > - return o instanceof Set && ((Set) o).isEmpty(); > > + return o instanceof Set<?> && ((Set<?>) o).isEmpty(); > > } > > > > /** > > @@ -288,10 +289,10 @@ public class Collections > > * @return an empty parameterized list. > > * @since 1.5 > > */ > > + @SuppressWarnings("unchecked") > > public static final <T> List<T> emptyList() > > { > > - /* FIXME: Could this be optimized? */ > > - return new EmptyList<T>(); > > + return (List<T>) EMPTY_LIST; > > } > > > > /** > > @@ -369,7 +370,7 @@ public class Collections > > */ > > public boolean equals(Object o) > > { > > - return o instanceof List && ((List) o).isEmpty(); > > + return o instanceof List<?> && ((List<?>) o).isEmpty(); > > } > > > > /** > > @@ -480,10 +481,10 @@ public class Collections > > * @return an empty parameterized map. > > * @since 1.5 > > */ > > + @SuppressWarnings("unchecked") > > public static final <K,V> Map<K,V> emptyMap() > > { > > - /* FIXME: Could this be optimized? */ > > - return new EmptyMap<K,V>(); > > + return (Map<K,V>) EMPTY_MAP; > > } > > > > /** > > @@ -511,9 +512,10 @@ public class Collections > > * There are no entries. > > * @return The empty set. > > */ > > + @SuppressWarnings("unchecked") > > public Set<Map.Entry<K, V>> entrySet() > > { > > - return EMPTY_SET; > > + return (Set<Map.Entry<K, V>>) EMPTY_SET; > > } > > > > // The remaining methods are optional, but provide a performance > > @@ -546,7 +548,7 @@ public class Collections > > */ > > public boolean equals(Object o) > > { > > - return o instanceof Map && ((Map) o).isEmpty(); > > + return o instanceof Map<?,?> && ((Map<?,?>) o).isEmpty(); > > } > > > > /** > > @@ -572,9 +574,10 @@ public class Collections > > * No entries. > > * @return The empty set. > > */ > > + @SuppressWarnings("unchecked") > > public Set<K> keySet() > > { > > - return EMPTY_SET; > > + return (Set<K>) EMPTY_SET; > > } > > > > /** > > @@ -601,9 +604,10 @@ public class Collections > > * Collection, will work. Besides, that's what the JDK uses! > > * @return The empty set. > > */ > > + @SuppressWarnings("unchecked") > > public Collection<V> values() > > { > > - return EMPTY_SET; > > + return (Collection<V>) EMPTY_SET; > > } > > > > /** > > @@ -1851,10 +1855,11 @@ public class Collections > > * @throws IndexOutOfBoundsException if either bound is greater > > * than 1. > > */ > > + @SuppressWarnings("unchecked") > > public List<T> subList(int from, int to) > > { > > if (from == to && (to == 0 || to == 1)) > > - return EMPTY_LIST; > > + return (List<T>) EMPTY_LIST; > > if (from == 0 && to == 1) > > return this; > > if (from > to) > > This seems wrong. The scope of unchecked is far too wide. Couldn't emptyList() > be used? > > > @@ -2480,7 +2485,7 @@ public class Collections > > * @throws ArrayStoreException if the type of any element of the > > * collection is not a subtype of the element type of a. > > */ > > - public <T> T[] toArray(T[] a) > > + public <E> E[] toArray(E[] a) > > { > > synchronized (mutex) > > { > > -- > > 1.7.7.6 > > > > > > > > -- > Andrew :) > > Free Java Software Engineer > Red Hat, Inc. (http://www.redhat.com) > > PGP Key: 248BDC07 (https://keys.indymedia.org/) > Fingerprint = EC5A 1F5E C0AD 1D15 8F1F 8F91 3B96 A578 248B DC07 > > >
