Refactor ValueStringBuilder

src/libraries/Common/src/System/Text/ValueStringBuilder.cs

@@ -54,27 +54,23 @@ public void EnsureCapacity(int capacity)
         }
 
         /// <summary>
-        /// Get a pinnable reference to the builder.
-        /// Does not ensure there is a null char after <see cref="Length"/>
-        /// This overload is pattern matched in the C# 7.3+ compiler so you can omit
-        /// the explicit method call, and write eg "fixed (char* c = builder)"
+        /// Ensures that the builder is terminated with a NUL character.
         /// </summary>
-        public ref char GetPinnableReference()
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        public void NullTerminate()
         {
-            return ref MemoryMarshal.GetReference(_chars);
+            EnsureCapacity(_pos + 1);
+            _chars[_pos] = '\0';
         }
 
         /// <summary>
         /// Get a pinnable reference to the builder.
+        /// Does not ensure there is a null char after <see cref="Length"/>
+        /// This overload is pattern matched in the C# 7.3+ compiler so you can omit
+        /// the explicit method call, and write eg "fixed (char* c = builder)"
         /// </summary>
-        /// <param name="terminate">Ensures that the builder has a null char after <see cref="Length"/></param>
-        public ref char GetPinnableReference(bool terminate)
+        public ref char GetPinnableReference()
         {
-            if (terminate)
-            {
-                EnsureCapacity(Length + 1);
-                _chars[Length] = '\0';
-            }
             return ref MemoryMarshal.GetReference(_chars);
         }
 
@@ -97,40 +93,10 @@ public override string ToString()
         /// <summary>Returns the underlying storage of the builder.</summary>
         public Span<char> RawChars => _chars;
 
-        /// <summary>
-        /// Returns a span around the contents of the builder.
-        /// </summary>
-        /// <param name="terminate">Ensures that the builder has a null char after <see cref="Length"/></param>
-        public ReadOnlySpan<char> AsSpan(bool terminate)
-        {
-            if (terminate)
-            {
-                EnsureCapacity(Length + 1);
-                _chars[Length] = '\0';
-            }
-            return _chars.Slice(0, _pos);
-        }
-
         public ReadOnlySpan<char> AsSpan() => _chars.Slice(0, _pos);
         public ReadOnlySpan<char> AsSpan(int start) => _chars.Slice(start, _pos - start);
         public ReadOnlySpan<char> AsSpan(int start, int length) => _chars.Slice(start, length);
 
-        public bool TryCopyTo(Span<char> destination, out int charsWritten)
-        {
-            if (_chars.Slice(0, _pos).TryCopyTo(destination))
-            {
-                charsWritten = _pos;
-                Dispose();
-                return true;
-            }
-            else
-            {
-                charsWritten = 0;
-                Dispose();
-                return false;
-            }
-        }
-
         public void Insert(int index, char value, int count)
         {
             if (_pos > _chars.Length - count)

src/libraries/Common/tests/Common.Tests.csproj

@@ -159,12 +159,7 @@
     <Compile Include="$(CommonPath)Interop\OSX\Interop.Libraries.cs"
              Link="Common\Interop\OSX\Interop.Libraries.cs" />
   </ItemGroup>
-  <ItemGroup>
-    <Folder Include="CommonTest\System\" />
-    <Folder Include="Common\System\Security\Cryptography\" />
-    <Folder Include="System\Net\Sockets\" />
-    <Folder Include="System\Net\VirtualNetwork\" />
-  </ItemGroup>
+
   <ItemGroup>
     <PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="$(MicrosoftCodeAnalysisVersion_LatestVS)" PrivateAssets="all" />
   </ItemGroup>

src/libraries/Common/tests/Tests/System/Text/ValueStringBuilderTests.cs

@@ -178,46 +178,6 @@ public void ToString_ClearsBuilder_ThenReusable()
 
             Assert.Equal(0, vsb.Length);
             Assert.Equal(string.Empty, vsb.ToString());
-            Assert.True(vsb.TryCopyTo(Span<char>.Empty, out _));
-
-            const string Text2 = "another test";
-            vsb.Append(Text2);
-            Assert.Equal(Text2.Length, vsb.Length);
-            Assert.Equal(Text2, vsb.ToString());
-        }
-
-        [Fact]
-        public void TryCopyTo_FailsWhenDestinationIsTooSmall_SucceedsWhenItsLargeEnough()
-        {
-            var vsb = new ValueStringBuilder();
-
-            const string Text = "expected text";
-            vsb.Append(Text);
-            Assert.Equal(Text.Length, vsb.Length);
-
-            Span<char> dst = new char[Text.Length - 1];
-            Assert.False(vsb.TryCopyTo(dst, out int charsWritten));
-            Assert.Equal(0, charsWritten);
-            Assert.Equal(0, vsb.Length);
-        }
-
-        [Fact]
-        public void TryCopyTo_ClearsBuilder_ThenReusable()
-        {
-            const string Text1 = "test";
-            var vsb = new ValueStringBuilder();
-
-            vsb.Append(Text1);
-            Assert.Equal(Text1.Length, vsb.Length);
-
-            Span<char> dst = new char[Text1.Length];
-            Assert.True(vsb.TryCopyTo(dst, out int charsWritten));
-            Assert.Equal(Text1.Length, charsWritten);
-            Assert.Equal(Text1, new string(dst));
-
-            Assert.Equal(0, vsb.Length);
-            Assert.Equal(string.Empty, vsb.ToString());
-            Assert.True(vsb.TryCopyTo(Span<char>.Empty, out _));
 
             const string Text2 = "another test";
             vsb.Append(Text2);
@@ -238,7 +198,6 @@ public void Dispose_ClearsBuilder_ThenReusable()
 
             Assert.Equal(0, vsb.Length);
             Assert.Equal(string.Empty, vsb.ToString());
-            Assert.True(vsb.TryCopyTo(Span<char>.Empty, out _));
 
             const string Text2 = "another test";
             vsb.Append(Text2);

src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Windows.cs

@@ -545,9 +545,10 @@ private unsafe bool StartWithCreateProcess(ProcessStartInfo startInfo)
                             logonFlags = Interop.Advapi32.LogonFlags.LOGON_NETCREDENTIALS_ONLY;
                         }
 
