Add object null checks in Memory<T> APIs to support default (#14816)

ahsonkhan · safern · commit 657fd9d915fe · 2018-01-16T13:09:57.000-08:00
* Add object null checks in Memory&lt;T&gt; APIs to support default

* Changing Empty property to return default value.

* Update use of object null checks

* Addressing PR feedback.

* Removing typeof char check.

* Fix typo

Signed-off-by: dotnet-bot-corefx-mirror &lt;dotnet-bot@microsoft.com&gt;
diff --git a/src/Common/src/CoreLib/System/Memory.cs b/src/Common/src/CoreLib/System/Memory.cs
@@ -20,7 +20,7 @@ public readonly struct Memory<T>
 
         // The highest order bit of _index is used to discern whether _object is an array/string or an owned memory
         // if (_index >> 31) == 1, object _object is an OwnedMemory<T>
-        // else, object _object is a T[] or a string.  It can only be a string if the Memory<T> was created by
+        // else, object _object is a T[] or a string. It can only be a string if the Memory<T> was created by
         // using unsafe / marshaling code to reinterpret a ReadOnlyMemory<char> wrapped around a string as
         // a Memory<T>.
         private readonly object _object;
@@ -122,7 +122,7 @@ public static implicit operator ReadOnlyMemory<T>(Memory<T> memory) =>
         /// <summary>
         /// Returns an empty <see cref="Memory{T}"/>
         /// </summary>
-        public static Memory<T> Empty { get; } = Array.Empty<T>();
+        public static Memory<T> Empty => default;
 
         /// <summary>
         /// The number of items in the memory.
@@ -187,15 +187,19 @@ public Span<T> Span
                 {
                     // This is dangerous, returning a writable span for a string that should be immutable.
                     // However, we need to handle the case where a ReadOnlyMemory<char> was created from a string
-                    // and then cast to a Memory<T>.  Such a cast can only be done with unsafe or marshaling code,
+                    // and then cast to a Memory<T>. Such a cast can only be done with unsafe or marshaling code,
                     // in which case that's the dangerous operation performed by the dev, and we're just following
                     // suit here to make it work as best as possible.
                     return new Span<T>(ref Unsafe.As<char, T>(ref s.GetRawStringData()), s.Length).Slice(_index, _length);
                 }
-                else
+                else if (_object != null)
                 {
                     return new Span<T>((T[])_object, _index, _length);
                 }
+                else
+                {
+                    return default;
+                }
             }
         }
 
@@ -224,7 +228,7 @@ public Span<T> Span
 
         public unsafe MemoryHandle Retain(bool pin = false)
         {
-            MemoryHandle memoryHandle;
+            MemoryHandle memoryHandle = default;
             if (pin)
             {
                 if (_index < 0)
@@ -243,9 +247,8 @@ public unsafe MemoryHandle Retain(bool pin = false)
                     void* pointer = Unsafe.Add<T>(Unsafe.AsPointer(ref s.GetRawStringData()), _index);
                     memoryHandle = new MemoryHandle(null, pointer, handle);
                 }
-                else
+                else if (_object is T[] array)
                 {
-                    var array = (T[])_object;
                     var handle = GCHandle.Alloc(array, GCHandleType.Pinned);
                     void* pointer = Unsafe.Add<T>(Unsafe.AsPointer(ref array.GetRawSzArrayData()), _index);
                     memoryHandle = new MemoryHandle(null, pointer, handle);
@@ -258,10 +261,6 @@ public unsafe MemoryHandle Retain(bool pin = false)
                     ((OwnedMemory<T>)_object).Retain();
                     memoryHandle = new MemoryHandle((OwnedMemory<T>)_object);
                 }
-                else
-                {
-                    memoryHandle = new MemoryHandle(null);
-                }
             }
             return memoryHandle;
         }
@@ -280,14 +279,10 @@ public bool TryGetArray(out ArraySegment<T> arraySegment)
                     return true;
                 }
             }
