re PR classpath/32844 (EnumSet causes code to be miscompiled)

2007-07-31  Dalibor Topic  <robilad@kaffe.org>

        PR 32844
        * java/util/EnumSet.java:
        Made class abstract per API spec.
        (size, iterator, add, addAll, clear, contains,
        containsAll, remove, removeAll, retainAll) Moved 
        into an anonymous inner class in of(T).
        (allOf, noneOf, copyOf, of, range) Made more 
        implementation independent.
        (of(T)) return an instance of an anonymous class
        implementing EmptySet.

From-SVN: r127291

libjava/classpath/ChangeLog

+2007-07-31  Dalibor Topic  <robilad@kaffe.org>
+
+	PR 32844
+	* java/util/EnumSet.java:
+	Made class abstract per API spec.
+	(size, iterator, add, addAll, clear, contains,
+	containsAll, remove, removeAll, retainAll) Moved 
+	into an anonymous inner class in of(T).
+	(allOf, noneOf, copyOf, of, range) Made more 
+	implementation independent.
+	(of(T)) return an instance of an anonymous class
+	implementing EmptySet.
+ 
 2007-07-28  Matthias Klose  <doko@ubuntu.com>
 
 	* include/jvmti.h(jniNativeInterface): Rename type.

libjava/classpath/java/util/EnumSet.java

@@ -43,16 +43,20 @@ import java.io.Serializable;
 /**
  * @author Tom Tromey (tromey@redhat.com)
  * @author Andrew John Hughes (gnu_andrew@member.fsf.org)
+ * @author Dalibor Topic (robilad@kaffe.org)
  * @since 1.5 
  */
 
-// FIXME: serialization is special.
-public class EnumSet<T extends Enum<T>>
+// FIXME: serialization is special, uses SerializationProxy.
+// of(E e) is the 'bottom' method that creates a real EnumSet.
+public abstract class EnumSet<T extends Enum<T>>
   extends AbstractSet<T>
   implements Cloneable, Serializable
 {
   private static final long serialVersionUID = 4782406773684236311L;
 
+  // These fields could go into the anonymous inner class in of(E),
+  // complementOf would need to be refactored then, though.
   BitSet store;
   int cardinality;
   Class<T> enumClass;
@@ -78,45 +82,59 @@ public class EnumSet<T extends Enum<T>>
     return r;
   }
 
-  public int size()
+  public static <T extends Enum<T>> EnumSet<T> allOf(Class<T> eltType)
   {
-    return cardinality;
+    // create an EnumSet from the list of values of the type
+    return copyOf(Arrays.asList(eltType.getEnumConstants()));
   }
 
