Array: fix some bugs in #fill. Fixes #4539

crystal-lang · Jun 10, 2017 · 4a9b867 · 4a9b867
1 parent b4e209b
commit 4a9b867
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 7 deletions.
diff --git a/spec/std/array_spec.cr b/spec/std/array_spec.cr
@@ -561,12 +561,24 @@ describe "Array" do
       a.fill('x', -2).should eq(expected)
     end
 
+    it "raises when given big negative number (#4539)" do
+      expect_raises(IndexError) do
+        ['a', 'b', 'c'].fill('x', -4)
+      end
+    end
+
     it "replaces only values between negative index and size" do
       a = ['a', 'b', 'c']
       expected = ['a', 'b', 'x']
       a.fill('x', -1, 1).should eq(expected)
     end
 
+    it "raises when given big negative number in from/count (#4539)" do
+      expect_raises(IndexError) do
+        ['a', 'b', 'c'].fill('x', -4, 1)
+      end
+    end
+
     it "replaces only values in range" do
       a = ['a', 'b', 'c']
       expected = ['x', 'x', 'c']

diff --git a/src/array.cr b/src/array.cr
@@ -706,14 +706,16 @@ class Array(T)
   #
   # Negative values of *from* count from the end of the array.
   #
+  # Raises `IndexError` if *from* is outside the array range.
+  #
   # ```
   # a = [1, 2, 3, 4]
   # a.fill(2) { |i| i * i } # => [1, 2, 4, 9]
   # ```
   def fill(from : Int)
     from += size if from < 0
 
-    raise IndexError.new if from >= size
+    raise IndexError.new unless 0 <= from < size
 
     from.upto(size - 1) { |i| @buffer[i] = yield i }
 
@@ -725,21 +727,22 @@ class Array(T)
   #
   # Negative values of *from* count from the end of the array.
   #
+  # Raises `IndexError` if *from* is outside the array range.
+  #
+  # Has no effect if *count* is zero or negative.
+  #
   # ```
   # a = [1, 2, 3, 4, 5, 6]
   # a.fill(2, 2) { |i| i * i } # => [1, 2, 4, 9, 5, 6]
   # ```
   def fill(from : Int, count : Int)
-    return self if count < 0
+    return self if count <= 0
 
     from += size if from < 0
-    count += size if count < 0
-
-    raise IndexError.new if from >= size || count + from > size
 
-    count += from - 1
+    raise IndexError.new unless 0 <= from < size && from + count <= size
 
-    from.upto(count) { |i| @buffer[i] = yield i }
+    from.upto(from + count - 1) { |i| @buffer[i] = yield i }
 
     self
   end