PR feedback

csharp/src/Google.Protobuf.Test/ByteStringTest.cs

@@ -37,6 +37,10 @@
 using System.Collections.Generic;
 using System.Collections;
 using System.Linq;
+using System.Buffers;
+using System.Runtime.InteropServices;
+using System.Threading;
+using System.Runtime.CompilerServices;
 #if !NET35
 using System.Threading.Tasks;
 #endif
@@ -218,6 +222,25 @@ public void WriteToStream()
             CollectionAssert.AreEqual(data, ms.ToArray());
         }
 
+        [Test]
+        public void WriteToStream_Stackalloc()
+        {
+            byte[] data = Encoding.UTF8.GetBytes("Hello world");
+            Span<byte> s = stackalloc byte[data.Length];
+            data.CopyTo(s);
+
+            MemoryStream ms = new MemoryStream();
+
+            using (UnmanagedMemoryManager<byte> manager = new UnmanagedMemoryManager<byte>(s))
+            {
+                ByteString bs = ByteString.AttachBytes(manager.Memory);
+
+                bs.WriteTo(ms);
+            }
+
+            CollectionAssert.AreEqual(data, ms.ToArray());
+        }
+
         [Test]
         public void ToStringUtf8()
         {
@@ -232,6 +255,21 @@ public void ToStringWithExplicitEncoding()
             Assert.AreEqual("\u20ac", bs.ToString(Encoding.Unicode));
         }
 
+        [Test]
+        public void ToString_Stackalloc()
+        {
+            byte[] data = Encoding.UTF8.GetBytes("Hello world");
+            Span<byte> s = stackalloc byte[data.Length];
+            data.CopyTo(s);
+
+            using (UnmanagedMemoryManager<byte> manager = new UnmanagedMemoryManager<byte>(s))
+            {
+                ByteString bs = ByteString.AttachBytes(manager.Memory);
+
+                Assert.AreEqual("Hello world", bs.ToString(Encoding.UTF8));
+            }
+        }
+
         [Test]
         public void FromBase64_WithText()
         {
@@ -248,6 +286,29 @@ public void FromBase64_Empty()
             Assert.AreSame(ByteString.Empty, ByteString.FromBase64(""));
         }
 
+        [Test]
+        public void ToBase64_Array()
+        {
+            ByteString bs = ByteString.CopyFrom(Encoding.UTF8.GetBytes("Hello world"));
+
+            Assert.AreEqual("SGVsbG8gd29ybGQ=", bs.ToBase64());
+        }
+
+        [Test]
+        public void ToBase64_Stackalloc()
+        {
+            byte[] data = Encoding.UTF8.GetBytes("Hello world");
+            Span<byte> s = stackalloc byte[data.Length];
+            data.CopyTo(s);
+
+            using (UnmanagedMemoryManager<byte> manager = new UnmanagedMemoryManager<byte>(s))
+            {
+                ByteString bs = ByteString.AttachBytes(manager.Memory);
+
+                Assert.AreEqual("SGVsbG8gd29ybGQ=", bs.ToBase64());
+            }
+        }
+
         [Test]
         public void FromStream_Seekable()
         {
@@ -325,5 +386,38 @@ public void GetContentsAsReadOnlyMemory()
             var copied = byteString.Memory.ToArray();
             CollectionAssert.AreEqual(byteString, copied);
         }
+
+        // Create Memory<byte> from non-array source.
+        // Use by ByteString tests that have optimized path for array backed Memory<byte>.
+        private sealed unsafe class UnmanagedMemoryManager<T> : MemoryManager<T> where T : unmanaged
+        {
+            private readonly T* _pointer;
+            private readonly int _length;
+
+            public UnmanagedMemoryManager(Span<T> span)
+            {
+                fixed (T* ptr = &MemoryMarshal.GetReference(span))
+                {
+                    _pointer = ptr;
+                    _length = span.Length;
+                }
+            }
+
+            public override Span<T> GetSpan() => new Span<T>(_pointer, _length);
+
+            public override MemoryHandle Pin(int elementIndex = 0)
+            {
+                if (elementIndex < 0 || elementIndex >= _length)
+                {
+                    throw new ArgumentOutOfRangeException(nameof(elementIndex));
+                }
+
+                return new MemoryHandle(_pointer + elementIndex);
+            }
+
+            public override void Unpin() { }
+
+            protected override void Dispose(bool disposing) { }
+        }
     }
 }

csharp/src/Google.Protobuf.Test/Google.Protobuf.Test.csproj

@@ -6,6 +6,7 @@
     <SignAssembly>true</SignAssembly>
     <PublicSign Condition=" '$(OS)' != 'Windows_NT' ">true</PublicSign>
     <IsPackable>False</IsPackable>
+    <AllowUnsafeBlocks>True</AllowUnsafeBlocks>
   </PropertyGroup>
 
   <ItemGroup>

csharp/src/Google.Protobuf/ByteString.cs

@@ -65,6 +65,16 @@ internal static ByteString AttachBytes(ReadOnlyMemory<byte> bytes)
             return new ByteString(bytes);
         }
 
+        /// <summary>
+        /// Internal use only. Ensure that the provided memory is not mutated and belongs to this instance.
+        /// This method encapsulates converting array to memory. Reduces need for SecuritySafeCritical
+        /// in .NET Framework.
+        /// </summary>
+        internal static ByteString AttachBytes(byte[] bytes)
+        {
+            return AttachBytes(bytes.AsMemory());
+        }
+
         /// <summary>
         /// Constructs a new ByteString from the given memory. The memory is
         /// *not* copied, and must not be modified after this constructor is called.

csharp/src/Google.Protobuf/ByteStringAsync.cs

@@ -51,7 +51,7 @@ internal static async Task<ByteString> FromStreamAsyncCore(Stream stream, Cancel
             // We have to specify the buffer size here, as there's no overload accepting the cancellation token
             // alone. But it's documented to use 81920 by default if not specified.
             await stream.CopyToAsync(memoryStream, 81920, cancellationToken);
-#if NETSTANDARD1_1 || NETSTANDARD2_0
+#if NETSTANDARD1_1
             byte[] bytes = memoryStream.ToArray();
 #else
             // Avoid an extra copy if we can.

csharp/src/Google.Protobuf/UnsafeByteOperations.cs

@@ -58,7 +58,7 @@ namespace Google.Protobuf
     /// <description>serialization may succeed but the wrong bytes may be written out</description>
     /// </item>
     /// <item>
-    /// <description>messages are no longer threadsafe</description>
+    /// <description>objects that are normally immutable (such as ByteString) are no longer immutable</description>
     /// </item>
     /// <item>
     /// <description>hashCode may be incorrect</description>