Bug #2184: Blocks strange behavior

• Status: Closed
• Assignee: matz (Yukihiro Matsumoto)
• Target version: -
• ruby -v: ruby 1.9.1p243 (2009-07-16 revision 24175) [i386-darwin10]

[kronos (Ivan Samsonov)]

=begin
1.8.7:
a = lambda {|x,y| x <=> y}
[1,2,3].max(&a) # => 3

1.9.1:
z = lambda {|x,y| x <=> y}
[3,2,1].max(&z) # => ArgumentError: wrong number of arguments (1 for 2)

But it works in 1.9:
z = lambda {|x| x[0] <=> x[1]}
[3,2,1].max(&z) # => 3
=end

[zenspider (Ryan Davis)]

=begin

On Oct 7, 2009, at 00:39 , Ivan Samsonov wrote:

Bug #2184: Blocks strange behavior
http://redmine.ruby-lang.org/issues/show/2184

Author: Ivan Samsonov
Status: Open, Priority: Normal
Category: core
ruby -v: ruby 1.9.1p243 (2009-07-16 revision 24175) [i386-darwin10]

1.8.7:
a = lambda {|x,y| x <=> y}
[1,2,3].max(&a) # => 3

1.9.1:
z = lambda {|x,y| x <=> y}
[3,2,1].max(&z) # => ArgumentError: wrong number of arguments (1 for
2)

But it works in 1.9:
z = lambda {|x| x[0] <=> x[1]}
[3,2,1].max(&z) # => 3

Interesting. In 1.9, lambda is more strict about arg checking and in
this case max is passing in an array of pairs. You can try any of the
following instead:

% multiruby -e 'z = lambda {|(x,y)| x <=> y}; [3,2,1].max(&z)'
...
TOTAL RESULT = 0 failures out of 5

Passed: 1.9.1-p129, 1.8.7-p72, 1.8.7-p160, 1.8.6-p368, 1.8.7-p174
Failed:

507 % multiruby -e 'z = Proc.new {|x,y| x <=> y}; [3,2,1].max(&z)'
...
TOTAL RESULT = 0 failures out of 5

Passed: 1.9.1-p129, 1.8.7-p72, 1.8.7-p160, 1.8.6-p368, 1.8.7-p174
Failed:

% multiruby -e 'z = proc {|x,y| x <=> y}; [3,2,1].max(&z)'
...
TOTAL RESULT = 0 failures out of 5

Passed: 1.9.1-p129, 1.8.7-p72, 1.8.7-p160, 1.8.6-p368, 1.8.7-p174
Failed:
509 %

=end

[naruse (Yui NARUSE)]

• Category set to core
• Status changed from Open to Assigned
• Assignee set to matz (Yukihiro Matsumoto)

=begin

=end

[mame (Yusuke Endoh)]

=begin
Hi,

1.9.1:
z = lambda {|x,y| x <=> y}
[3,2,1].max(&z) # => ArgumentError: wrong number of arguments (1 for 2)

But it works in 1.9:
z = lambda {|x| x[0] <=> x[1]}
[3,2,1].max(&z) # => 3

Since r11431, 1.9 had been optimized Enumerable#each_with_index, min
and max by reusing array for yield parameters. This causes the above
behavior.

But the optimization was found wrong in [ruby-dev:32181], which may
cause unexpected behaviors:

$ ./ruby -e '(1..5).max {|a| p a; a << :foo; a[0] <=> a[1] }'
[2, 1]
[3, 2, :foo]
[4, 3, :foo, :foo]
[5, 4, :foo, :foo, :foo] <= unexpected accumulation

$ ./ruby -e 'b = nil; (1..5).max {|a| p a; b ||= a.freeze; a[0] <=> a[1] }; p b'
[2, 1]
[3, 2]
[4, 3]
[5, 4]
[5, 4] <= expected [2, 1]

So, the opt of Enumerable#each_with_index was reverted, but min and
max seemed to be forgotten.

Here is a patch to fix Enumerable#min, max and minmax. Unless anyone
expresses opposition, I'll commit.

