No-op ConcurrentStack.PushRange(T[]) if empty (#62126)

* No-op ConcurrentStack.PushRange(T[]) if empty Resolves #62121. * Add more bounds checks Assert that ArgumentOutOfRangeException is thrown if specifying indexes for an empty array with PushRange(T[], int, int). * Review feedback Allow out-of-bounds index on empty array if count is zero. * Add extra test case Add a further test case for a non-empty array but with a count parameter value of zero.
dotnet · Nov 30, 2021 · 9857fe0 · 9857fe0
1 parent 8f87ac7
commit 9857fe0
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 2 deletions.
diff --git a/...raries/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentStack.cs b/...raries/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentStack.cs
@@ -349,7 +349,6 @@ public void PushRange(T[] items, int startIndex, int count)
             if (count == 0)
                 return;
 
-
             Node head, tail;
             head = tail = new Node(items[startIndex]);
             for (int i = startIndex + 1; i < startIndex + count; i++)
@@ -410,7 +409,7 @@ private static void ValidatePushPopRangeInput(T[] items, int startIndex, int cou
                 throw new ArgumentOutOfRangeException(nameof(count), SR.ConcurrentStack_PushPopRange_CountOutOfRange);
             }
             int length = items.Length;
-            if (startIndex >= length || startIndex < 0)
+            if (startIndex > length || startIndex < 0)
             {
                 throw new ArgumentOutOfRangeException(nameof(startIndex), SR.ConcurrentStack_PushPopRange_StartOutOfRange);
             }

diff --git a/src/libraries/System.Collections.Concurrent/tests/ConcurrentStackTests.cs b/src/libraries/System.Collections.Concurrent/tests/ConcurrentStackTests.cs
@@ -63,6 +63,37 @@ public void PushRange_NoItems_NothingAdded()
             Assert.True(s.IsEmpty);
         }
 
+        [Fact]
+        public void PushRange_NoItems_NothingAdded_EmptyArrayWithRangeSpecified()
+        {
+            var s = new ConcurrentStack<int>();
+            Assert.True(s.IsEmpty);
+
+            s.PushRange(new int[0], 0, 0);
+            Assert.True(s.IsEmpty);
+        }
+
+        [Fact]
+        public void PushRange_NoItems_NothingAdded_NonEmptyArrayWithZeroCountSpecified()
+        {
+            var s = new ConcurrentStack<int>();
+            Assert.True(s.IsEmpty);
+
+            int[] arr = new int[2];
+            s.PushRange(arr, arr.Length, 0);
+            Assert.True(s.IsEmpty);
+        }
+
+        [Fact]
+        public void PushRange_NoItems_NothingAdded_EmptyArrayNoRangeSpecified()
+        {
+            var s = new ConcurrentStack<int>();
+            Assert.True(s.IsEmpty);
+
+            s.PushRange(new int[0]);
+            Assert.True(s.IsEmpty);
+        }
+
         [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))]
         [InlineData(8, 10)]
         [InlineData(16, 100)]
@@ -134,6 +165,7 @@ public void PushRange_InvalidArguments_Throws()
             Assert.Throws<ArgumentOutOfRangeException>(() => stack.PushRange(new int[1], 0, -1));
             Assert.Throws<ArgumentOutOfRangeException>(() => stack.PushRange(new int[1], -1, 1));
             Assert.Throws<ArgumentOutOfRangeException>(() => stack.PushRange(new int[1], 2, 1));
+            AssertExtensions.Throws<ArgumentException>(null, () => stack.PushRange(new int[0], 0, 1));
             AssertExtensions.Throws<ArgumentException>(null, () => stack.PushRange(new int[1], 0, 10));
         }