From 71ce7e1825c5b8fe08dd96cd77c6a379afd34256 Mon Sep 17 00:00:00 2001 From: Nobuyoshi Nakada Date: Sun, 15 Jan 2023 13:03:27 +0900 Subject: [Bug #19335] `Integer#remainder` should respect `#coerce` (#7120) Also `Numeric#remainder` should. --- numeric.c | 21 +++++++++++++++++---- spec/ruby/core/numeric/remainder_spec.rb | 3 +++ test/ruby/test_float.rb | 6 ++++++ test/ruby/test_integer_comb.rb | 23 ++++++++++++++++++----- 4 files changed, 44 insertions(+), 9 deletions(-) diff --git a/numeric.c b/numeric.c index 0312f4939f..15729e4728 100644 --- a/numeric.c +++ b/numeric.c @@ -740,6 +740,9 @@ num_modulo(VALUE x, VALUE y) static VALUE num_remainder(VALUE x, VALUE y) { + if (!rb_obj_is_kind_of(y, rb_cNumeric)) { + do_coerce(&x, &y, TRUE); + } VALUE z = num_funcall1(x, '%', y); if ((!rb_equal(z, INT2FIX(0))) && @@ -4363,12 +4366,22 @@ static VALUE int_remainder(VALUE x, VALUE y) { if (FIXNUM_P(x)) { - return num_remainder(x, y); + if (FIXNUM_P(y)) { + VALUE z = fix_mod(x, y); + assert(FIXNUM_P(z)); + if (z != INT2FIX(0) && (SIGNED_VALUE)(x ^ y) < 0) + z = fix_minus(z, y); + return z; + } + else if (!RB_BIGNUM_TYPE_P(y)) { + return num_remainder(x, y); + } + x = rb_int2big(FIX2LONG(x)); } - else if (RB_BIGNUM_TYPE_P(x)) { - return rb_big_remainder(x, y); + else if (!RB_BIGNUM_TYPE_P(x)) { + return Qnil; } - return Qnil; + return rb_big_remainder(x, y); } static VALUE diff --git a/spec/ruby/core/numeric/remainder_spec.rb b/spec/ruby/core/numeric/remainder_spec.rb index 1e2f5f3a96..674fa22d8e 100644 --- a/spec/ruby/core/numeric/remainder_spec.rb +++ b/spec/ruby/core/numeric/remainder_spec.rb @@ -6,6 +6,9 @@ describe "Numeric#remainder" do @obj = NumericSpecs::Subclass.new @result = mock("Numeric#% result") @other = mock("Passed Object") + ruby_version_is "3.3" do + @other.should_receive(:coerce).with(@obj).and_return([@obj, @other]) + end end it "returns the result of calling self#% with other if self is 0" do diff --git a/test/ruby/test_float.rb b/test/ruby/test_float.rb index 35cde1e951..b91b904d1e 100644 --- a/test/ruby/test_float.rb +++ b/test/ruby/test_float.rb @@ -227,6 +227,12 @@ class TestFloat < Test::Unit::TestCase assert_equal(-3.5, (-11.5).remainder(-4)) assert_predicate(Float::NAN.remainder(4), :nan?) assert_predicate(4.remainder(Float::NAN), :nan?) + + ten = Object.new + def ten.coerce(other) + [other, 10] + end + assert_equal(4, 14.0.remainder(ten)) end def test_to_s diff --git a/test/ruby/test_integer_comb.rb b/test/ruby/test_integer_comb.rb index 1ad13dd31b..150f45cfd7 100644 --- a/test/ruby/test_integer_comb.rb +++ b/test/ruby/test_integer_comb.rb @@ -408,19 +408,32 @@ class TestIntegerComb < Test::Unit::TestCase end def test_remainder + coerce = EnvUtil.labeled_class("CoerceNum") do + def initialize(num) + @num = num + end + def coerce(other) + [other, @num] + end + def inspect + "#{self.class.name}(#@num)" + end + alias to_s inspect + end + VS.each {|a| - VS.each {|b| - if b == 0 + (VS + VS.map {|b| [coerce.new(b), b]}).each {|b, i = b| + if i == 0 assert_raise(ZeroDivisionError) { a.divmod(b) } else - r = a.remainder(b) + r = assert_nothing_raised(ArgumentError, "#{a}.remainder(#{b})") {a.remainder(b)} assert_kind_of(Integer, r) if a < 0 - assert_operator(-b.abs, :<, r, "#{a}.remainder(#{b})") + assert_operator(-i.abs, :<, r, "#{a}.remainder(#{b})") assert_operator(0, :>=, r, "#{a}.remainder(#{b})") elsif 0 < a assert_operator(0, :<=, r, "#{a}.remainder(#{b})") - assert_operator(b.abs, :>, r, "#{a}.remainder(#{b})") + assert_operator(i.abs, :>, r, "#{a}.remainder(#{b})") else assert_equal(0, r, "#{a}.remainder(#{b})") end -- cgit v1.2.1