Add missing overrides to SyncTextWriter (#21956)
authorStephen Toub <stoub@microsoft.com>
Fri, 11 Jan 2019 20:04:11 +0000 (15:04 -0500)
committerJan Kotas <jkotas@microsoft.com>
Fri, 11 Jan 2019 20:04:11 +0000 (12:04 -0800)
Several recently added virtuals to TextWriter were missing overrides on SyncTextWriter.

We were missing overrides of the synchronous `Write(ReadOnlySpan<char>)` and `WriteLine(ReadOnlySpan<char>)` overloads.  The impact here is primarily performance, though it'll still be an observable change in corner-case scenarios: rather than using the base implementation which would copy the span to an array and then invoke the properly overridden array-based Write{Line} methods, it'll now just delegate to the wrapped span methods.  So it calls the correct overload now on the wrapped instance, and saves some cost in the process.

We were also missing overrides for the async `WriteLineAsync()`, `WriteAsync(ReadOnlyMemory<char>, CancellationToken)`, WriteLineAsync(ReadOnlyMemory<char>, CancellationToken)` overloads.  Fixing these doesn't change the overload that's invoked on the wrapped writer, but it does cause the calls to become synchronous rather than queue a task that in turn calls the synchronized overload.  Whether the new behavior is better is arguable, but it keeps it consistent with every other XxAsync method on SyncTextWriter, which in my opinion is more important... if we wanted to alter the behavior for all of the XxAsync methods, we should do it for all of them.

Also, in the StringBuilder-based overrides, we were ignoring cancellation.  And in the recently added DisposeAsync, we should follow-suit with the rest of the async APIs and make it synchronous.

src/System.Private.CoreLib/shared/System/IO/TextWriter.cs

index 030a015..baf53b4 100644 (file)
@@ -786,12 +786,6 @@ namespace System.IO
             }
 
             [MethodImpl(MethodImplOptions.Synchronized)]
-            public override ValueTask DisposeAsync()
-            {
-                return _out.DisposeAsync();
-            }
-
-            [MethodImpl(MethodImplOptions.Synchronized)]
             public override void Flush() => _out.Flush();
 
             [MethodImpl(MethodImplOptions.Synchronized)]
@@ -804,6 +798,9 @@ namespace System.IO
             public override void Write(char[] buffer, int index, int count) => _out.Write(buffer, index, count);
 
             [MethodImpl(MethodImplOptions.Synchronized)]
+            public override void Write(ReadOnlySpan<char> buffer) => _out.Write(buffer);
+
+            [MethodImpl(MethodImplOptions.Synchronized)]
             public override void Write(bool value) => _out.Write(value);
 
             [MethodImpl(MethodImplOptions.Synchronized)]
@@ -864,6 +861,9 @@ namespace System.IO
             public override void WriteLine(char[] buffer, int index, int count) => _out.WriteLine(buffer, index, count);
 
             [MethodImpl(MethodImplOptions.Synchronized)]
+            public override void WriteLine(ReadOnlySpan<char> buffer) => _out.WriteLine(buffer);
+
+            [MethodImpl(MethodImplOptions.Synchronized)]
             public override void WriteLine(bool value) => _out.WriteLine(value);
 
             [MethodImpl(MethodImplOptions.Synchronized)]
@@ -910,6 +910,13 @@ namespace System.IO
             //
 
             [MethodImpl(MethodImplOptions.Synchronized)]
+            public override ValueTask DisposeAsync()
+            {
+                Dispose();
+                return default;
+            }
+
+            [MethodImpl(MethodImplOptions.Synchronized)]
             public override Task WriteAsync(char value)
             {
                 Write(value);
@@ -926,6 +933,11 @@ namespace System.IO
             [MethodImpl(MethodImplOptions.Synchronized)]
             public override Task WriteAsync(StringBuilder value, CancellationToken cancellationToken = default)
             {
+                if (cancellationToken.IsCancellationRequested)
+                {
+                    return Task.FromCanceled(cancellationToken);
+                }
+
                 Write(value);
                 return Task.CompletedTask;
             }
@@ -938,6 +950,30 @@ namespace System.IO
             }
 
             [MethodImpl(MethodImplOptions.Synchronized)]
+            public override Task WriteAsync(ReadOnlyMemory<char> buffer, CancellationToken cancellationToken = default)
+            {
+                if (cancellationToken.IsCancellationRequested)
+                {
+                    return Task.FromCanceled(cancellationToken);
+                }
+
+                Write(buffer.Span);
+                return Task.CompletedTask;
+            }
+
+            [MethodImpl(MethodImplOptions.Synchronized)]
+            public override Task WriteLineAsync(ReadOnlyMemory<char> buffer, CancellationToken cancellationToken = default)
+            {
+                if (cancellationToken.IsCancellationRequested)
+                {
+                    return Task.FromCanceled(cancellationToken);
+                }
+
+                WriteLine(buffer.Span);
+                return Task.CompletedTask;
+            }
+
+            [MethodImpl(MethodImplOptions.Synchronized)]
             public override Task WriteLineAsync(char value)
             {
                 WriteLine(value);
@@ -945,6 +981,13 @@ namespace System.IO
             }
 
             [MethodImpl(MethodImplOptions.Synchronized)]
+            public override Task WriteLineAsync()
+            {
+                WriteLine();
+                return Task.CompletedTask;
+            }
+
+            [MethodImpl(MethodImplOptions.Synchronized)]
             public override Task WriteLineAsync(string value)
             {
                 WriteLine(value);
@@ -954,6 +997,11 @@ namespace System.IO
             [MethodImpl(MethodImplOptions.Synchronized)]
             public override Task WriteLineAsync(StringBuilder value, CancellationToken cancellationToken = default)
             {
+                if (cancellationToken.IsCancellationRequested)
+                {
+                    return Task.FromCanceled(cancellationToken);
+                }
+
                 WriteLine(value);
                 return Task.CompletedTask;
             }