From: Yukihiro Matsumoto <matz@...>
Date: 2008-01-08T14:06:14+09:00
Subject: Re: Enumerable#zip Needs Love

Hi,

In message "Re: Enumerable#zip Needs Love"
    on Tue, 8 Jan 2008 13:50:09 +0900, James Gray <james@grayproductions.net> writes:

|>  (a) stopping at the shortest

|* With the 1.8 system, we easily can have it both ways.  We can find  
|the bigger group and lead with that to save all data, or find the  
|shorter group and lead with it to trim data.
|* Under 1.9 we lose data by default and it's challenging to save it  
|since all groups must be expanded to the same length.

Points taken.

|>   (b) return value

|* An Array is a super set of Enumerable, so we gain more options  
|without losing anything.

Although an array is a Enumerable, and implements every methods in
Enumerable, I still hesitate to call it "super set", just because it
introduces several restriction of Enumerable, for example non infinity
length.  This is typical super set/sub set issue.

But using zip to create zipped array is certainly important use-case,
and consistency is not the primary goal of Ruby's design.

|* It seems to break a fair bit of 1.8 code.  I can name three  
|libraries affected by the change off the top of my head.
|* Performance seems like another good reason to do this, since you  
|said it would speed things up.

Points taken.

|> But use-case is more important than theoretical rationale.  There are
|> several options we can take:
|>
|>  (3) hardest way - (2) + honoring length of the receiver
|>
|>      I am not sure how to implement it yet.
|
|I don't understand.  We had this in 1.8.  Can we not go back to that  
|code?

The internal was totally re-written in 1.9 using fiber, but that's OK.
It's our matter.  In fact, I figured out how to do it while waiting
for your reply.  And actually implemented it already.  See attached
patch.

							matz.

diff --git a/enum.c b/enum.c
index 04fade3..bf18e27 100644
--- a/enum.c
+++ b/enum.c
@@ -1347,6 +1347,36 @@ enum_each_with_index(int argc, VALUE *argv, VALUE obj)
 
 
 static VALUE
+zip_a(VALUE val, NODE *memo, int argc, VALUE *argv)
+{
+    volatile VALUE result = memo->u1.value;
+    volatile VALUE args = memo->u2.value;
+    volatile VALUE tmp;
+    long idx = memo->u3.cnt++;
+    int i;
+
+    tmp = rb_ary_new2(RARRAY_LEN(args) + 1);
+    rb_ary_store(tmp, 0, val);
+    for (i=0; i<RARRAY_LEN(args); i++) {
+	rb_ary_push(tmp, rb_ary_entry(RARRAY_PTR(args)[i], idx));
+    }
+    rb_ary_push(result, tmp);
+    return Qnil;
+}
+
+static VALUE
+call_next(VALUE *v)
+{
+    return v[0] = rb_funcall(v[1], id_next, 0, 0);
+}
+
+static VALUE
+call_stop(VALUE *v)
+{
+    return v[0] = Qundef;
+}
+
+static VALUE
 zip_i(VALUE val, NODE *memo, int argc, VALUE *argv)
 {
     volatile VALUE result = memo->u1.value;
@@ -1357,8 +1387,20 @@ zip_i(VALUE val, NODE *memo, int argc, VALUE *argv)
     tmp = rb_ary_new2(RARRAY_LEN(args) + 1);
     rb_ary_store(tmp, 0, enum_values_pack(argc, argv));
     for (i=0; i<RARRAY_LEN(args); i++) {
-	VALUE v = rb_funcall(RARRAY_PTR(args)[i], id_next, 0, 0);
-	rb_ary_push(tmp, v);
+	if (NIL_P(RARRAY_PTR(args)[i])) {
+	    rb_ary_push(tmp, Qnil);
+	}
+	else {
+	    VALUE v[2];
+
+	    v[1] = RARRAY_PTR(args)[i];
+	    rb_rescue2(call_next, (VALUE)v, call_stop, (VALUE)v, rb_eStopIteration, 0);
+	    if (v[0] == Qundef) {
+		RARRAY_PTR(args)[i] = Qnil;
+		v[0] = Qnil;
+	    }
+	    rb_ary_push(tmp, v[0]);
+	}
     }
     if (NIL_P(result)) {
 	rb_yield(tmp);
@@ -1369,12 +1411,6 @@ zip_i(VALUE val, NODE *memo, int argc, VALUE *argv)
     return Qnil;
 }
 
-static VALUE
-zip_b(NODE *memo)
-{
-    return rb_block_call(memo->u3.value, id_each, 0, 0, zip_i, (VALUE)memo);
-}
-
 /*
  *  call-seq:
  *     enum.zip(arg, ...)                   => enumerator
@@ -1400,19 +1436,30 @@ zip_b(NODE *memo)
 static VALUE
 enum_zip(int argc, VALUE *argv, VALUE obj)
 {
-    int i;
-    VALUE result;
+    int i, block;
+    ID conv;
     NODE *memo;
 
-    for (i=0; i<argc; i++) {
-	argv[i] = rb_funcall(argv[i], rb_intern("to_enum"), 1, ID2SYM(id_each));
+    
+    if (!rb_block_given_p()) {
+	conv = rb_intern("to_a");
+	for (i=0; i<argc; i++) {
+	    argv[i] = rb_funcall(argv[i], conv, 0, 0);
+	}
+	memo = rb_node_newnode(NODE_MEMO, rb_ary_new(), rb_ary_new4(argc, argv), 0);
+	rb_block_call(obj, id_each, 0, 0, zip_a, (VALUE)memo);
+
+	return memo->u1.value;
     }
-    RETURN_ENUMERATOR(obj, argc, argv);
-    result = rb_block_given_p() ? Qnil : rb_ary_new();
-    memo = rb_node_newnode(NODE_MEMO, result, rb_ary_new4(argc, argv), obj);
-    rb_rescue2(zip_b, (VALUE)memo, 0, 0, rb_eStopIteration, (VALUE)0);
+    else {
+	conv = rb_intern("to_enum");
+	for (i=0; i<argc; i++) {
+	    argv[i] = rb_funcall(argv[i], conv, 1, ID2SYM(id_each));
+	}
+	memo = rb_node_newnode(NODE_MEMO, Qnil, rb_ary_new4(argc, argv), 0);
 
-    return result;
+	return rb_block_call(obj, id_each, 0, 0, zip_i, (VALUE)memo);
+    }
 }
 
 static VALUE
diff --git a/enumerator.c b/enumerator.c
index 0474469..d9179db 100644
--- a/enumerator.c
+++ b/enumerator.c
@@ -365,9 +365,9 @@ enumerator_with_index(VALUE obj)
 }
 
 static VALUE
-next_ii(VALUE i, VALUE obj)
+next_ii(VALUE i, VALUE obj, int argc, VALUE *argv)
 {
-    rb_fiber_yield(1, &i);
+    rb_fiber_yield(argc, argv);
     return Qnil;
 }