[Mono-devel-list] StringBuilder patch

[Paolo Molaro]

On 12/12/03 Torstensson, Patrik wrote:
> diff -u -r1.108 object.h
> --- object.h	7 Dec 2003 13:12:09 -0000	1.108
> +++ object.h	12 Dec 2003 21:21:01 -0000
> @@ -112,9 +112,8 @@
>  
>  typedef struct {
>  	MonoObject object;
> -	gint32 capacity;
>  	gint32 length;
> -	MonoArray *chars;
> +	MonoString *str;
>  } MonoStringBuilder;
>  
>  typedef struct {
> Index: marshal.c
> ===================================================================
> RCS file: /cvs/public/mono/mono/metadata/marshal.c,v
> retrieving revision 1.130
> diff -u -r1.130 marshal.c
> --- marshal.c	9 Dec 2003 14:30:48 -0000	1.130
> +++ marshal.c	12 Dec 2003 21:21:05 -0000
> @@ -167,12 +167,12 @@
>  	l = strlen (text);
>  
>  	ut = g_utf8_to_utf16 (text, l, NULL, &items_written, &error);
> -
> -	if (items_written > sb->capacity)
> -		items_written = sb->capacity;
> +	
> +	if (items_written > sb->str->length)
> +		items_written = sb->str->length;

Add a comment there, something like:
	/* sb->str->length is the StringBuilder capacity */
or, maybe better, add a macro to the header file and use that:
#define mono_stringbuilder_capacity(sb) ((sb)->str->length)

> Index: System/String.cs
> ===================================================================
> RCS file: /cvs/public/mcs/class/corlib/System/String.cs,v
> retrieving revision 1.98
> diff -u -r1.98 String.cs
> --- System/String.cs	6 Dec 2003 16:54:59 -0000	1.98
> +++ System/String.cs	12 Dec 2003 23:43:26 -0000
[...]
> @@ -1197,7 +1197,26 @@
>  			ptr = p;
>  			return n;
>  		}
> -		
> +
> +		internal unsafe void InternalSetChar(int idx, char val) 
> +		{
> +			if ((UInt32) idx >= (UInt32) Length)

Use:
			if ((uint) idx >= (uint) length)

> diff -u -r1.26 StringBuilder.cs
> --- System.Text/StringBuilder.cs	17 Nov 2003 22:42:53 -0000	1.26
> +++ System.Text/StringBuilder.cs	12 Dec 2003 23:43:26 -0000
[...]
> -		public override string ToString() {
> -			if (thestring != null)
> -				return thestring;
> -			return (thestring = ToString(0, sLength));
> +		public override string ToString () 
> +		{
> +			if (_length == 0)
> +				return String.Empty;
> +
> +			if (null != _cached_str)
> +				return _cached_str;
> +
> +			// If we only have a half-full buffer we return a new string.
> +			if (_length < (_str.Length / 2)) 

Use a shift directly here, instead of the division, so we don't have to
bother optimizing it.

> +			{
> +				_cached_str = _str.Substring(0, _length);
> +				return _cached_str;
> +			}
> +
> +			_cached_str = _str;
> +			_str.InternalSetLength(_length);
> +
> +			return _str;
>  		}

Strings have the property that they are 0-terminated (so they can be
passed to P/Invoked methods without a copy). Maybe the best place to
ensure that is String.InternalSetLength(). The old impl got this for
free, since we always alloc strings with one char more than their
length, but in the stringbuilder there could be a non-zero char
left there from previous operations. 

Thanks. The patch can be committed (sorry it took a long time to review it).

lupus

-- 
-----------------------------------------------------------------
lupus at debian.org                                     debian/rules
lupus at ximian.com                             Monkeys do it better