-            else
+            else if (_object is T[] arr)
             {
-                T[] arr = _object as T[];
-                if (typeof(T) != typeof(char) || arr != null)
-                {
-                    arraySegment = new ArraySegment<T>(arr, _index, _length);
-                    return true;
-                }
+                arraySegment = new ArraySegment<T>(arr, _index, _length);
+                return true;
             }
 
             arraySegment = default(ArraySegment<T>);
@@ -333,7 +328,7 @@ public bool Equals(Memory<T> other)
         [EditorBrowsable(EditorBrowsableState.Never)]
         public override int GetHashCode()
         {
-            return CombineHashCodes(_object.GetHashCode(), _index.GetHashCode(), _length.GetHashCode());
+            return _object != null ? CombineHashCodes(_object.GetHashCode(), _index.GetHashCode(), _length.GetHashCode()) : 0;
         }
 
         private static int CombineHashCodes(int left, int right)
diff --git a/src/Common/src/CoreLib/System/ReadOnlyMemory.cs b/src/Common/src/CoreLib/System/ReadOnlyMemory.cs
@@ -75,7 +75,7 @@ public ReadOnlyMemory(T[] array, int start, int length)
             _length = length;
         }
 
-        /// <summary>Creates a new memory over the existing object, start, and length.  No validation is performed.</summary>
+        /// <summary>Creates a new memory over the existing object, start, and length. No validation is performed.</summary>
         /// <param name="obj">The target object.</param>
         /// <param name="start">The index at which to begin the memory.</param>
         /// <param name="length">The number of items in the memory.</param>
@@ -104,7 +104,7 @@ internal ReadOnlyMemory(object obj, int start, int length)
         /// <summary>
         /// Returns an empty <see cref="ReadOnlyMemory{T}"/>
         /// </summary>
-        public static ReadOnlyMemory<T> Empty { get; } = Array.Empty<T>();
+        public static ReadOnlyMemory<T> Empty => default;
 
         /// <summary>
         /// The number of items in the memory.
@@ -169,10 +169,14 @@ public ReadOnlySpan<T> Span
                 {
                     return new ReadOnlySpan<T>(ref Unsafe.As<char, T>(ref s.GetRawStringData()), s.Length).Slice(_index, _length);
                 }
-                else
+                else if (_object != null)
                 {
                     return new ReadOnlySpan<T>((T[])_object, _index, _length);
                 }
+                else
+                {
+                    return default;
+                }
             }
         }
 
@@ -206,7 +210,7 @@ public ReadOnlySpan<T> Span
         /// </param>
         public unsafe MemoryHandle Retain(bool pin = false)
         {
-            MemoryHandle memoryHandle;
+            MemoryHandle memoryHandle = default;
             if (pin)
             {
                 if (_index < 0)
@@ -220,9 +224,8 @@ public unsafe MemoryHandle Retain(bool pin = false)
                     void* pointer = Unsafe.Add<T>(Unsafe.AsPointer(ref s.GetRawStringData()), _index);
                     memoryHandle = new MemoryHandle(null, pointer, handle);
                 }
-                else
+                else if (_object is T[] array)
                 {
-                    var array = (T[])_object;
                     var handle = GCHandle.Alloc(array, GCHandleType.Pinned);
                     void* pointer = Unsafe.Add<T>(Unsafe.AsPointer(ref array.GetRawSzArrayData()), _index);
                     memoryHandle = new MemoryHandle(null, pointer, handle);
@@ -235,10 +238,6 @@ public unsafe MemoryHandle Retain(bool pin = false)
                     ((OwnedMemory<T>)_object).Retain();
                     memoryHandle = new MemoryHandle((OwnedMemory<T>)_object);
                 }
-                else
-                {
-                    memoryHandle = new MemoryHandle(null);
-                }
             }
             return memoryHandle;
         }
@@ -258,14 +257,10 @@ public bool DangerousTryGetArray(out ArraySegment<T> arraySegment)
                     return true;
                 }
             }
