Merge pull request #2579 from herwinw/integer_zero_shift

Fix timeout in spec for integer left shift
natalie-lang · Feb 7, 2025 · 025613c · 025613c
2 parents c6c2d1b + e881433
commit 025613c
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 38 deletions.
diff --git a/spec/core/integer/left_shift_spec.rb b/spec/core/integer/left_shift_spec.rb
@@ -182,8 +182,7 @@
     end
 
     ruby_bug "#18517", ""..."3.2" do
-      # NATFIXME: Timeout error
-      xit "returns 0 when m > 0 long and n == 0" do
+      it "returns 0 when m > 0 long and n == 0" do
         (0 << (2**40)).should == 0
       end
     end
@@ -192,22 +191,23 @@
       (0 << bignum_value).should == 0
     end
 
-    ruby_bug "#18518", ""..."3.3" do
-      # NATFIXME: Timeout error
-      xit "raises NoMemoryError when m > 0 and n != 0" do
-        coerce_long = mock("long")
-        coerce_long.stub!(:to_int).and_return(2**40)
-        coerce_bignum = mock("bignum")
-        coerce_bignum.stub!(:to_int).and_return(bignum_value)
-        exps = [2**40, bignum_value, coerce_long, coerce_bignum]
-
-        exps.each { |exp|
-          -> { (1 << exp) }.should raise_error(NoMemoryError)
-          -> { (-1 << exp) }.should raise_error(NoMemoryError)
-          -> { (bignum_value << exp) }.should raise_error(NoMemoryError)
-          -> { (-bignum_value << exp) }.should raise_error(NoMemoryError)
-        }
-      end
+    it "raises RangeError when m > 0 and n != 0" do
+      # https://bugs.ruby-lang.org/issues/18518#note-9
+      limit = RUBY_ENGINE == 'ruby' ? 2**67 : 2**32
+
+      coerce_long = mock("long")
+      coerce_long.stub!(:to_int).and_return(limit)
+      coerce_bignum = mock("bignum")
+      coerce_bignum.stub!(:to_int).and_return(bignum_value)
+      exps = [limit, coerce_long]
+      exps << bignum_value << coerce_bignum if bignum_value >= limit
+
+      exps.each { |exp|
+        -> { (1 << exp) }.should raise_error(RangeError, 'shift width too big')
+        -> { (-1 << exp) }.should raise_error(RangeError, 'shift width too big')
+        -> { (bignum_value << exp) }.should raise_error(RangeError, 'shift width too big')
+        -> { (-bignum_value << exp) }.should raise_error(RangeError, 'shift width too big')
+      }
     end
   end
 end
diff --git a/spec/core/integer/right_shift_spec.rb b/spec/core/integer/right_shift_spec.rb
@@ -204,8 +204,7 @@
     end
 
     ruby_bug "#18517", ""..."3.2" do
-      # NATFIXME: Timeout error
-      xit "returns 0 when m < 0 long and n == 0" do
+      it "returns 0 when m < 0 long and n == 0" do
         (0 >> -(2**40)).should == 0
       end
     end
@@ -214,22 +213,24 @@
       (0 >> -bignum_value).should == 0
     end
 
-    ruby_bug "#18518", ""..."3.4" do
-      # NATFIXME: Timeout error
-      xit "raises NoMemoryError when m < 0 and n != 0" do
-        coerce_long = mock("long")
-        coerce_long.stub!(:to_int).and_return(-(2**40))
-        coerce_bignum = mock("bignum")
-        coerce_bignum.stub!(:to_int).and_return(-bignum_value)
-        exps = [-(2**40), -bignum_value, coerce_long, coerce_bignum]
-
-        exps.each { |exp|
-          -> { (1 >> exp) }.should raise_error(NoMemoryError)
-          -> { (-1 >> exp) }.should raise_error(NoMemoryError)
-          -> { (bignum_value >> exp) }.should raise_error(NoMemoryError)
-          -> { (-bignum_value >> exp) }.should raise_error(NoMemoryError)
-        }
-      end
+    # NATFIXME: Timeout error
+    xit "raises RangeError when m < 0 and n != 0" do
+      # https://bugs.ruby-lang.org/issues/18518#note-9
+      limit = RUBY_ENGINE == 'ruby' ? 2**67 : 2**32
+
+      coerce_long = mock("long")
+      coerce_long.stub!(:to_int).and_return(-limit)
+      coerce_bignum = mock("bignum")
+      coerce_bignum.stub!(:to_int).and_return(-bignum_value)
+      exps = [-limit, coerce_long]
+      exps << -bignum_value << coerce_bignum if bignum_value >= limit
+
+      exps.each { |exp|
+        -> { (1 >> exp) }.should raise_error(RangeError, 'shift width too big')
+        -> { (-1 >> exp) }.should raise_error(RangeError, 'shift width too big')
+        -> { (bignum_value >> exp) }.should raise_error(RangeError, 'shift width too big')
+        -> { (-bignum_value >> exp) }.should raise_error(RangeError, 'shift width too big')
+      }
     end
   end
 end
diff --git a/src/integer_object.cpp b/src/integer_object.cpp
@@ -458,23 +458,32 @@ Value IntegerObject::bitwise_xor(Env *env, Integer &self, Value arg) {
 }
 
 Value IntegerObject::left_shift(Env *env, Integer &self, Value arg) {
+    if (is_zero(self))
+        return Value::integer(0);
     auto integer = arg.to_int(env);
     if (is_bignum(integer)) {
-        if (IntegerObject::is_negative(self))
+        if (IntegerObject::is_negative(self) && IntegerObject::is_negative(integer))
             return Value::integer(-1);
-        else
+        else if (IntegerObject::is_negative(integer))
             return Value::integer(0);
+        else
+            env->raise("RangeError", "shift width too big");
     }
 
     auto nat_int = integer.to_nat_int_t();
 
     if (nat_int < 0)
         return IntegerObject::right_shift(env, self, Value::integer(-nat_int));
 
+    if (nat_int >= (static_cast<nat_int_t>(1) << 32) || nat_int <= -(static_cast<nat_int_t>(1) << 32))
+        env->raise("RangeError", "shift width too big");
+
     return create(self << nat_int);
 }
 
 Value IntegerObject::right_shift(Env *env, Integer &self, Value arg) {
+    if (is_zero(self))
+        return Value::integer(0);
     auto integer = arg.to_int(env);
     if (is_bignum(integer)) {
         if (IntegerObject::is_negative(self))