Using "sort!" in a C extension (1.9 problem)

[Andre Nathan]

Hello

In ruby 1.8, I could use Array#sort! in a C extension like this:

static VALUE
call_sort_bang(VALUE ary)
{
return rb_funcall(ary, rb_intern("sort!"), 0);
}

static VALUE
sort_i(VALUE ary)
{
long v1 = FIX2LONG(RARRAY_PTR(ary)[0]);
long v2 = FIX2LONG(RARRAY_PTR(ary)[1]);
return LONG2FIX(v1 - v2);
}

static VALUE
foo(void)
{
VALUE ary = rb_ary_new();
rb_ary_push(ary, INT2FIX(1));
rb_ary_push(ary, INT2FIX(0));
rb_ary_push(ary, INT2FIX(2));
rb_iterate(call_sort_bang, ary, sort_i, 0);
rb_funcall(rb_mKernel, rb_intern("p"), 1, ary);
return a;
}

This would print "[0, 1, 2]" as expected. However, it doesn't work in
ruby1.9 compiled from svn (checked out today). The problem seems to be
that only one of the array elements is being passed to sort_i(), instead
of a pair of elements as it's done in 1.8.

What is the correct way to do this in 1.9?


Thanks in advance,
Andre

 

[Andre Nathan]

My example was a bit contrieved because I could just have used #sort
without a block. What I'm actually doing is sorting an array by the
length of its elements, which are also arrays. That's why I need the
block version of #sort.

 

[Siep Korteling]

My example was a bit contrieved because I could just have used #sort
without a block. What I'm actually doing is sorting an array by the
length of its elements, which are also arrays. That's why I need the
block version of #sort.

Like this?
a=[[2],[1,2,3],[1],[1,2]]
p a.sort_by{|ar|ar.length}

 

[Andre Nathan]

Like this?
a=[[2],[1,2,3],[1],[1,2]]
p a.sort_by{|ar|ar.length}

Yes but in a C extension. The code I have is not working with 1.9.

Andre

 

[Andre Nathan]

Like this?
a=[[2],[1,2,3],[1],[1,2]]
p a.sort_by{|ar|ar.length}

Yes but in a C extension. The code I have is not working with 1.9.

Well not exactly. sort_by works but I'd prefer to use sort! with a
block, as it does the sorting in-place.

In 1.9, if I inspect "ary" below, I only see the current array element.
In 1.8, it is an array of the two elements which are being compared.

static VALUE
sort_i(VALUE ary)
{
long v1 = FIX2LONG(RARRAY_PTR(ary)[0]);
long v2 = FIX2LONG(RARRAY_PTR(ary)[1]);
return LONG2FIX(v1 - v2);
}

Andre

 

[KUBO Takehiro]

Hi,

rb_iterate(call_sort_bang, ary, sort_i, 0);

Use rb_ary_sort_bang instead of call_sort_bang.
I don't know why call_sort_bang doesn't work.

 

[Andre Nathan]

Use rb_ary_sort_bang instead of call_sort_bang.
I don't know why call_sort_bang doesn't work.

Thanks! For some reason I thought rb_ary_sort_bang was static while it
isn't...

It would still be interesting to know why this didn't work, since some
methods are indeed static and rb_funcall() is the only way to call them.

Andre

 

[Andre Nathan]

Thanks! For some reason I thought rb_ary_sort_bang was static while it
isn't...

Actually...

I was using rb_funcall() because this way "sort!" understands it's being
given a block, while when calling rb_ary_sort_block() directly it
doesnt:

static VALUE
sort_by_length(VALUE ary)
{
long v0 = RARRAY_LEN(RARRAY_PTR(ary)[0]);
long v1 = RARRAY_LEN(RARRAY_PTR(ary)[1]);
return LONG2FIX(v0 - v1);
}

