Project

General

Profile

Actions

Bug #19530

closed

`Array#sum` and `Enumerable#sum` sometimes show different behaviours

Added by dstosik (David Stosik) over 1 year ago. Updated 6 months ago.

Status:
Closed
Assignee:
-
Target version:
-
ruby -v:
ruby 3.2.1 (2023-02-08 revision 31819e82c8) [arm64-darwin22]
[ruby-core:112873]

Description

Hi everyone. 👋🏻

We recently discovered that Array#sum and Enumerable#sum will output different results in some edge cases.

Here's the smallest script I managed to write to reproduce the issue:

class Money
  def initialize(amount)
    @amount = amount.to_f
  end

  def +(other)
    self.class.new(@amount + other.to_f)
  end

  def to_f
    @amount
  end
end

p [7.0].each.sum(Money.new(0))   #=> #<Money:0x00000001005b35f0 @amount=7.0>
p [7.0]     .sum(Money.new(0))   #=> 7.0 💥

I understand that it is expected that #sum may not honor custom definitions of the #+ method (particularly when the summed values are Float).

However, I would like to bring your attention to the fact that, in the example above, calling #sum on an Array of Float values and calling #sum on an Enumerable that yields the same Float values will return results of different types.

I've reproduced the same behaviour with multiple versions of Ruby going from 2.6.5 to 3.2.1.

Ideally, I would expect [7.0].sum(Money.new(0)) to return a Money object identical to the one returned by [7.0].each.sum(Money.new(0)).
I think it would make sense if at least they returned an identical value (even if it is a Float).

Addendum: I forgot to mention this extract of the Array#sum documentation:

When no block is given, returns the object equivalent to:

sum = init
array.each {|element| sum += element }
sum

With the example above, it would indeed return a Money object.

Actions #1

Updated by dstosik (David Stosik) over 1 year ago

  • Description updated (diff)

Updated by byroot (Jean Boussier) over 1 year ago

Looking at Array#sum implementation, it very clearly has a fast path for when all the elements of the array are native numeric types, with a fallback for other objects. But it also doesn't check the initial value type before going in the fast path.

That should be easy enough to fix.

Actions #4

Updated by byroot (Jean Boussier) over 1 year ago

  • Backport changed from 2.7: UNKNOWN, 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN to 2.7: WONTFIX, 3.0: REQUIRED, 3.1: REQUIRED, 3.2: REQUIRED
Actions #5

Updated by byroot (Jean Boussier) over 1 year ago

  • Status changed from Open to Closed

Applied in changeset git|ca437aeb395e77125fcd2ab9bc83bbcd3e357610.


rb_ary_sum: don't enter fast path if initial isn't a native numeric type.

[Bug #19530]

If the initial value isn't one of the special cased types, we directly
jump to the slow path.

Updated by nagachika (Tomoyuki Chikanaga) 6 months ago

  • Backport changed from 2.7: WONTFIX, 3.0: REQUIRED, 3.1: REQUIRED, 3.2: REQUIRED to 2.7: WONTFIX, 3.0: REQUIRED, 3.1: REQUIRED, 3.2: WONTFIX

Since this was a long standing bug, I won't backport it to stable branch in general. If you know any real applications suffered from this behavior, please let me know.
Thanks.

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0