LinkedList.clone() bug

[Brad Fitzpatrick]

>
> Thanks for the bug report. The patch isn't correct in the presence of
> subsclassing due to the use of new instead of clone - for example if you
had
> something like:
>
> class MyLinkedList extends LinkedList{}
>
> MyLinkedList l = new MyLinkedList();
> Object clone = l.clone();
>
> then the runtime type of the clone will be LinkedList instead of
MyLinkedList
> as would be expected. This is a mistake I've made in the past while
working on
> the collections classes ;-)
>
> Instead, I think the correct fix is something like:
>
> Index: LinkedList.java
> ===================================================================
> RCS file: /cvs/gcc/gcc/libjava/java/util/LinkedList.java,v
> retrieving revision 1.5
> diff -u -r1.5 LinkedList.java
> --- LinkedList.java     2000/12/04 10:20:00     1.5
> +++ LinkedList.java     2001/07/13 03:18:11
> @@ -474,7 +474,7 @@
>      catch (CloneNotSupportedException ex)
>        {
>        }
> -    copy.size = 0;
> +    copy.clear();
>      copy.addAll(this);
>      return copy;
>    }
>
> I havn't tested this, however. Could you try it out and get back to me?
>

Without testing, my guess is that won't work, as it was seeming like right
after the try/catch, (copy == this) was true.

I think the result of your change would be that the original object clone
was called on would be cleared, as would be the "cloned" object.  Consider
this:

java.lang.Object                   -- clone() throws
CloneNotSupportedException
     ||
java.util.AbstractCollection       -- no clone()
     ||
java.util.AbstractList             -- no clone()
     ||
java.util.AbstractSequentialList   -- no clone()
     ||
java.util.LinkedList:

public Object clone()
{
   LinkedList copy = null;
   try {
      copy = (LinkedList) super.clone()
   } catch (CloneNotSupportedException ex) { }

   copy.size = 0;
   copy.addAll(this);
   return copy;
}

To me, it would seem like clone() would throw an NullPointerException when
it did copy.size = 0, since copy should still be null?  Wouldn't
super.clone() throw an exception, since AbstractSequentialList has no
clone()?

But in my testcase, both l1 and the cloned l2 had their contents doubled,
which again suggests this == copy.

Maybe I don't understand what's going on (quite likely), but it seems as if
the LinkedList clone() is not quite right.

Also--- gcj-3.0 compiles my test case fine to bytecodes ... only the native
build is messed up.

In any case, I'll try out your fix and see if it works, even if I don't
understand why it should work.  :)

- Brad