Bug #12706


Hash#each yields inconsistent number of args

Added by bughit (bug hit) almost 5 years ago. Updated about 1 year ago.

Target version:


def foo(a, b)
  p [a, b]

def bar(a, b = 2)
  p [a, b]

foo_lambda = method(:foo).to_proc
bar_lambda = method(:bar).to_proc

{a: 1}.each(&foo_lambda)
{a: 1}.each(&bar_lambda)

From #12705, yielding to method lambdas uses lambda/method arg semnatics

the yield to foo produces [:a, 1] suggesting that each is yielding two values yield key, value
but yield to bar produces [[:a, 1], 2] suggesting that each is yielding one value yield [key, value]

it would be better if you always knew what to expect from it

Related issues

Related to Ruby master - Bug #16948: hash.each(&method(:something)) behavior changed without warning on masterClosedActions
Related to Ruby master - Bug #17197: Some Hash methods still have arity 2 instead of 1Rejectednobu (Nobuyoshi Nakada)Actions

Updated by matz (Yukihiro Matsumoto) over 1 year ago

It was caused by the optimization introduced in 2.1. It should check if a block is a lambda before making optimization.
We worry about compatibility but let's fix it in 2.8(3.0) and see it can cause problems. Please mark the change as experimental .


Actions #2

Updated by mame (Yusuke Endoh) over 1 year ago

  • Status changed from Open to Closed

Applied in changeset git|47141797bed55eb10932c9a722a5132f50d4f3d8.

hash.c: Do not use the fast path (rb_yield_values) for lambda blocks

As a semantics, Hash#each yields a 2-element array (pairs of keys and
values). So, { a: 1 }.each(&->(k, v) { }) should raise an exception
due to lambda's arity check.
However, the optimization that avoids Array allocation by using
rb_yield_values for blocks whose arity is more than 1 (introduced at
b9d29603375d17c3d1d609d9662f50beaec61fa1 and some commits), seemed to
overlook the lambda case, and wrongly allowed the code above to work.

This change experimentally attempts to make it strict; now the code
above raises an ArgumentError. This is an incompatible change; if the
compatibility issue is bigger than our expectation, it may be reverted
(until Ruby 3.0 release).

[Bug #12706]

Updated by Eregon (Benoit Daloze) over 1 year ago

Does this cause any issue in practice?

AFAIK it's not worth the incompatibility and could break many things.
We had to follow MRI behavior here for Hash#each and Hash#map in TruffleRuby, e.g.,

IMHO the right thing to do is to yield 2 values here, and having an Array for backward compatibility if arity != 2 seems OK.

Actions #4

Updated by jeremyevans0 (Jeremy Evans) about 1 year ago

  • Related to Bug #16948: hash.each(&method(:something)) behavior changed without warning on master added

Updated by marcandre (Marc-Andre Lafortune) about 1 year ago

Interesting. Does it intend to fix just this case, or any inconsistencies I listed in ?

Eregon (Benoit Daloze): given the current state of affairs, I consider lambdas & multiple yield a very bad idea until things make some sense.

Actions #6

Updated by nobu (Nobuyoshi Nakada) 9 months ago

  • Related to Bug #17197: Some Hash methods still have arity 2 instead of 1 added

Also available in: Atom PDF