Add String.Join overloads accepting a char separator (dotnet/coreclr#7942)
authorJames Ko <jamesqko@gmail.com>
Wed, 23 Nov 2016 23:25:28 +0000 (18:25 -0500)
committerAlex Ghiondea <AlexGhiondea@users.noreply.github.com>
Wed, 23 Nov 2016 23:25:28 +0000 (15:25 -0800)
* Add String.Join overloads that accept char separators

Commit migrated from https://github.com/dotnet/coreclr/commit/3f59c073ccc8d27c80e8ae1509685a078e9f1415

src/coreclr/src/mscorlib/model.xml
src/coreclr/src/mscorlib/mscorlib.shared.sources.props
src/coreclr/src/mscorlib/src/System/String.Manipulation.cs
src/coreclr/src/mscorlib/src/System/UnSafeCharBuffer.cs [deleted file]

index 5be0c7c..71c8cab 100644 (file)
       <Member Name="IsNormalized(System.Text.NormalizationForm)" />
       <Member Name="IsNullOrEmpty(System.String)" />
       <Member Name="IsNullOrWhiteSpace(System.String)" />
+      <Member Name="Join(System.Char,System.Object[])" />
+      <Member Name="Join&lt;T&gt;(System.Char,System.Collections.Generic.IEnumerable&lt;T&gt;)" />
+      <Member Name="Join(System.Char,System.String[])" />
+      <Member Name="Join(System.Char,System.String[],System.Int32,System.Int32)" />
       <Member Name="Join(System.String,System.Object[])" />
       <Member Name="Join&lt;T&gt;(System.String,System.Collections.Generic.IEnumerable&lt;T&gt;)" />
       <Member Name="Join(System.String,System.Collections.Generic.IEnumerable&lt;System.String&gt;)" />
index b4c3440..015cc46 100644 (file)
     <SystemSources Include="$(BclSourcesRoot)\System\UnitySerializationHolder.cs" />
     <SystemSources Include="$(BclSourcesRoot)\System\UnhandledExceptionEventArgs.cs" />
     <SystemSources Include="$(BclSourcesRoot)\System\UnhandledExceptionEventHandler.cs" />
-    <SystemSources Include="$(BclSourcesRoot)\System\UnSafeCharBuffer.cs" />
     <SystemSources Include="$(BclSourcesRoot)\System\ValueType.cs" />
     <SystemSources Include="$(BclSourcesRoot)\System\Version.cs" />
     <SystemSources Include="$(BclSourcesRoot)\System\Void.cs" />
index 0b3f533..c5af655 100644 (file)
@@ -541,37 +541,144 @@ namespace System
             }
             return result;
         }
+
+        public static string Join(char separator, params string[] value)
+        {
+            if (value == null)
+            {
+                throw new ArgumentNullException(nameof(value));
+            }
+
+            return Join(separator, value, 0, value.Length);
+        }
+
+        public unsafe static string Join(char separator, params object[] values)
+        {
+            // Defer argument validation to the internal function
+            return JoinCore(&separator, 1, values);
+        }
+
+        public unsafe static string Join<T>(char separator, IEnumerable<T> values)
+        {
+            // Defer argument validation to the internal function
+            return JoinCore(&separator, 1, values);
+        }
+
+        public unsafe static string Join(char separator, string[] value, int startIndex, int count)
+        {
+            // Defer argument validation to the internal function
+            return JoinCore(&separator, 1, value, startIndex, count);
+        }
     
         // Joins an array of strings together as one string with a separator between each original string.
         //
