[Mono-dev] [PATCH] String.Split() broken behavior

[mono-devel at tlarson.com]

Paolo, can you have a look at the revised patch? This should fix 
everything that was wrong with the previous version.

Paolo Molaro wrote:
>> --- mcs/class/corlib/System/String.cs	(revision 87045)
>> +++ mcs/class/corlib/System/String.cs	(working copy)
>> @@ -216,12 +216,11 @@
>>  			if (count == 1) 
>>  				return new String[1] { ToString() };
>>  
>> -			return InternalSplit (separator, count);
>> +			return InternalSplit (separator, count, 0);
>>  		}
>>  
>>  #if NET_2_0
>>  		[ComVisible (false)]
>> -		[MonoDocumentationNote ("optimization")]
>>     
>
> Since the code is not optimized, please don't remove this.
>
>   
On the contrary, the code IS optimized, which is why I removed it that 
attribute. In fact, the original code, to which that attribute applied, 
is no longer even there. This is a fundamentally new implementation.

In particular, the old code was a two-pass algorithm that scanned the 
string building an array of pieces, then scanned the array removing the 
empties. The new code is a single-pass algorithm that scans the string, 
building an array of pieces while leaving out the empties. The need for 
this optimization is exactly why the attribute was there to begin with. 
What the original author didn't know what that aside being inefficient, 
the original implementation also produced incorrect results. The 
optimization I wrote is also, therefore, a bugfix.

>> @@ -2461,7 +2441,7 @@
>>  		private extern void InternalCopyTo (int sIndex, char[] dest, int destIndex, int count);
>>  
>>  		[MethodImplAttribute (MethodImplOptions.InternalCall)]
>> -		private extern String[] InternalSplit (char[] separator, int count);
>> +		private extern String[] InternalSplit (char[] separator, int count, int options);
>>     
>
> A change in an icall signature requires a change to the corlib internal
> version.
>
>   
Fixed (I think). Please double-check my work and make sure I did it right.

> When the changes are made and the tests actually pass, since this is
> runtime code we need either an explicit signoff that the code is
> contributed under the X11 license or a copyright assignment.
>
> Sorry it took so long for the patch review.
>
> Thanks!
>
> lupus
>
>   
OK, then. This patch is hereby officially contributed under the X11 license.

-Tyler
-------------- next part --------------
A non-text attachment was scrubbed...
Name: string.patch
Type: text/x-patch
Size: 12899 bytes
Desc: not available
Url : http://lists.ximian.com/pipermail/mono-devel-list/attachments/20071018/065a353b/attachment.bin