Project

General

Profile

Actions

Bug #2184

closed

Blocks strange behavior

Added by kronos (Ivan Samsonov) over 14 years ago. Updated almost 13 years ago.

Status:
Closed
Target version:
-
ruby -v:
ruby 1.9.1p243 (2009-07-16 revision 24175) [i386-darwin10]
Backport:
[ruby-core:25989]

Description

=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

Actions #1

Updated by zenspider (Ryan Davis) over 14 years ago

=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

Actions #2

Updated by naruse (Yui NARUSE) over 14 years ago

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

=begin

=end

Actions #3

Updated by mame (Yusuke Endoh) about 14 years ago

=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
=end

Actions #4

Updated by mame (Yusuke Endoh) about 14 years ago

  • 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

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0