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

reflex · December 29, 2007, 6:50pm

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

reflex · December 29, 2007, 11:13pm

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.

reflex · December 29, 2007, 11:34pm

Andre N. wrote:

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}

reflex · December 30, 2007, 12:06am

On Sun, 2007-12-30 at 07:34 +0900, Siep K. wrote:

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

reflex · December 30, 2007, 1:31am

On Sun, 2007-12-30 at 08:06 +0900, Andre N. wrote:

On Sun, 2007-12-30 at 07:34 +0900, Siep K. wrote:

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

reflex · December 30, 2007, 3:01am

Hi,

On Dec 30, 2007 2:49 AM, Andre N. [email protected] wrote:

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.

reflex · December 30, 2007, 3:37am

On Sun, 2007-12-30 at 10:59 +0900, KUBO Takehiro wrote:

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

reflex · December 30, 2007, 4:11am

On Sun, 2007-12-30 at 11:36 +0900, Andre N. wrote:

On Sun, 2007-12-30 at 10:59 +0900, KUBO Takehiro wrote:

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…

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

reflex · December 30, 2007, 4:15pm

On Sun, 2007-12-30 at 13:15 +0900, KUBO Takehiro wrote:

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

reflex · December 30, 2007, 5:16am

On Dec 30, 2007 12:11 PM, Andre N. [email protected] wrote:

On Sun, 2007-12-30 at 11:36 +0900, Andre N. wrote:

On Sun, 2007-12-30 at 10:59 +0900, KUBO Takehiro wrote:

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

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.

reflex · December 31, 2007, 4:43am

On Sat, 29 Dec 2007 19:30:31 -0500, Andre N. wrote:

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

(from eigenclass.org’s summary)

[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

reflex · December 31, 2007, 2:19pm

Hi

On Mon, 2007-12-31 at 12:39 +0900, Ken B. wrote:

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

reflex · December 31, 2007, 2:50pm

On Mon, 2007-12-31 at 13:10 +0900, Ken B. wrote:

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

reflex · December 31, 2007, 5:10am

On Sat, 29 Dec 2007 23:15:19 -0500, KUBO Takehiro wrote:

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

reflex · January 2, 2008, 2:13pm

Hi

On Sun, 2007-12-30 at 13:15 +0900, KUBO Takehiro wrote:

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

reflex · January 2, 2008, 3:37pm

Hi,

On Jan 2, 2008 10:13 PM, Andre N. [email protected] wrote:

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

No. I found a correct way.

add the following line to ‘extconf.rb’ before create_makefile.
have_type(‘rb_block_call_func’, ‘ruby.h’)
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));
}

reflex · January 2, 2008, 5:42pm

“A” == Andre N. [email protected] writes:

sort_by_length(VALUE i, VALUE dummy

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

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()

Guy Decoux

reflex · January 2, 2008, 4:51pm

On Wed, 2008-01-02 at 23:36 +0900, KUBO Takehiro wrote:

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

reflex · January 2, 2008, 5:51pm

On Thu, 2008-01-03 at 01:41 +0900, ts wrote:

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

reflex · January 2, 2008, 5:56pm

“A” == Andre N. [email protected] writes:

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

Well, you have the case argc = 0 and it will be Qnil in this case
(argv[0]
don’t exist)

Guy Decoux