Bug #18377: Integer#times has different behavior depending on the size of the integer

• Status: Closed
• Assignee: -
• Target version: -
• ruby -v: 3.0.2
• Backport: 2.6: UNKNOWN, 2.7: UNKNOWN, 3.0: UNKNOWN

[jemmai (Jemma Issroff)]

If we redefine + or >, Integer#times adheres to this redefinition for big integers but not for small integers.

Reproduction script to run on Ruby 3.0.2:

ARGV.first.to_i.times do
  Integer.undef_method(:+)
  Integer.define_method(:+) do |_other|
    puts "my custom add"
    exit
  end
end

Using 1 as an argument has no printed output:

$ ruby test_add.rb 1

whereas using 2 ** 64 (18446744073709551615) prints "my custom add":

$ ruby test_add.rb 18446744073709551615
my custom add

We see the same difference when we override behavior of <:

ARGV.first.to_i.times do
  Integer.undef_method(:<)
  Integer.define_method(:<) do |_other|
    puts "my custom less than"
    exit
  end
end

$ ruby test_less_than.rb 1

$ ruby test_less_than.rb 18446744073709551615
my custom less than

Note that the boxed number which changes the Integer#times behavior will vary on different machines.

Options

There are three potential paths forward here:

1. Change behavior for small integers to match behavior for big integer.
   This will have a performance cost. It’s uncommon to redefine + or < on Integers. For this reason, I would argue that correctness on an obscure edge case (especially one that has been unreported since almost the beginning of the language) is less important than performance on the default case.

2. Change behavior for big integers to match behavior for small integers.
   This will mean that behavior is incorrect on both big integers and small integers, but at least it is consistent. Right now it is arbitrary and hard to explain why the behavior would change based on the integer itself. If we keep this behavior consistent for all integers, we can document this clearly in a way that is easy to understand.

3. Do nothing.
   This bug has existed for many years unreported. The last option is to not pursue any code changes, but potentially document this behavior.

Proposal

I propose we pursue option 2 and make the behavior consistent and documented. As I said, it’s unlikely that developers override integer behavior of +, and especially unlikely to do this within an Integer#times block. As long as this is documented and consistent, it seems okay to me to have this known incorrectness to save the performance cost of accommodating different definitions of +.

[Eregon (Benoit Daloze)]

I think this should remain unspecified, or explicitly mention both behavior (respecting +, < or not) are valid.
For instance it's been shown rewriting Integer#times in Ruby is beneficial for JITs (among other things it also enables inlining and OSR), and then using +/< is natural.
But if times is written in C/Java it doesn't make sense to call the +/< methods dynamically.

The inconsistency for bignums is indeed surprising, I think that is worth fixing in CRuby.

[jeremyevans0 (Jeremy Evans)]

Eregon (Benoit Daloze) wrote in #note-1:

The inconsistency for bignums is indeed surprising, I think that is worth fixing in CRuby.

Agreed. Here's a pull request to make bignums operate like fixnums: https://github.com/ruby/ruby/pull/5199

• Status changed from Open to Closed

Applied in changeset git|fe1725236c8a4d6cb780874c470f7f443185ed38.

---

Don't call + and < in Integer.times for !FIXNUM

The methods aren't called for FIXNUM, and it's best to have
consistent behavior.

Fixes [Bug #18377]

[jemmai (Jemma Issroff)]

Thanks for the quick response Jeremy! I'm very new to opening issues on here and making fixes on Ruby. Quick question on process though - I had basically that PR written already. Should I have linked it above?

I had thought (maybe incorrectly) to wait until someone responded agreeing with the proposal to link the code change too.

[jeremyevans0 (Jeremy Evans)]

jemmai (Jemma Issroff) wrote in #note-4:

Thanks for the quick response Jeremy! I'm very new to opening issues on here and making fixes on Ruby. Quick question on process though - I had basically that PR written already. Should I have linked it above?

If you already have a fix prepared, then it is best to link to it when making your post. That saves the committers from having to develop the fix ourselves. It was easy in this case, but certainly the more difficult the fix, the better to provide a link up front.

I had thought (maybe incorrectly) to wait until someone responded agreeing with the proposal to link the code change too.

It's best to get the link to the proposed fix up front. For one, it helps gauge how invasive the fix would be (minimal in this case). I would only wait to post a link to the fix if you haven't developed such a fix yet. Usually this would be for a more complex bug with multiple different approaches to fixing, where developing a fix would take substantial time, and you don't want to take that time unless you are fairly sure the approach used will be accepted.