-            else
+            else if (_object is T[] arr)
             {
-                T[] arr = _object as T[];
-                if (typeof(T) != typeof(char) || arr != null)
-                {
-                    arraySegment = new ArraySegment<T>(arr, _index, _length);
-                    return true;
-                }
+                arraySegment = new ArraySegment<T>(arr, _index, _length);
+                return true;
             }
 
             arraySegment = default;
@@ -313,7 +308,7 @@ public bool Equals(ReadOnlyMemory<T> other)
         [EditorBrowsable(EditorBrowsableState.Never)]
         public override int GetHashCode()
         {
-            return CombineHashCodes(_object.GetHashCode(), _index.GetHashCode(), _length.GetHashCode());
+            return _object != null ? CombineHashCodes(_object.GetHashCode(), _index.GetHashCode(), _length.GetHashCode()) : 0;
         }
         
         private static int CombineHashCodes(int left, int right)

Original file line number	Diff line number	Diff line change
`@@ -20,7 +20,7 @@ public readonly struct Memory<T>`
`20`	`20`
`21`	`21`	`// The highest order bit of _index is used to discern whether _object is an array/string or an owned memory`
`22`	`22`	`// if (_index >> 31) == 1, object _object is an OwnedMemory<T>`
`23`		`- // else, object _object is a T[] or a string. It can only be a string if the Memory<T> was created by`
	`23`	`+ // else, object _object is a T[] or a string. It can only be a string if the Memory<T> was created by`
`24`	`24`	`// using unsafe / marshaling code to reinterpret a ReadOnlyMemory<char> wrapped around a string as`
`25`	`25`	`// a Memory<T>.`
`26`	`26`	`private readonly object _object;`
`@@ -122,7 +122,7 @@ public static implicit operator ReadOnlyMemory<T>(Memory<T> memory) =>`
`122`	`122`	`/// <summary>`
`123`	`123`	`/// Returns an empty <see cref="Memory{T}"/>`
`124`	`124`	`/// </summary>`
`125`		`- public static Memory<T> Empty { get; } = Array.Empty<T>();`
	`125`	`+ public static Memory<T> Empty => default;`
`126`	`126`
`127`	`127`	`/// <summary>`
`128`	`128`	`/// The number of items in the memory.`
`@@ -187,15 +187,19 @@ public Span<T> Span`
`187`	`187`	`{`
`188`	`188`	`// This is dangerous, returning a writable span for a string that should be immutable.`
`189`	`189`	`// However, we need to handle the case where a ReadOnlyMemory<char> was created from a string`
`190`		`- // and then cast to a Memory<T>. Such a cast can only be done with unsafe or marshaling code,`
	`190`	`+ // and then cast to a Memory<T>. Such a cast can only be done with unsafe or marshaling code,`
`191`	`191`	`// in which case that's the dangerous operation performed by the dev, and we're just following`
`192`	`192`	`// suit here to make it work as best as possible.`
`193`	`193`	`return new Span<T>(ref Unsafe.As<char, T>(ref s.GetRawStringData()), s.Length).Slice(_index, _length);`
`194`	`194`	`}`
`195`		`- else`
	`195`	`+ else if (_object != null)`
`196`	`196`	`{`
`197`	`197`	`return new Span<T>((T[])_object, _index, _length);`
`198`	`198`	`}`
	`199`	`+ else`
	`200`	`+ {`
	`201`	`+ return default;`
	`202`	`+ }`
`199`	`203`	`}`
`200`	`204`	`}`
`201`	`205`
`@@ -224,7 +228,7 @@ public Span<T> Span`
`224`	`228`
`225`	`229`	`public unsafe MemoryHandle Retain(bool pin = false)`
`226`	`230`	`{`
`227`		`- MemoryHandle memoryHandle;`
	`231`	`+ MemoryHandle memoryHandle = default;`
`228`	`232`	`if (pin)`
`229`	`233`	`{`
`230`	`234`	`if (_index < 0)`
`@@ -243,9 +247,8 @@ public unsafe MemoryHandle Retain(bool pin = false)`
`243`	`247`	`void* pointer = Unsafe.Add<T>(Unsafe.AsPointer(ref s.GetRawStringData()), _index);`
`244`	`248`	`memoryHandle = new MemoryHandle(null, pointer, handle);`
`245`	`249`	`}`
`246`		`- else`
	`250`	`+ else if (_object is T[] array)`