diff --git a/enum.c b/enum.c
index 6eeb62c..4f930ae 100644
--- a/enum.c
+++ b/enum.c
@@ -1058,10 +1058,7 @@ min_ii(VALUE i, VALUE *memo, int argc, VALUE *argv)
*memo = i;
}
else {

• VALUE ary = memo[1];
• RARRAY_PTR(ary)[0] = i;
• RARRAY_PTR(ary)[1] = *memo;
• cmp = rb_yield(ary);

• cmp = rb_yield_values(2, i, *memo);
  if (rb_cmpint(cmp, i, *memo) < 0) {
  *memo = i;
  }
  @@ -1087,18 +1084,16 @@ min_ii(VALUE i, VALUE *memo, int argc, VALUE *argv)
  static VALUE
  enum_min(VALUE obj)
  {

• VALUE result[2];

• VALUE result = Qundef;

• result[0] = Qundef;
  if (rb_block_given_p()) {
• result[1] = rb_ary_new3(2, Qnil, Qnil);
• rb_block_call(obj, id_each, 0, 0, min_ii, (VALUE)result);

• rb_block_call(obj, id_each, 0, 0, min_ii, (VALUE)&result);
  }
  else {

• rb_block_call(obj, id_each, 0, 0, min_i, (VALUE)result);

• rb_block_call(obj, id_each, 0, 0, min_i, (VALUE)&result);
  }

• if (result[0] == Qundef) return Qnil;
• return result[0];

• if (result == Qundef) return Qnil;
• return result;
  }

static VALUE
@@ -1131,10 +1126,7 @@ max_ii(VALUE i, VALUE *memo, int argc, VALUE *argv)
*memo = i;
}
else {

• VALUE ary = memo[1];
• RARRAY_PTR(ary)[0] = i;
• RARRAY_PTR(ary)[1] = *memo;
• cmp = rb_yield(ary);

• cmp = rb_yield_values(2, i, *memo);
  if (rb_cmpint(cmp, i, *memo) > 0) {
  *memo = i;
  }
  @@ -1159,24 +1151,21 @@ max_ii(VALUE i, VALUE *memo, int argc, VALUE *argv)
  static VALUE
  enum_max(VALUE obj)
  {

• VALUE result[2];

• VALUE result = Qundef;

• result[0] = Qundef;
  if (rb_block_given_p()) {
• result[1] = rb_ary_new3(2, Qnil, Qnil);
• rb_block_call(obj, id_each, 0, 0, max_ii, (VALUE)result);

• rb_block_call(obj, id_each, 0, 0, max_ii, (VALUE)&result);
  }
  else {

• rb_block_call(obj, id_each, 0, 0, max_i, (VALUE)result);

• rb_block_call(obj, id_each, 0, 0, max_i, (VALUE)&result);
  }

• if (result[0] == Qundef) return Qnil;
• return result[0];

• if (result == Qundef) return Qnil;
• return result;
  }

struct minmax_t {
VALUE min;
VALUE max;

• VALUE ary;
  VALUE last;
  };

@@ -1242,17 +1231,11 @@ minmax_ii_update(VALUE i, VALUE j, struct minmax_t *memo)
memo->max = j;
}
else {

• VALUE ary = memo->ary;

•    rb_ary_store(ary, 0, i);
  

•    rb_ary_store(ary, 1, memo->min);
  

• n = rb_cmpint(rb_yield(ary), i, memo->min);

• n = rb_cmpint(rb_yield_values(2, i, memo->min), i, memo->min);
  if (n < 0) {
  memo->min = i;
  }

•    rb_ary_store(ary, 0, j);
  

•    rb_ary_store(ary, 1, memo->max);
  

• n = rb_cmpint(rb_yield(ary), j, memo->max);

• n = rb_cmpint(rb_yield_values(2, j, memo->max), j, memo->max);
  if (n > 0) {
  memo->max = j;
  }
  @@ -1264,7 +1247,7 @@ minmax_ii(VALUE i, VALUE _memo, int argc, VALUE *argv)
  {
  struct minmax_t *memo = (struct minmax_t *)_memo;
  int n;

• VALUE ary, j;

• VALUE j;

  ENUM_WANT_SVALUE();

@@ -1275,10 +1258,7 @@ minmax_ii(VALUE i, VALUE _memo, int argc, VALUE *argv)
j = memo->last;
memo->last = Qundef;

• ary = memo->ary;
• rb_ary_store(ary, 0, j);
• rb_ary_store(ary, 1, i);
• n = rb_cmpint(rb_yield(ary), j, i);

• n = rb_cmpint(rb_yield_values(2, j, i), j, i);
  if (n == 0)
  i = j;
  else if (n < 0) {
  @@ -1317,7 +1297,6 @@ enum_minmax(VALUE obj)
  memo.min = Qundef;
  memo.last = Qundef;
  if (rb_block_given_p()) {

• memo.ary = ary;
  rb_block_call(obj, id_each, 0, 0, minmax_ii, (VALUE)&memo);
  if (memo.last != Qundef)
  minmax_ii_update(memo.last, memo.last, &memo);

--
Yusuke ENDOH [e-mail]
=end

[mame (Yusuke Endoh)]

• Status changed from Assigned to Closed
• % Done changed from 0 to 100

=begin
This issue was solved with changeset r26866.
Ivan, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

=end