Merge pull request #37 from martindevans/unsafelist_bounds_checks

Unsafe List Indexers Bounds Checks
genaray · Oct 25, 2023 · 503a195 · 503a195
2 parents f49de55 + cbbf865
commit 503a195
Show file tree

Hide file tree

Showing 3 changed files with 71 additions and 11 deletions.
diff --git a/Arch.LowLevel.Tests/UnsafeListTest.cs b/Arch.LowLevel.Tests/UnsafeListTest.cs
@@ -16,13 +16,63 @@ public class UnsafeListTest
     public void UnsafeListAdd()
     {
         using var list = new UnsafeList<int>(8);
+        That(list.IsReadOnly, Is.False);
         list.Add(1);
         list.Add(2);
         list.Add(3);
 
         That(list.Count, Is.EqualTo(3));
     }
 
+    /// <summary>
+    ///     Checks if <see cref="UnsafeList{T}"/> GetHashCode is different for different lists
+    /// </summary>
+    [Test]
+    public void UnsafeListGetHashCode()
+    {
+        using var list1 = new UnsafeList<int>(8);
+        using var list2 = new UnsafeList<int>(8);
+
+        That(list1.GetHashCode(), Is.Not.EqualTo(list2.GetHashCode()));
+    }
+
+    /// <summary>
+    ///     Checks if <see cref="UnsafeList{T}"/> can access items by index
+    /// </summary>
+    [Test]
+    public void UnsafeListRefIndex()
+    {
+        using var list = new UnsafeList<int>(8);
+        list.Add(7);
+
+        ref var item0 = ref list[0];
+        That(item0, Is.EqualTo(7));
+        item0 = 11;
+        That(list[0], Is.EqualTo(11));
+
+        Throws<IndexOutOfRangeException>(() => { var x = list[-1]; });
+        Throws<IndexOutOfRangeException>(() => { var x = list[2]; });
+    }
+
+    /// <summary>
+    ///     Checks if <see cref="UnsafeList{T}"/> can access items by index
+    /// </summary>
+    [Test]
+    public void UnsafeListIndex()
+    {
+        using var unsafelist = new UnsafeList<int>(8);
+        unsafelist.Add(7);
+
+        var list = (IList<int>)unsafelist;
+
+        That(list[0], Is.EqualTo(7));
+        list[0] = 11;
+        That(list[0], Is.EqualTo(11));
+
+        Throws<IndexOutOfRangeException>(() => { var x = list[-1]; });
+        Throws<IndexOutOfRangeException>(() => { var x = list[2]; });
+    }
+
     /// <summary>
     ///     Checks if <see cref="UnsafeList{T}"/> is capable of being copied to an array.
     /// </summary>

diff --git a/Arch.LowLevel/UnsafeList.cs b/Arch.LowLevel/UnsafeList.cs
@@ -1,9 +1,6 @@
 using System.Collections;
 using System.Diagnostics;
-using System.Diagnostics.Contracts;
-using System.Drawing;
 using System.Runtime.CompilerServices;
-using System.Runtime.InteropServices;
 using System.Text;
 
 namespace Arch.LowLevel;
@@ -88,8 +85,8 @@ public void Add(T item)
         {
             EnsureCapacity(Capacity * 2);
         }
-        
-        this[Count] = item;
+
+        _array[Count] = item;
         Count++;
     }
 
@@ -147,7 +144,7 @@ public void RemoveAt(int index)
             //Buffer.MemoryCopy(_array+(index+1), _array+index,Count-index,Count-index);
             UnsafeArray.Copy(ref _array, index + 1, ref _array, index, Count - index);
         }
-        this[Count] = default;
+        _array[Count] = default;
     }
 
     /// <summary>
@@ -269,10 +266,10 @@ public void TrimExcess()
     T IList<T>.this[int index]
     {
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        get => _array[index];
+        get => _array[CheckIndex(index)];
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        set => _array[index] = value;
+        set => _array[CheckIndex(index)] = value;
     }
 
     /// <summary>
@@ -282,7 +279,20 @@ T IList<T>.this[int index]
     public ref T this[int i]
     {
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        get => ref _array[i];
+        get => ref _array[CheckIndex(i)];
+    }
+
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
+    private readonly int CheckIndex(int index)
+    {
+#if DEBUG
+        if (index < 0)
+            throw new IndexOutOfRangeException("Index cannot be less than zero");
+        if (index >= Count)
+            throw new IndexOutOfRangeException("Index cannot be greater than or equal to the count");
+#endif
+
+        return index;
     }
 
     /// <summary>

diff --git a/Arch.LowLevel/UnsafeStack.cs b/Arch.LowLevel/UnsafeStack.cs
@@ -67,7 +67,7 @@ public int Capacity
     }
 
     /// <summary>
-    ///     If this stack is full.
+    ///     If this stack is empty.
     /// </summary>
     public bool IsEmpty
     {
@@ -76,7 +76,7 @@ public bool IsEmpty
     }
 
     /// <summary>
-    ///     If this stack is empty. 
+    ///     If this stack is full. 
     /// </summary>
     public bool IsFull
     {