-  public Iterator<T> iterator()
-  {
-    return new Iterator<T>()
-    {
-      int next = -1;
-      int count = 0;
-
-      public boolean hasNext()
+  public static <T extends Enum<T>> EnumSet<T> noneOf(Class<T> eltType)
   {
-	return count < cardinality;
+    return complementOf(allOf(eltType));
   }
 
-      public T next()
+  public static <T extends Enum<T>> EnumSet<T> copyOf(EnumSet<T> other)
   {
-	next = store.nextSetBit(next + 1);
-	++count;
-	return enumClass.getEnumConstants()[next];
+    return other.clone();
   }
 
-      public void remove()
+  public static <T extends Enum<T>> EnumSet<T> copyOf(Collection<T> other)
   {
-	if (! store.get(next))
+    if (other instanceof EnumSet)
+      return copyOf((EnumSet<T>) other);
+    if (other.isEmpty())
+	throw new IllegalArgumentException("Collection is empty");
+
+    EnumSet<T> r = null;
+
+    for (T val : other)
       {
-	    store.clear(next);
-	    --cardinality;
+	if (r == null)
+	  r = of(val);
+	else
+	  r.add(val);
       }
+
+    return r;
   }
-    };
+
+  public static <T extends Enum<T>> EnumSet<T> complementOf(EnumSet<T> other)
+  {
+    EnumSet<T> r = other.clone();
+    r.store.flip(0, r.store.size());
+    r.cardinality = r.store.size() - other.cardinality;
+    return r;
   }
 
+  public static <T extends Enum<T>> EnumSet<T> of(T first)
+  {
+    EnumSet<T> r = new EnumSet<T>()
+    {
       public boolean add(T val)
       {
 	if (store.get(val.ordinal()))
 	  return false;
+
 	store.set(val.ordinal());
 	++cardinality;
 	return true;
@@ -157,9 +175,11 @@ public class EnumSet<T extends Enum<T>>
       {
 	if (! (o instanceof Enum))
 	  return false;
+
 	Enum<T> e = (Enum<T>) o;
 	if (e.getDeclaringClass() != enumClass)
 	  return false;
+
 	return store.get(e.ordinal());
       }
 
@@ -170,18 +190,51 @@ public class EnumSet<T extends Enum<T>>
 	  EnumSet<T> other = (EnumSet<T>) c;
 	  if (enumClass == other.enumClass)
 	    return store.containsAll(other.store);
+
 	  return false;
 	}
 	return super.containsAll(c);
       }
 
+      public Iterator<T> iterator()
+      {
+	return new Iterator<T>()
+	{
+	  int next = -1;
+	  int count = 0;
+
+	  public boolean hasNext()
+	  {
+	    return count < cardinality;
+	  }
+
+	  public T next()
+	  {
+	    next = store.nextSetBit(next + 1);
+	    ++count;
+	    return enumClass.getEnumConstants()[next];
+	  }
+
+	  public void remove()
+	  {
+	    if (! store.get(next))
+	    {
+		store.clear(next);
+		--cardinality;
+	    }
+	  }
+	};
+      }
+
       public boolean remove(Object o)
       {
 	if (! (o instanceof Enum))
 	  return false;
+
 	Enum<T> e = (Enum<T>) o;
 	if (e.getDeclaringClass() != enumClass)
 	  return false;
+
 	store.clear(e.ordinal());
 	--cardinality;
 	return true;
@@ -194,6 +247,7 @@ public class EnumSet<T extends Enum<T>>
 	  EnumSet<T> other = (EnumSet<T>) c;
 	  if (enumClass != other.enumClass)
 	    return false;
+
 	  store.andNot(other.store);
 	  int save = cardinality;
 	  cardinality = store.cardinality();
@@ -209,6 +263,7 @@ public class EnumSet<T extends Enum<T>>
 	  EnumSet<T> other = (EnumSet<T>) c;
 	  if (enumClass != other.enumClass)
 	    return false;
+
 	  store.and(other.store);
 	  int save = cardinality;
 	  cardinality = store.cardinality();
@@ -217,137 +272,56 @@ public class EnumSet<T extends Enum<T>>
 	return super.retainAll(c);
       }
 
-  public static <T extends Enum<T>> EnumSet<T> allOf(Class<T> eltType)
-  {
-    EnumSet<T> r = new EnumSet<T>();
-    r.store = new BitSet(eltType.getEnumConstants().length);
-    r.store.set(0, r.store.size());
-    r.cardinality = r.store.size();
-    r.enumClass = eltType;
-    return r;
-  }
-
-  public static <T extends Enum<T>> EnumSet<T> noneOf(Class<T> eltType)
-  {
-    EnumSet<T> r = new EnumSet<T>();
-    r.store = new BitSet(eltType.getEnumConstants().length);
-    r.enumClass = eltType;
-    return r;
-  }
-
-  public static <T extends Enum<T>> EnumSet<T> copyOf(EnumSet<T> other)
-  {
-    // We can't just use `other.clone' since we don't want to make a
-    // subclass.
-    EnumSet<T> r = new EnumSet<T>();
-    r.store = (BitSet) other.store.clone();
-    r.cardinality = other.cardinality;
-    r.enumClass = other.enumClass;
-    return r;
-  }
-
-  public static <T extends Enum<T>> EnumSet<T> copyOf(Collection<T> other)
-  {
-    if (other instanceof EnumSet)
-      return copyOf((EnumSet<T>) other);
-    EnumSet<T> r = new EnumSet<T>();
-    for (T val : other)
-      {
-	if (r.store == null)
-	  {
-	    r.enumClass = val.getDeclaringClass();
-	    r.store = new BitSet(r.enumClass.getEnumConstants().length);
-	  }
-	r.store.set(val.ordinal());
-      }
-    // The collection must contain at least one element.
-    if (r.store == null)
-      throw new IllegalArgumentException();
-    r.cardinality = r.store.cardinality();
-    return r;
-  }
-
-  public static <T extends Enum<T>> EnumSet<T> complementOf(EnumSet<T> other)
+      public int size()
       {
-    EnumSet<T> r = new EnumSet<T>();
-    r.store = (BitSet) other.store.clone();
-    r.store.flip(0, r.store.size());
-    r.cardinality = r.store.size() - other.cardinality;
-    r.enumClass = other.enumClass;
-    return r;
+	return cardinality;
       }
+    };
 
-  public static <T extends Enum<T>> EnumSet<T> of(T first)
-  {
-    EnumSet<T> r = new EnumSet<T>();
+    // initialize the class
     r.enumClass = first.getDeclaringClass();
     r.store = new BitSet(r.enumClass.getEnumConstants().length);
-    r.store.set(first.ordinal());
-    r.cardinality = 1;
+
+    r.add(first);
     return r;
   }
 
   public static <T extends Enum<T>> EnumSet<T> of(T first, T second)
   {
-    EnumSet<T> r = new EnumSet<T>();
-    r.enumClass = first.getDeclaringClass();
-    r.store = new BitSet(r.enumClass.getEnumConstants().length);
-    r.store.set(first.ordinal());
-    r.store.set(second.ordinal());
-    r.cardinality = r.store.cardinality();
+    EnumSet<T> r = of(first);
+    r.add(second);
     return r;
   }
 
   public static <T extends Enum<T>> EnumSet<T> of(T first, T second, T third)
   {
-    EnumSet<T> r = new EnumSet<T>();
-    r.enumClass = first.getDeclaringClass();
-    r.store = new BitSet(r.enumClass.getEnumConstants().length);
-    r.store.set(first.ordinal());
-    r.store.set(second.ordinal());
-    r.store.set(third.ordinal());
-    r.cardinality = r.store.cardinality();
+    EnumSet<T> r = of(first, second);
+    r.add(third);
     return r;
   }
 
   public static <T extends Enum<T>> EnumSet<T> of(T first, T second, T third,
 						  T fourth)
   {
-    EnumSet<T> r = new EnumSet<T>();
-    r.enumClass = first.getDeclaringClass();
-    r.store = new BitSet(r.enumClass.getEnumConstants().length);
-    r.store.set(first.ordinal());
-    r.store.set(second.ordinal());
-    r.store.set(third.ordinal());
-    r.store.set(fourth.ordinal());
-    r.cardinality = r.store.cardinality();
+    EnumSet<T> r = of(first, second, third);
+    r.add(fourth);
     return r;
   }
 
   public static <T extends Enum<T>> EnumSet<T> of(T first, T second, T third,
 						  T fourth, T fifth)
   {
-    EnumSet<T> r = new EnumSet<T>();
-    r.enumClass = first.getDeclaringClass();
-    r.store = new BitSet(r.enumClass.getEnumConstants().length);
-    r.store.set(first.ordinal());
-    r.store.set(second.ordinal());
-    r.store.set(third.ordinal());
-    r.store.set(fourth.ordinal());
-    r.store.set(fifth.ordinal());
-    r.cardinality = r.store.cardinality();
+    EnumSet<T> r = of(first, second, third, fourth);
+    r.add(fifth);
     return r;
   }
 
   public static <T extends Enum<T>> EnumSet<T> of(T first, T... rest)
   {
-    EnumSet<T> r = new EnumSet<T>();
-    r.enumClass = first.getDeclaringClass();
-    r.store = new BitSet(r.enumClass.getEnumConstants().length);
-    r.store.set(first.ordinal());
+    EnumSet<T> r = noneOf(first.getDeclaringClass());
+    r.add(first);
     for (T val : rest)
-      r.store.set(val.ordinal());
-    r.cardinality = r.store.cardinality();
+      r.add(val);
     return r;
   }
 
@@ -355,11 +329,24 @@ public class EnumSet<T extends Enum<T>>
   {
     if (from.compareTo(to) > 0)
       throw new IllegalArgumentException();
-    EnumSet<T> r = new EnumSet<T>();
-    r.store = new BitSet(from.getDeclaringClass().getEnumConstants().length);
-    r.store.set(from.ordinal(), to.ordinal() + 1);
-    r.enumClass = from.getDeclaringClass();
-    r.cardinality = to.ordinal() - from.ordinal() + 1;
+    Class<T> type = from.getDeclaringClass();
+    EnumSet<T> r = noneOf(type);
+
+    T[] values = type.getEnumConstants();
+    // skip over values until start of range is found
+    int i = 0;
+    while (from != values[i])
+      i++;
+
+    // add values until end of range is found
+    while (to != values[i]) {
+      r.add(values[i]);
+      i++;
+    }
+
+    // add end of range
+    r.add(to);
+
     return r;
   }
 }