`247`	`251`	`{`
`248`		`- var array = (T[])_object;`
`249`	`252`	`var handle = GCHandle.Alloc(array, GCHandleType.Pinned);`
`250`	`253`	`void* pointer = Unsafe.Add<T>(Unsafe.AsPointer(ref array.GetRawSzArrayData()), _index);`
`251`	`254`	`memoryHandle = new MemoryHandle(null, pointer, handle);`
`@@ -258,10 +261,6 @@ public unsafe MemoryHandle Retain(bool pin = false)`
`258`	`261`	`((OwnedMemory<T>)_object).Retain();`
`259`	`262`	`memoryHandle = new MemoryHandle((OwnedMemory<T>)_object);`
`260`	`263`	`}`
`261`		`- else`
`262`		`- {`
`263`		`- memoryHandle = new MemoryHandle(null);`
`264`		`- }`
`265`	`264`	`}`
`266`	`265`	`return memoryHandle;`
`267`	`266`	`}`
`@@ -280,14 +279,10 @@ public bool TryGetArray(out ArraySegment<T> arraySegment)`
`280`	`279`	`return true;`
`281`	`280`	`}`
`282`	`281`	`}`
`283`		`- else`
	`282`	`+ else if (_object is T[] arr)`
`284`	`283`	`{`
`285`		`- T[] arr = _object as T[];`
`286`		`- if (typeof(T) != typeof(char) \|\| arr != null)`
`287`		`- {`
`288`		`- arraySegment = new ArraySegment<T>(arr, _index, _length);`
`289`		`- return true;`
`290`		`- }`
	`284`	`+ arraySegment = new ArraySegment<T>(arr, _index, _length);`
	`285`	`+ return true;`
`291`	`286`	`}`
`292`	`287`
`293`	`288`	`arraySegment = default(ArraySegment<T>);`
`@@ -333,7 +328,7 @@ public bool Equals(Memory<T> other)`
`333`	`328`	`[EditorBrowsable(EditorBrowsableState.Never)]`
`334`	`329`	`public override int GetHashCode()`
`335`	`330`	`{`
`336`		`- return CombineHashCodes(_object.GetHashCode(), _index.GetHashCode(), _length.GetHashCode());`
	`331`	`+ return _object != null ? CombineHashCodes(_object.GetHashCode(), _index.GetHashCode(), _length.GetHashCode()) : 0;`
`337`	`332`	`}`
`338`	`333`
`339`	`334`	`private static int CombineHashCodes(int left, int right)`
Original file line number	Diff line number	Diff line change
`@@ -75,7 +75,7 @@ public ReadOnlyMemory(T[] array, int start, int length)`
`75`	`75`	`_length = length;`
`76`	`76`	`}`
`77`	`77`
`78`		`- /// <summary>Creates a new memory over the existing object, start, and length. No validation is performed.</summary>`
	`78`	`+ /// <summary>Creates a new memory over the existing object, start, and length. No validation is performed.</summary>`
`79`	`79`	`/// <param name="obj">The target object.</param>`
`80`	`80`	`/// <param name="start">The index at which to begin the memory.</param>`
`81`	`81`	`/// <param name="length">The number of items in the memory.</param>`
`@@ -104,7 +104,7 @@ internal ReadOnlyMemory(object obj, int start, int length)`
`104`	`104`	`/// <summary>`
`105`	`105`	`/// Returns an empty <see cref="ReadOnlyMemory{T}"/>`
`106`	`106`	`/// </summary>`
`107`		`- public static ReadOnlyMemory<T> Empty { get; } = Array.Empty<T>();`
	`107`	`+ public static ReadOnlyMemory<T> Empty => default;`