static VALUE
foo(void)
{
VALUE a = rb_ary_new();

rb_ary_push(a, rb_ary_new3(2, INT2FIX(3), INT2FIX(3)));
rb_ary_push(a, rb_ary_new3(1, INT2FIX(2)));
rb_ary_push(a, rb_ary_new3(3, INT2FIX(0), INT2FIX(0), INT2FIX(0)));

rb_iterate(rb_ary_sort_bang, a, sort_by_length, 0);

rb_funcall(rb_mKernel, rb_intern("p"), 1, a);

return a;
}

This prints "[[0, 0, 0], [2], [3, 3]]" and not "[[2], [3,3], [0, 0, 0]]"
as expected. sort_by_length() is never called, and the sub-arrays were
sorted by their contents, because rb_block_given_p() returns false in
rb_ary_sort_bang().

I also tried

rb_block_call(a, rb_intern("sort!"), 0, 0, sort_by_length, 0);

but that has the same problem I had with rb_funcall -- sort_by_length is
given the current array element only as its argument.

Andre

 

[KUBO Takehiro]

Sorry. This is my mistake.

but that has the same problem I had with rb_funcall -- sort_by_length is
given the current array element only as its argument.

How about the following patch to ruby svn trunk?

--- vm_insnhelper.c (revision 14790)
+++ vm_insnhelper.c (working copy)
@@ -652,9 +652,12 @@
else if (argc == 0) {
arg = Qnil;
}
- else {
+ else if (argc == 1) {
arg = argv[0];
}
+ else {
+ arg = rb_ary_new4(argc, argv);
+ }

vm_push_frame(th, 0, FRAME_MAGIC_IFUNC,
self, (VALUE)block->dfp,

I'm not certain this is a correct patch. But at least, sort_by_length
works fine.

 

[Andre Nathan]

How about the following patch to ruby svn trunk?

Thanks, that fixed it for me! I have another machine with a checkout
from 2007-12-07 which doesn't have this problem, and the code in
vm_yield_with_cfunc() is

if (lambda) {
arg = rb_ary_new4(argc, argv);
}
else {
if (argc == 1) {
arg = *argv;
}
else if (argc > 1) {
arg = rb_ary_new4(argc, argv);
}
else {
arg = rb_ary_new();
}
}

so I believe your patch is correct.

Thanks a lot,
Andre

 

[Ken Bloom]

Like this?
a=[[2],[1,2,3],[1],[1,2]]
p a.sort_by{|ar|ar.length}

Yes but in a C extension. The code I have is not working with 1.9.

Well not exactly. sort_by works but I'd prefer to use sort! with a
block, as it does the sorting in-place.

In 1.9, if I inspect "ary" below, I only see the current array element.
In 1.8, it is an array of the two elements which are being compared.

static VALUE
sort_i(VALUE ary)
{
long v1 = FIX2LONG(RARRAY_PTR(ary)[0]); long v2 =
FIX2LONG(RARRAY_PTR(ary)[1]); return LONG2FIX(v1 - v2);
}

Andre

This appears to be the result of a ruby 1.9 semantic change for block
arguments.


(from eigenclass.org's summary)

News semantics for block arguments

|v| now works like the former |v,|:

[RUBY_VERSION, RUBY_RELEASE_DATE] # => ["1.8.5", "2006-08-25"]
def m; yield 1, 2; end
m{|v| v} # => [1, 2] # !>
multiple values for a block parameter (2 for 1)

vs.

[RUBY_VERSION, RUBY_RELEASE_DATE] # => ["1.9.0", "2007-08-03"]
def m; yield 1, 2; end
m{|v| v} # => 1

So sort_i needs to be revised to take two parameters, as follows:

static VALUE
sort_i(VALUE first, VALUE second)
{
long v1 = FIX2LONG(first);
long v2 = FIX2LONG(second);
return LONG2FIX(v1 - v2);
}

(Note that I have not tested this, and I have not programmed a C
extension ever without the help of SWIG.)

--Ken

 

[Ken Bloom]

Sorry. This is my mistake.

but that has the same problem I had with rb_funcall -- sort_by_length
is given the current array element only as its argument.

How about the following patch to ruby svn trunk?

--- vm_insnhelper.c (revision 14790)
+++ vm_insnhelper.c (working copy)
@@ -652,9 +652,12 @@
else if (argc == 0) {
arg = Qnil;
}
- else {
+ else if (argc == 1) {
arg = argv[0];
}
+ else {
+ arg = rb_ary_new4(argc, argv);
+ }

vm_push_frame(th, 0, FRAME_MAGIC_IFUNC,
self, (VALUE)block->dfp,

I'm not certain this is a correct patch. But at least, sort_by_length
works fine.

Was this a bug that needs to be patched, or an intentional change from
Ruby 1.8 to Ruby 1.9? I thought it was the later.

--Ken

 

[Andre Nathan]

Hi

So sort_i needs to be revised to take two parameters, as follows:

static VALUE
sort_i(VALUE first, VALUE second)
{
long v1 = FIX2LONG(first);
long v2 = FIX2LONG(second);
return LONG2FIX(v1 - v2);
}

I've tried this, but "second" has the value "false", while "first" is
still the current element of the array.

Andre

 

[Andre Nathan]

Was this a bug that needs to be patched, or an intentional change from
Ruby 1.8 to Ruby 1.9? I thought it was the later.

The patch doesn't seem to change the new behaviour:

$ irb1.9
irb(main):001:0> [1,2,3].sort{|x,y| p [x,y]; 1}
[1, 2]
[2, 3]
=> [3, 2, 1]
irb(main):002:0> [1,2,3].sort{|x| p [x]; 1}
[1]
[2]
=> [3, 2, 1]
irb(main):003:0> [1,2,3].sort{|*x| p [x]; 1}
[[1, 2]]
[[2, 3]]
=> [3, 2, 1]

Andre

 

[Andre Nathan]

Hi

How about the following patch to ruby svn trunk?

--- vm_insnhelper.c (revision 14790)
+++ vm_insnhelper.c (working copy)
@@ -652,9 +652,12 @@
else if (argc == 0) {
arg = Qnil;
}
- else {
+ else if (argc == 1) {
arg = argv[0];
}
+ else {
+ arg = rb_ary_new4(argc, argv);
+ }

vm_push_frame(th, 0, FRAME_MAGIC_IFUNC,
self, (VALUE)block->dfp,

Is this patch going to be applied? Should I send a report to ruby-core?

Thanks,
Andre

 

[KUBO Takehiro]

Hi,

Is this patch going to be applied? Should I send a report to ruby-core?

No. I found a correct way.

1. add the following line to 'extconf.rb' before create_makefile.
have_type('rb_block_call_func', 'ruby.h')

2. fix sort_by_length as follows.

static VALUE
sort_by_length(VALUE i, VALUE dummy
#ifdef HAVE_TYPE_RB_BLOCK_CALL_FUNC
, int argc, VALUE *argv
#endif
)
{
#ifdef HAVE_TYPE_RB_BLOCK_CALL_FUNC
/* ruby 1.9 uses argc and argv. */
VALUE a0 = argv[0];
VALUE a1 = argv[1];
#else
/* ruby 1.8 uses i. */
VALUE a0 = RARRAY_PTR(i)[0];
VALUE a1 = RARRAY_PTR(i)[1];
#endif
Check_Type(a0, T_ARRAY);
Check_Type(a1, T_ARRAY);
return LONG2FIX(RARRAY_LEN(a0) - RARRAY_LEN(a1));
}

 

[Andre Nathan]

static VALUE
sort_by_length(VALUE i, VALUE dummy
#ifdef HAVE_TYPE_RB_BLOCK_CALL_FUNC
, int argc, VALUE *argv
#endif
)

That works, thanks! What is the meaning of the first two arguments in
the 1.9 case?

Andre

 

[Andre Nathan]

Well, if I'm right in your case i = argv[0] and dummy is the last argument
that you have given to rb_block_call()

I see. It's a bit confusing to have an argument that has the same value
of argv[0], but I guess with other functions there will be situations
where they'll not be the same.

Andre