libjava/java/util/EnumSet$1.h

@@ -6,23 +6,25 @@
 
 #pragma interface
 
-#include <java/lang/Object.h>
+#include <java/util/EnumSet.h>
 
-class java::util::EnumSet$1 : public ::java::lang::Object
+class java::util::EnumSet$1 : public ::java::util::EnumSet
 {
 
 public: // actually package-private
-  EnumSet$1(::java::util::EnumSet *);
-public:
-  jboolean hasNext();
-  ::java::lang::Enum * EnumSet$1$next();
-  void remove();
-  ::java::lang::Object * next();
-public: // actually package-private
-  jint __attribute__((aligned(__alignof__( ::java::lang::Object)))) next__;
-  jint count;
-  ::java::util::EnumSet * this$0;
+  EnumSet$1();
 public:
+  jboolean EnumSet$1$add(::java::lang::Enum *);
+  jboolean addAll(::java::util::Collection *);
+  void clear();
+  jboolean contains(::java::lang::Object *);
+  jboolean containsAll(::java::util::Collection *);
+  ::java::util::Iterator * iterator();
+  jboolean remove(::java::lang::Object *);
+  jboolean removeAll(::java::util::Collection *);
+  jboolean retainAll(::java::util::Collection *);
+  jint size();
+  jboolean add(::java::lang::Object *);
   static ::java::lang::Class class$;
 };
 