`108`	`108`
`109`	`109`	`/// <summary>`
`110`	`110`	`/// The number of items in the memory.`
`@@ -169,10 +169,14 @@ public ReadOnlySpan<T> Span`
`169`	`169`	`{`
`170`	`170`	`return new ReadOnlySpan<T>(ref Unsafe.As<char, T>(ref s.GetRawStringData()), s.Length).Slice(_index, _length);`
`171`	`171`	`}`
`172`		`- else`
	`172`	`+ else if (_object != null)`
`173`	`173`	`{`
`174`	`174`	`return new ReadOnlySpan<T>((T[])_object, _index, _length);`
`175`	`175`	`}`
	`176`	`+ else`
	`177`	`+ {`
	`178`	`+ return default;`
	`179`	`+ }`
`176`	`180`	`}`
`177`	`181`	`}`
`178`	`182`
`@@ -206,7 +210,7 @@ public ReadOnlySpan<T> Span`
`206`	`210`	`/// </param>`
`207`	`211`	`public unsafe MemoryHandle Retain(bool pin = false)`
`208`	`212`	`{`
`209`		`- MemoryHandle memoryHandle;`
	`213`	`+ MemoryHandle memoryHandle = default;`
`210`	`214`	`if (pin)`
`211`	`215`	`{`
`212`	`216`	`if (_index < 0)`
`@@ -220,9 +224,8 @@ public unsafe MemoryHandle Retain(bool pin = false)`
`220`	`224`	`void* pointer = Unsafe.Add<T>(Unsafe.AsPointer(ref s.GetRawStringData()), _index);`
`221`	`225`	`memoryHandle = new MemoryHandle(null, pointer, handle);`
`222`	`226`	`}`
`223`		`- else`
	`227`	`+ else if (_object is T[] array)`
`224`	`228`	`{`
`225`		`- var array = (T[])_object;`
`226`	`229`	`var handle = GCHandle.Alloc(array, GCHandleType.Pinned);`
`227`	`230`	`void* pointer = Unsafe.Add<T>(Unsafe.AsPointer(ref array.GetRawSzArrayData()), _index);`
`228`	`231`	`memoryHandle = new MemoryHandle(null, pointer, handle);`
`@@ -235,10 +238,6 @@ public unsafe MemoryHandle Retain(bool pin = false)`
`235`	`238`	`((OwnedMemory<T>)_object).Retain();`
`236`	`239`	`memoryHandle = new MemoryHandle((OwnedMemory<T>)_object);`
`237`	`240`	`}`
`238`		`- else`
`239`		`- {`
`240`		`- memoryHandle = new MemoryHandle(null);`
`241`		`- }`
`242`	`241`	`}`
`243`	`242`	`return memoryHandle;`
`244`	`243`	`}`
`@@ -258,14 +257,10 @@ public bool DangerousTryGetArray(out ArraySegment<T> arraySegment)`
`258`	`257`	`return true;`
`259`	`258`	`}`
`260`	`259`	`}`
`261`		`- else`
	`260`	`+ else if (_object is T[] arr)`
`262`	`261`	`{`
`263`		`- T[] arr = _object as T[];`
`264`		`- if (typeof(T) != typeof(char) \|\| arr != null)`
`265`		`- {`
`266`		`- arraySegment = new ArraySegment<T>(arr, _index, _length);`
`267`		`- return true;`
`268`		`- }`
	`262`	`+ arraySegment = new ArraySegment<T>(arr, _index, _length);`
	`263`	`+ return true;`
`269`	`264`	`}`
`270`	`265`
`271`	`266`	`arraySegment = default;`
`@@ -313,7 +308,7 @@ public bool Equals(ReadOnlyMemory<T> other)`
`313`	`308`	`[EditorBrowsable(EditorBrowsableState.Never)]`
`314`	`309`	`public override int GetHashCode()`
`315`	`310`	`{`
`316`		`- return CombineHashCodes(_object.GetHashCode(), _index.GetHashCode(), _length.GetHashCode());`
	`311`	`+ return _object != null ? CombineHashCodes(_object.GetHashCode(), _index.GetHashCode(), _length.GetHashCode()) : 0;`
`317`	`312`	`}`
`318`	`313`
`319`	`314`	`private static int CombineHashCodes(int left, int right)`