Project

General

Profile

Actions

Bug #13700

closed

Enumerable#sum may not work for Ranges subclasses due to optimization

Added by sos4nt (Stefan Schüßler) almost 7 years ago. Updated over 4 years ago.

Status:
Closed
Assignee:
-
Target version:
-
ruby -v:
ruby 2.4.1p111 (2017-03-22 revision 58053) [x86_64-darwin15]
[ruby-core:81847]

Description

Enumerable#sum is optimized for integer ranges. Unfortunately, this can break subclasses:

class StepTwoRange < Range
  def each(&block)
    step(2, &block)
  end
end

r = StepTwoRange.new(0, 10)
r.to_a     #=> [0, 2, 4, 6, 8, 10]

r.to_a.sum #=> 30
r.sum      #=> 55

The optimization should therefore only be triggered for instances of Range and not for instances of subclasses. (or more specifically, not for subclasses overriding each)

If this behavior is intentional, it should at least be documented.

Actions #1

Updated by sos4nt (Stefan Schüßler) almost 7 years ago

  • Description updated (diff)
Actions #2

Updated by sos4nt (Stefan Schüßler) almost 7 years ago

  • Description updated (diff)

Updated by shevegen (Robert A. Heiler) almost 7 years ago

Reminds me a bit of what hanmac wrote elsewhere; I can't find it right now and forgot it mostly already but I think he also mentioned some unexpected behaviour when ... subclassing I think? Or some custom class that he wrote...

Updated by Hanmac (Hans Mackowiak) almost 7 years ago

shevegen (Robert A. Heiler) wrote:

Reminds me a bit of what hanmac wrote elsewhere; I can't find it right now and forgot it mostly already but I think he also mentioned some unexpected behaviour when ... subclassing I think? Or some custom class that he wrote...

my comment was for https://bugs.ruby-lang.org/issues/13663
with String#upto you can't overwrite the internal String#<=>

Updated by sos4nt (Stefan Schüßler) almost 7 years ago

Side note: in my opinion, Enumerable should not check the receiver's class to provide specific optimizations. Instead, Range should override sum and handle the optimization itself. Range#sum would also be a better place to document this behavior. (it still doesn't solve the subclassing issue, though)

Actions #6

Updated by jeremyevans (Jeremy Evans) over 4 years ago

  • Status changed from Open to Closed

Applied in changeset git|e496e96547b64c3a2fa6f285c3bc9bd21a245ac6.


Document that Enumerable#sum may not respect redefinition of Range#each

It already documented that it may not respect redefinition
of Integer#+.

Fixes [Bug #13700]

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0