libjava/java/util/EnumSet$2.h

+
+// DO NOT EDIT THIS FILE - it is machine generated -*- c++ -*-
+
+#ifndef __java_util_EnumSet$2__
+#define __java_util_EnumSet$2__
+
+#pragma interface
+
+#include <java/lang/Object.h>
+
+class java::util::EnumSet$2 : public ::java::lang::Object
+{
+
+public: // actually package-private
+  EnumSet$2(::java::util::EnumSet$1 *);
+public:
+  jboolean hasNext();
+  ::java::lang::Enum * EnumSet$2$next();
+  void remove();
+  ::java::lang::Object * next();
+public: // actually package-private
+  jint __attribute__((aligned(__alignof__( ::java::lang::Object)))) next__;
+  jint count;
+  ::java::util::EnumSet$1 * this$1;
+public:
+  static ::java::lang::Class class$;
+};
+
+#endif // __java_util_EnumSet$2__

libjava/java/util/EnumSet.h

@@ -17,16 +17,6 @@ public: // actually package-private
   EnumSet();
 public:
   virtual ::java::util::EnumSet * EnumSet$clone();
-  virtual jint size();
-  virtual ::java::util::Iterator * iterator();
-  virtual jboolean EnumSet$add(::java::lang::Enum *);
-  virtual jboolean addAll(::java::util::Collection *);
-  virtual void clear();
-  virtual jboolean contains(::java::lang::Object *);
-  virtual jboolean containsAll(::java::util::Collection *);
-  virtual jboolean remove(::java::lang::Object *);
-  virtual jboolean removeAll(::java::util::Collection *);
-  virtual jboolean retainAll(::java::util::Collection *);
   static ::java::util::EnumSet * allOf(::java::lang::Class *);
   static ::java::util::EnumSet * noneOf(::java::lang::Class *);
   static ::java::util::EnumSet * copyOf(::java::util::EnumSet *);
@@ -40,7 +30,6 @@ public:
   static ::java::util::EnumSet * of(::java::lang::Enum *, JArray< ::java::lang::Enum * > *);
   static ::java::util::EnumSet * range(::java::lang::Enum *, ::java::lang::Enum *);
   virtual ::java::lang::Object * clone();
-  virtual jboolean add(::java::lang::Object *);
 private:
   static const jlong serialVersionUID = 4782406773684236311LL;
 public: // actually package-private