[Feature #21979] Allow negative offset in unpack

Author: nobu

pack.c

@@ -1024,9 +1024,10 @@ pack_unpack_internal(VALUE str, VALUE fmt, enum unpack_mode mode, long offset)
     StringValue(fmt);
     rb_must_asciicompat(fmt);
 
-    if (offset < 0) rb_raise(rb_eArgError, "offset can't be negative");
     len = RSTRING_LEN(str);
-    if (offset > len) rb_raise(rb_eArgError, "offset outside of string");
+    if (offset < 0 ? (offset += len) < 0 : offset > len) {
+        rb_raise(rb_eArgError, "offset outside of string");
+    }
 
     s = RSTRING_PTR(str);
     send = s + len;

spec/ruby/core/string/unpack1_spec.rb

@@ -20,8 +20,22 @@
     "؈".unpack1("C", offset: 1).should == 136
   end
 
-  it "raises an ArgumentError when the offset is negative" do
-    -> { "a".unpack1("C", offset: -1) }.should.raise(ArgumentError, "offset can't be negative")
+  describe "when the offset is negative" do
+    ruby_version_is "4.1" do
+      it "starts unpacking from the end" do
+        "abc".unpack1("C", offset: -2).should == 98
+      end
+
+      it "raises an ArgumentError if it is less than -length" do
+        -> { "a".unpack1("C", offset: -2) }.should.raise(ArgumentError, "offset outside of string")
+      end
+    end
+
+    ruby_version_is ""..."4.1" do
+      it "raises an ArgumentError" do
+        -> { "a".unpack1("C", offset: -1) }.should.raise(ArgumentError, "offset can't be negative")
+      end
+    end
   end
 
   it "returns nil if the offset is at the end of the string" do

spec/ruby/core/string/unpack_spec.rb

@@ -18,8 +18,22 @@
     "؈".unpack("CC", offset: 1).should == [136, nil]
   end
 
-  it "raises an ArgumentError when the offset is negative" do
-    -> { "a".unpack("C", offset: -1) }.should.raise(ArgumentError, "offset can't be negative")
+  describe "when the offset is negative" do
+    ruby_version_is "4.1" do
+      it "starts unpacking from the end" do
+        "abc".unpack("CC", offset: -2).should == [98, 99]
+      end
+
+      it "raises an ArgumentError if it is less than -length" do
+        -> { "a".unpack("C", offset: -2) }.should.raise(ArgumentError, "offset outside of string")
+      end
+    end
+
+    ruby_version_is ""..."4.1" do
+      it "raises an ArgumentError" do
+        -> { "a".unpack("C", offset: -1) }.should.raise(ArgumentError, "offset can't be negative")
+      end
+    end
   end
 
   it "returns nil if the offset is at the end of the string" do

test/ruby/test_pack.rb

@@ -913,27 +913,29 @@ def test_unpack1
 
   def test_unpack1_offset
     assert_equal 65, "ZA".unpack1("C", offset: 1)
+    assert_equal 65, "ZA".unpack1("C", offset: -1)
     assert_equal "01000001", "YZA".unpack1("B*", offset: 2)
     assert_nil "abc".unpack1("C", offset: 3)
-    assert_raise_with_message(ArgumentError, /offset can't be negative/) {
-      "a".unpack1("C", offset: -1)
-    }
     assert_raise_with_message(ArgumentError, /offset outside of string/) {
       "a".unpack1("C", offset: 2)
     }
+    assert_raise_with_message(ArgumentError, /offset outside of string/) {
+      "a".unpack1("C", offset: -2)
+    }
     assert_nil "a".unpack1("C", offset: 1)
   end
 
   def test_unpack_offset
     assert_equal [65], "ZA".unpack("C", offset: 1)
+    assert_equal [65], "ZA".unpack("C", offset: -1)
     assert_equal ["01000001"], "YZA".unpack("B*", offset: 2)
     assert_equal [nil, nil, nil], "abc".unpack("CCC", offset: 3)
-    assert_raise_with_message(ArgumentError, /offset can't be negative/) {
-      "a".unpack("C", offset: -1)
-    }
     assert_raise_with_message(ArgumentError, /offset outside of string/) {
       "a".unpack("C", offset: 2)
     }
+    assert_raise_with_message(ArgumentError, /offset outside of string/) {
+      "a".unpack("C", offset: -2)
+    }
     assert_equal [nil], "a".unpack("C", offset: 1)
   end