-        public static String Join(String separator, params String[] value) {
-            if (value==null)
+        public static string Join(string separator, params string[] value)
+        {
+            if (value == null)
+            {
                 throw new ArgumentNullException(nameof(value));
-            Contract.EndContractBlock();
+            }
             return Join(separator, value, 0, value.Length);
         }
 
         [ComVisible(false)]
-        public static string Join(string separator, params object[] values)
+        public unsafe static string Join(string separator, params object[] values)
+        {
+            separator = separator ?? string.Empty;
+            fixed (char* pSeparator = &separator.m_firstChar)
+            {
+                // Defer argument validation to the internal function
+                return JoinCore(pSeparator, separator.Length, values);
+            }
+        }
+
+        [ComVisible(false)]
+        public unsafe static string Join<T>(string separator, IEnumerable<T> values)
+        {
+            separator = separator ?? string.Empty;
+            fixed (char* pSeparator = &separator.m_firstChar)
+            {
+                // Defer argument validation to the internal function
+                return JoinCore(pSeparator, separator.Length, values);
+            }
+        }
+
+        [ComVisible(false)]
+        public static string Join(string separator, IEnumerable<string> values)
         {
             if (values == null)
+            {
                 throw new ArgumentNullException(nameof(values));
-            Contract.EndContractBlock();
+            }
+
+            using (IEnumerator<string> en = values.GetEnumerator())
+            {
+                if (!en.MoveNext())
+                {
+                    return string.Empty;
+                }
+
+                string firstValue = en.Current;
+
+                if (!en.MoveNext())
+                {
+                    // Only one value available
+                    return firstValue ?? string.Empty;
+                }
+
+                // Null separator and values are handled by the StringBuilder
+                StringBuilder result = StringBuilderCache.Acquire();
+                result.Append(firstValue);
+
+                do
+                {
+                    result.Append(separator);
+                    result.Append(en.Current);
+                }
+                while (en.MoveNext());
+
+                return StringBuilderCache.GetStringAndRelease(result);
+            }
+        }
+
+        // Joins an array of strings together as one string with a separator between each original string.
+        //
+        [System.Security.SecuritySafeCritical]  // auto-generated
+        public unsafe static string Join(string separator, string[] value, int startIndex, int count)
+        {
+            separator = separator ?? string.Empty;
+            fixed (char* pSeparator = &separator.m_firstChar)
+            {
+                // Defer argument validation to the internal function
+                return JoinCore(pSeparator, separator.Length, value, startIndex, count);
+            }
+        }
+
+        private unsafe static string JoinCore(char* separator, int separatorLength, object[] values)
+        {
+            if (values == null)
+            {
+                throw new ArgumentNullException(nameof(values));
+            }
 
             if (values.Length == 0)
+            {
                 return string.Empty;
+            }
 
             string firstString = values[0]?.ToString();
 
             if (values.Length == 1)
+            {
                 return firstString ?? string.Empty;
+            }
 
             StringBuilder result = StringBuilderCache.Acquire();
             result.Append(firstString);
 
             for (int i = 1; i < values.Length; i++)
             {
-                result.Append(separator);
+                result.Append(separator, separatorLength);
                 object value = values[i];
                 if (value != null)
                 {
@@ -582,18 +689,19 @@ namespace System
             return StringBuilderCache.GetStringAndRelease(result);
         }
 
-        [ComVisible(false)]
-        public static String Join<T>(String separator, IEnumerable<T> values)
+        private unsafe static string JoinCore<T>(char* separator, int separatorLength, IEnumerable<T> values)
         {
             if (values == null)
+            {
                 throw new ArgumentNullException(nameof(values));
-            Contract.Ensures(Contract.Result<String>() != null);
-            Contract.EndContractBlock();
+            }
 
             using (IEnumerator<T> en = values.GetEnumerator())
             {
                 if (!en.MoveNext())
+                {
                     return string.Empty;
+                }
                 
                 // We called MoveNext once, so this will be the first item
                 T currentValue = en.Current;
@@ -614,14 +722,14 @@ namespace System
                 }
 
                 StringBuilder result = StringBuilderCache.Acquire();
-                
+
                 result.Append(firstString);
 
                 do
                 {
                     currentValue = en.Current;
 
-                    result.Append(separator);
+                    result.Append(separator, separatorLength);
                     if (currentValue != null)
                     {
                         result.Append(currentValue.ToString());
@@ -633,107 +741,111 @@ namespace System
             }
         }
 
-        [ComVisible(false)]
-        public static String Join(String separator, IEnumerable<String> values) {
-            if (values == null)
-                throw new ArgumentNullException(nameof(values));
-            Contract.Ensures(Contract.Result<String>() != null);
-            Contract.EndContractBlock();
-
-            using(IEnumerator<String> en = values.GetEnumerator()) {
-                if (!en.MoveNext())
-                    return String.Empty;
-
-                String firstValue = en.Current;
-
-                if (!en.MoveNext()) {
-                    // Only one value available
-                    return firstValue ?? String.Empty;
-                }
-
-                // Null separator and values are handled by the StringBuilder
-                StringBuilder result = StringBuilderCache.Acquire();
-                result.Append(firstValue);
-
-                do {
-                    result.Append(separator);
-                    result.Append(en.Current);
-                } while (en.MoveNext());
-                return StringBuilderCache.GetStringAndRelease(result);
-            }
-        }
+        private unsafe static string JoinCore(char* separator, int separatorLength, string[] value, int startIndex, int count)
+        {
+            // If the separator is null, it is converted to an empty string before entering this function.
+            // Even for empty strings, fixed should never return null (it should return a pointer to a null char).
+            Contract.Assert(separator != null);
+            Contract.Assert(separatorLength >= 0);
 
-        // Joins an array of strings together as one string with a separator between each original string.
-        //
-        [System.Security.SecuritySafeCritical]  // auto-generated
-        public unsafe static String Join(String separator, String[] value, int startIndex, int count) {
-            //Range check the array
             if (value == null)
+            {
                 throw new ArgumentNullException(nameof(value));
-
+            }
             if (startIndex < 0)
+            {
                 throw new ArgumentOutOfRangeException(nameof(startIndex), Environment.GetResourceString("ArgumentOutOfRange_StartIndex"));
+            }
             if (count < 0)
+            {
                 throw new ArgumentOutOfRangeException(nameof(count), Environment.GetResourceString("ArgumentOutOfRange_NegativeCount"));
-
+            }
             if (startIndex > value.Length - count)
+            {
                 throw new ArgumentOutOfRangeException(nameof(startIndex), Environment.GetResourceString("ArgumentOutOfRange_IndexCountBuffer"));
-            Contract.EndContractBlock();
-
-            //Treat null as empty string.
-            if (separator == null) {
-                separator = String.Empty;
             }
-
-            //If count is 0, that skews a whole bunch of the calculations below, so just special case that.
-            if (count == 0) {
-                return String.Empty;
+            
+            if (count <= 1)
+            {
+                return count == 0 ?
+                    string.Empty :
+                    value[startIndex] ?? string.Empty;
             }
 
-            if (count == 1) {
-                return value[startIndex] ?? String.Empty;
+            long totalSeparatorsLength = (long)(count - 1) * separatorLength;
+            if (totalSeparatorsLength > int.MaxValue)
+            {
+                throw new OutOfMemoryException();
             }
+            int totalLength = (int)totalSeparatorsLength;
 
-            int jointLength = 0;
-            //Figure out the total length of the strings in value
-            int endIndex = startIndex + count - 1;
-            for (int stringToJoinIndex = startIndex; stringToJoinIndex <= endIndex; stringToJoinIndex++) {
-                string currentValue = value[stringToJoinIndex];
-
-                if (currentValue != null) {
-                    jointLength += currentValue.Length;
+            // Calculate the length of the resultant string so we know how much space to allocate.
+            for (int i = startIndex, end = startIndex + count; i < end; i++)
+            {
+                string currentValue = value[i];
+                if (currentValue != null)
+                {
+                    totalLength += currentValue.Length;
+                    if (totalLength < 0) // Check for overflow
+                    {
+                        throw new OutOfMemoryException();
+                    }
                 }
             }
-            
-            //Add enough room for the separator.
-            jointLength += (count - 1) * separator.Length;
 
-            // Note that we may not catch all overflows with this check (since we could have wrapped around the 4gb range any number of times
-            // and landed back in the positive range.) The input array might be modifed from other threads, 
-            // so we have to do an overflow check before each append below anyway. Those overflows will get caught down there.
-            if ((jointLength < 0) || ((jointLength + 1) < 0) ) {
-                throw new OutOfMemoryException();
-            }
+            // Copy each of the strings into the resultant buffer, interleaving with the separator.
+            string result = FastAllocateString(totalLength);
+            int copiedLength = 0;
 
-            //If this is an empty string, just return.
-            if (jointLength == 0) {
-                return String.Empty;
-            }
+            for (int i = startIndex, end = startIndex + count; i < end; i++)
+            {
+                // It's possible that another thread may have mutated the input array
+                // such that our second read of an index will not be the same string
+                // we got during the first read.
 
-            string jointString = FastAllocateString( jointLength );
-            fixed (char * pointerToJointString = &jointString.m_firstChar) {
-                UnSafeCharBuffer charBuffer = new UnSafeCharBuffer( pointerToJointString, jointLength);                
-                
-                // Append the first string first and then append each following string prefixed by the separator.
-                charBuffer.AppendString( value[startIndex] );
-                for (int stringToJoinIndex = startIndex + 1; stringToJoinIndex <= endIndex; stringToJoinIndex++) {
-                    charBuffer.AppendString( separator );
-                    charBuffer.AppendString( value[stringToJoinIndex] );
+                // We range check again to avoid buffer overflows if this happens.
+
+                string currentValue = value[i];
+                if (currentValue != null)
+                {
+                    int valueLen = currentValue.Length;
+                    if (valueLen > totalLength - copiedLength)
+                    {
+                        copiedLength = -1;
+                        break;
+                    }
+
+                    // Fill in the value.
+                    FillStringChecked(result, copiedLength, currentValue);
+                    copiedLength += valueLen;
+                }
+                    
+                if (i < end - 1)
+                {
+                    // Fill in the separator.
+                    fixed (char* pResult = &result.m_firstChar)
+                    {
+                        // If we are called from the char-based overload, we will not
+                        // want to call MemoryCopy each time we fill in the separator. So
+                        // specialize for 1-length separators.
+                        if (separatorLength == 1)
+                        {
+                            pResult[copiedLength] = *separator;
+                        }
+                        else
+                        {
+                            wstrcpy(pResult + copiedLength, separator, separatorLength);
+                        }
+                    }
+                    copiedLength += separatorLength;
                 }
-                Contract.Assert(*(pointerToJointString + charBuffer.Length) == '\0', "String must be null-terminated!");
             }
 
-            return jointString;
+            // If we copied exactly the right amount, return the new string.  Otherwise,
+            // something changed concurrently to mutate the input array: fall back to
+            // doing the concatenation again, but this time with a defensive copy. This
+            // fall back should be extremely rare.
+            return copiedLength == totalLength ? result : Concat((string[])value.Clone());
         }
         
         //
diff --git a/src/coreclr/src/mscorlib/src/System/UnSafeCharBuffer.cs b/src/coreclr/src/mscorlib/src/System/UnSafeCharBuffer.cs
deleted file mode 100644 (file)
index 78059b6..0000000
+++ /dev/null
@@ -1,57 +0,0 @@
-// Licensed to the .NET Foundation under one or more agreements.
-// The .NET Foundation licenses this file to you under the MIT license.
-// See the LICENSE file in the project root for more information.
-
-/*============================================================
-**
-**
-** Purpose: A class to detect incorrect usage of UnSafeBuffer
-**
-** 
-===========================================================*/
-
-namespace System {
-    using System.Security;
-    using System.Diagnostics;
-    using System.Diagnostics.Contracts;
-    
-    unsafe internal struct UnSafeCharBuffer{
-        [SecurityCritical]
-        char * m_buffer;
-        int m_totalSize;
-        int m_length;    
-    
-        [System.Security.SecurityCritical]  // auto-generated
-        public UnSafeCharBuffer( char *buffer,  int bufferSize) {
-            Contract.Assert( buffer != null, "buffer pointer can't be null."  );
-            Contract.Assert( bufferSize >= 0, "buffer size can't be negative."  );        
-            m_buffer = buffer;
-            m_totalSize = bufferSize;    
-            m_length = 0;
-        }
-    
-        [System.Security.SecuritySafeCritical]  // auto-generated
-        public void AppendString(string stringToAppend) {
-            if( String.IsNullOrEmpty( stringToAppend ) ) {
-                return;
-            }
-            
-            if ( (m_totalSize - m_length) < stringToAppend.Length ) {
-                throw new IndexOutOfRangeException();
-            }
-            
-            fixed( char* pointerToString = stringToAppend ) {        
-                Buffer.Memcpy( (byte*) (m_buffer + m_length), (byte *) pointerToString, stringToAppend.Length * sizeof(char));
-            }    
-            
-            m_length += stringToAppend.Length;
-            Contract.Assert(m_length <= m_totalSize, "Buffer has been overflowed!");
-        }
-                
-        public int Length {
-            get {
-                return m_length;
-            } 
-        }   
-    }    
-}