[Mono-list] Array.cs

[Martin Baulig]

Paolo Molaro <lupus@ximian.com> writes:

> On 03/06/02 Martin Baulig wrote:
> > > Can you post some info about what kind of changes to the array code
> > > you want to do?
> > 
> > well, several things which are now all in CVS:
> 
> My reason for asking information was to see the changes _before_ they
> went in cvs. I hope I don't sound like a prick, but any change to
> fundamental classes most likely bites me before anyone else, because I
> routinely run the most complex C# app (mcs) with our class library.
> Excuse me if I'm a little paranoid with changes to Array.
> And while a simple bug added may not appear in the test suite or
> may be lost in the noise of hundreds of errors, it is likely to show up
> when running large or complex programs (ie I can't compile).

Hi,

I'm sorry for this, I got your mail after committing my changes.

> One problem is that the test suite needs to be changed to be more
> fine-grained: a single test method contains many subtests and if the
> first fails, the others are not run. This needs to be fixed.

Good point, I'll look at this.

> > 2.) Rewrote the Array:Copy implementation so that it can now copy multi-
> >     dimensional arrays.
> > 
> >     I'm also planning to implement this directly in C so that it can use
> >     memcpy() whenever possible.
> 
> See icall.c:ves_icall_System_Array_FastCopy().

Well, this internall is both unused and incomplete. It needs to check
whether both arrays are of the same type and use the "normal" copy if not.

> > 6.) Made sure that all tests in the test suite pass.
> 
> Several Array test fail for me here after updating to cvs (though they
> likely failed before as well).

Can you give me a list of which tests fail ?

However, my guess is that this is already fixed in CVS. Before
committing my changes, all Array tests passed. After committing, I
updated my tree, recompiled everything and realized that they do no
longer, but I was too tired to look at the problem. This morning, I
updated and recompiled again and now all of them pass again.

> This is in Array.CopyTo (). Here is the history of this line in the last
> few changes:
> 
> 1.19         (martin   01-Mar-02): 			if (index >= array.GetLength(0))
> 1.20         (lupus    04-Mar-02): 			if (index + this.GetLength(0) > array.GetLength(0))
> 1.21         (martin   05-Mar-02): 			if (index >= array.GetLength(0))
> 
> I.e. I changed that line a couple of days ago, because it broke the
> compiler and you changed it back (broking the compiler again).
> Please, tell me what test case your change fixes that mine wouldn't.

Ooops, you're right - your version is the correct one. I must have
been confused by the words in the docu and the fact that there was
indeed a test which failed here - but since it no longer does, I guess
that the actual bug was somewhere else.

I'll revert it back later on.

> Here is the test that your change breaks, instead.
> 
> using System;
> 
> namespace Test {
>         public class Test {
>                 public static int Main () {
>                         int[] src = new int [0];
>                         int[] dest = new int [0];
>                         src.CopyTo (dest, 0);
>                         return 0;
>                 }
>         }
> }

Well, you just found a bug in the documentation. Your code is
incorrect according to the documentation (it says the the exceptio is
thrown if either "index is equal to or greater than the length of
array" or "the number of element in the source array is greater than
the available space from index to the end of the destination array"),
but it works fine with the microsoft runtime.

So according to the docu, both checks must be done - but according to
the implementation, only your check is to be done.

I'll revert this back, add the test to the testsuite and a note to
API-notes.txt.

-- 
Martin Baulig
martin@gnome.org