+                        commandLine.NullTerminate();
                         fixed (char* passwordInClearTextPtr = startInfo.PasswordInClearText ?? string.Empty)
                         fixed (char* environmentBlockPtr = environmentBlock)
-                        fixed (char* commandLinePtr = &commandLine.GetPinnableReference(terminate: true))
+                        fixed (char* commandLinePtr = &commandLine.GetPinnableReference())
                         {
                             IntPtr passwordPtr = (startInfo.Password != null) ?
                                 Marshal.SecureStringToGlobalAllocUnicode(startInfo.Password) : IntPtr.Zero;
@@ -579,8 +580,9 @@ ref processInfo         // pointer to PROCESS_INFORMATION
                     }
                     else
                     {
+                        commandLine.NullTerminate();
                         fixed (char* environmentBlockPtr = environmentBlock)
-                        fixed (char* commandLinePtr = &commandLine.GetPinnableReference(terminate: true))
+                        fixed (char* commandLinePtr = &commandLine.GetPinnableReference())
                         {
                             retVal = Interop.Kernel32.CreateProcess(
                                 null,                // we don't need this since all the info is in commandLine

src/libraries/System.Private.CoreLib/src/System/IO/PathHelper.Windows.cs

@@ -51,7 +51,8 @@ internal static string Normalize(ref ValueStringBuilder path)
             var builder = new ValueStringBuilder(stackalloc char[PathInternal.MaxShortPath]);
 
             // Get the full path
-            GetFullPathName(path.AsSpan(terminate: true), ref builder);
+            path.NullTerminate();
+            GetFullPathName(path.AsSpan(), ref builder);
 
             string result = builder.AsSpan().IndexOf('~') >= 0
                 ? TryExpandShortFileName(ref builder, originalPath: null)
@@ -176,8 +177,9 @@ internal static string TryExpandShortFileName(ref ValueStringBuilder outputBuild
 
             while (!success)
             {
+                inputBuilder.NullTerminate();
                 uint result = Interop.Kernel32.GetLongPathNameW(
-                    ref inputBuilder.GetPinnableReference(terminate: true), ref outputBuilder.GetPinnableReference(), (uint)outputBuilder.Capacity);
+                    ref inputBuilder.GetPinnableReference(), ref outputBuilder.GetPinnableReference(), (uint)outputBuilder.Capacity);
 
                 // Replace any temporary null we added
                 if (inputBuilder[foundIndex] == '\0') inputBuilder[foundIndex] = '\\';

src/libraries/System.Runtime.Numerics/src/System/Number.BigInteger.cs

@@ -620,7 +620,8 @@ static uint MultiplyAdd(Span<uint> bits, uint multiplier, uint addValue)
 
             if (targetSpan)
             {
-                spanSuccess = sb.TryCopyTo(destination, out charsWritten);
+                charsWritten = (spanSuccess = sb.AsSpan().TryCopyTo(destination)) ? sb.Length : 0;
+                sb.Dispose();
                 return null;
             }
             else