Project

General

Profile

Actions

Feature #11256

closed

anonymous block forwarding

Added by bughit (bug hit) over 9 years ago. Updated almost 3 years ago.

Status:
Closed
Target version:
-
[ruby-core:69559]

Description

since capturing a block into a proc is slow: foo(&block)
and creating chains of blocks is kind of ugly and ultimately also inefficient: foo{yield}
why not allow block forwarding without capturing: foo(&) foo(1, 2, &)


Related issues 3 (0 open3 closed)

Related to Ruby master - Feature #3447: argument delegationClosedmatz (Yukihiro Matsumoto)Actions
Related to Ruby master - Feature #14045: Lazy Proc allocation for block parametersClosedko1 (Koichi Sasada)Actions
Related to Ruby master - Bug #18828: [Ripper] Anonymous parameter forwarding failures are not checkedClosedActions

Updated by bughit (bug hit) over 9 years ago

# takes a block
def bar
  # forwards it to foo without instantiating a proc
  foo(1, &)
end

Actions #2

Updated by matz (Yukihiro Matsumoto) over 9 years ago

Actions #3

Updated by mame (Yusuke Endoh) almost 7 years ago

  • Related to Feature #14045: Lazy Proc allocation for block parameters added

Updated by mame (Yusuke Endoh) almost 7 years ago

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

Lazy Proc allocation for block parameters (#14045) is implemented, so capturing a block into a proc is not slow.

One of the motivation now disappeared, but there is another motivation of this feature: the simplicity of the notation. Matz, is this syntax still hopeful to be accepted?

Updated by matz (Yukihiro Matsumoto) almost 7 years ago

  • Status changed from Assigned to Closed

As @mame (Yusuke Endoh) stated, we don't need it after we had lazy Proc allocation.

Matz.

Updated by ko1 (Koichi Sasada) almost 7 years ago

  • Status changed from Closed to Assigned

Matz:

but there is another motivation of this feature: the simplicity of the notation

With proposal syntax, we don't need to use a variable name for block parameter. What do you think about it?

Updated by nobu (Nobuyoshi Nakada) almost 7 years ago

Just to be clear, does it require both of the definition and the use, instead of only the latter?

I mean this is allowed:

def foo(&)
  bar(&)
end

but these are not:

def foo()
  bar(&)
end

def foo(&block)
  bar(&)
end

Updated by nobu (Nobuyoshi Nakada) almost 7 years ago

Current patch.

diff --git a/parse.y b/parse.y
index 1026d5c896..2a98016002 100644
--- a/parse.y
+++ b/parse.y
@@ -347,6 +347,8 @@ static int parser_yyerror(struct parser_params*, const char*);
 
 #define lambda_beginning_p() (lpar_beg && lpar_beg == paren_nest)
 
+#define ANON_BLOCK_ID '&'
+
 static enum yytokentype yylex(YYSTYPE*, YYLTYPE*, struct parser_params*);
 
 #ifndef RIPPER
@@ -2560,6 +2562,18 @@ block_arg	: tAMPER arg_value
 			$$ = $2;
 		    %*/
 		    }
+		| tAMPER
+		    {
+		    /*%%%*/
+			if (!local_id(ANON_BLOCK_ID)) {
+			    compile_error(PARSER_ARG "no anonymous block parameter");
+			}
+			$$ = NEW_BLOCK_PASS(new_lvar(ANON_BLOCK_ID, &@1));
+			$$->nd_loc = @$;
+		    /*%
+			$$ = Qnil;
+		    %*/
+		    }
 		;
 
 opt_block_arg	: ',' block_arg
@@ -4913,6 +4927,15 @@ f_block_arg	: blkarg_mark tIDENTIFIER
 			$$ = dispatch1(blockarg, $2);
 		    %*/
 		    }
+		| blkarg_mark
+		    {
+		    /*%%%*/
+			$$ = ANON_BLOCK_ID;
+			arg_var($$);
+		    /*%
+			$$ = dispatch1(blockarg, Qnil);
+		    %*/
+		    }
 		;
 
 opt_f_block_arg	: ',' f_block_arg

Updated by bughit (bug hit) almost 7 years ago

nobu (Nobuyoshi Nakada) wrote:

Just to be clear, does it require both of the definition and the use, instead of only the latter?

I mean this is allowed:

def foo(&)
  bar(&)
end

but these are not:

def foo()
  bar(&)
end

def foo(&block)
  bar(&)
end

def foo(&) is more self documenting so should be legal syntax, but should not be required because in general a method taking a block does not have to be marked.

Updated by mame (Yusuke Endoh) almost 7 years ago

  • Target version set to 2.6

According to ko1, Matz said that the details of the spec is not mature yet, so this ticket is postponed to 2.6.

A patch that allow def foo; bar(&); end:

diff --git a/compile.c b/compile.c
index 1b7158979a..79fde2f1a9 100644
--- a/compile.c
+++ b/compile.c
@@ -4341,8 +4341,13 @@ setup_args(rb_iseq_t *iseq, LINK_ANCHOR *const args, const NODE *argn,
     INIT_ANCHOR(arg_block);
     INIT_ANCHOR(args_splat);
     if (argn && nd_type(argn) == NODE_BLOCK_PASS) {
-	COMPILE(arg_block, "block", argn->nd_body);
-	*flag |= VM_CALL_ARGS_BLOCKARG;
+	if (argn->nd_body != NODE_SPECIAL_ANONYMOUS_BLOCK) {
+	    COMPILE(arg_block, "block", argn->nd_body);
+	    *flag |= VM_CALL_ARGS_BLOCKARG;
+	}
+	else {
+	    *flag |= VM_CALL_ARGS_BLOCKARG | VM_CALL_ARGS_BLOCKARG_THROUGH;
+	}
 	argn = argn->nd_head;
     }
 
diff --git a/insns.def b/insns.def
index 1c20573254..cf5702fea0 100644
--- a/insns.def
+++ b/insns.def
@@ -841,7 +841,7 @@ DEFINE_INSN
 send
 (CALL_INFO ci, CALL_CACHE cc, ISEQ blockiseq)
 (...)
-(VALUE val) // inc += - (int)(ci->orig_argc + ((ci->flag & VM_CALL_ARGS_BLOCKARG) ? 1 : 0));
+(VALUE val) // inc += - (int)(ci->orig_argc + (((ci->flag & (VM_CALL_ARGS_BLOCKARG | VM_CALL_ARGS_BLOCKARG_THROUGH)) == VM_CALL_ARGS_BLOCKARG) ? 1 : 0));
 {
     struct rb_calling_info calling;
 
@@ -924,7 +924,7 @@ DEFINE_INSN
 invokesuper
 (CALL_INFO ci, CALL_CACHE cc, ISEQ blockiseq)
 (...)
-(VALUE val) // inc += - (int)(ci->orig_argc + ((ci->flag & VM_CALL_ARGS_BLOCKARG) ? 1 : 0));
+(VALUE val) // inc += - (int)(ci->orig_argc + (((ci->flag & (VM_CALL_ARGS_BLOCKARG | VM_CALL_ARGS_BLOCKARG_THROUGH)) == VM_CALL_ARGS_BLOCKARG) ? 1 : 0));
 {
     struct rb_calling_info calling;
     calling.argc = ci->orig_argc;
diff --git a/iseq.c b/iseq.c
index 186f8622e7..b7b398c47e 100644
--- a/iseq.c
+++ b/iseq.c
@@ -1433,6 +1433,7 @@ rb_insn_operand_intern(const rb_iseq_t *iseq,
 		if (ci->flag & VM_CALL_ARGS_SPLAT) rb_ary_push(flags, rb_str_new2("ARGS_SPLAT"));
 		if (ci->flag & VM_CALL_ARGS_BLOCKARG) rb_ary_push(flags, rb_str_new2("ARGS_BLOCKARG"));
 		if (ci->flag & VM_CALL_ARGS_BLOCKARG_BLOCKPARAM) rb_ary_push(flags, rb_str_new2("ARGS_BLOCKARG_BLOCKPARAM"));
+		if (ci->flag & VM_CALL_ARGS_BLOCKARG_THROUGH) rb_ary_push(flags, rb_str_new2("ARGS_BLOCKARG_THROUGH"));
 		if (ci->flag & VM_CALL_FCALL) rb_ary_push(flags, rb_str_new2("FCALL"));
 		if (ci->flag & VM_CALL_VCALL) rb_ary_push(flags, rb_str_new2("VCALL"));
 		if (ci->flag & VM_CALL_ARGS_SIMPLE) rb_ary_push(flags, rb_str_new2("ARGS_SIMPLE"));
diff --git a/node.c b/node.c
index 5fa5e1fa50..f28653a3fc 100644
--- a/node.c
+++ b/node.c
@@ -773,7 +773,12 @@ dump_node(VALUE buf, VALUE indent, int comment, NODE *node)
 	ANN("example: foo(x, &blk)");
 	F_NODE(nd_head, "other arguments");
 	LAST_NODE;
-	F_NODE(nd_body, "block argument");
+	if (node->nd_body != NODE_SPECIAL_ANONYMOUS_BLOCK) {
+	    F_NODE(nd_body, "block argument");
+	}
+	else {
+	    F_MSG(nd_body, "block argument", "NODE_SPECIAL_ANONYMOUS_BLOCK");
+	}
 	return;
 
       case NODE_DEFN:
diff --git a/node.h b/node.h
index 6ff68e1800..88d8493770 100644
--- a/node.h
+++ b/node.h
@@ -458,6 +458,7 @@ typedef struct RNode {
 
 #define NODE_SPECIAL_REQUIRED_KEYWORD ((NODE *)-1)
 #define NODE_SPECIAL_NO_NAME_REST     ((NODE *)-1)
+#define NODE_SPECIAL_ANONYMOUS_BLOCK  ((NODE *)-1)
 
 RUBY_SYMBOL_EXPORT_BEGIN
 
diff --git a/parse.y b/parse.y
index 2eb1a0e1f8..a772d06f6d 100644
--- a/parse.y
+++ b/parse.y
@@ -2565,6 +2565,15 @@ block_arg	: tAMPER arg_value
 			$$ = $2;
 		    %*/
 		    }
+		| tAMPER
+		    {
+		    /*%%%*/
+			$$ = NEW_BLOCK_PASS(NODE_SPECIAL_ANONYMOUS_BLOCK);
+			$$->nd_loc = @$;
+		    /*%
+			$$ = dispatch0(anonymous_block_arg);
+		    %*/
+		    }
 		;
 
 opt_block_arg	: ',' block_arg
@@ -4924,6 +4933,13 @@ f_block_arg	: blkarg_mark tIDENTIFIER
 			$$ = dispatch1(blockarg, $2);
 		    %*/
 		    }
+		| blkarg_mark
+		    {
+		    /*%%%*/
+		    /*%
+			$$ = dispatch1(blockarg, Qnil);
+		    %*/
+		    }
 		;
 
 opt_f_block_arg	: ',' f_block_arg
diff --git a/vm_args.c b/vm_args.c
index 997b0b2f48..8a2eb055cb 100644
--- a/vm_args.c
+++ b/vm_args.c
@@ -839,6 +839,10 @@ vm_caller_setup_arg_block(const rb_execution_context_t *ec, rb_control_frame_t *
 	    !VM_ENV_FLAGS(VM_CF_LEP(reg_cfp), VM_FRAME_FLAG_MODIFIED_BLOCK_PARAM)) {
 	    calling->block_handler = VM_CF_BLOCK_HANDLER(reg_cfp);
 	}
+	else if (ci->flag & VM_CALL_ARGS_BLOCKARG_THROUGH) {
+	    ++reg_cfp->sp;
+	    calling->block_handler = GET_BLOCK_HANDLER();
+	}
 	else if (NIL_P(block_code)) {
 	    calling->block_handler = VM_BLOCK_HANDLER_NONE;
 	}
diff --git a/vm_core.h b/vm_core.h
index 374fcff1b2..e5f37ce48f 100644
--- a/vm_core.h
+++ b/vm_core.h
@@ -938,6 +938,7 @@ enum vm_call_flag_bits {
     VM_CALL_ARGS_SPLAT_bit,     /* m(*args) */
     VM_CALL_ARGS_BLOCKARG_bit,  /* m(&block) */
     VM_CALL_ARGS_BLOCKARG_BLOCKPARAM_bit,  /* m(&block) and block is a passed block parameter */
+    VM_CALL_ARGS_BLOCKARG_THROUGH_bit,     /* m(&) */
     VM_CALL_FCALL_bit,          /* m(...) */
     VM_CALL_VCALL_bit,          /* m */
     VM_CALL_ARGS_SIMPLE_bit,    /* (ci->flag & (SPLAT|BLOCKARG)) && blockiseq == NULL && ci->kw_arg == NULL */
@@ -953,6 +954,7 @@ enum vm_call_flag_bits {
 #define VM_CALL_ARGS_SPLAT      (0x01 << VM_CALL_ARGS_SPLAT_bit)
 #define VM_CALL_ARGS_BLOCKARG   (0x01 << VM_CALL_ARGS_BLOCKARG_bit)
 #define VM_CALL_ARGS_BLOCKARG_BLOCKPARAM (0x01 << VM_CALL_ARGS_BLOCKARG_BLOCKPARAM_bit)
+#define VM_CALL_ARGS_BLOCKARG_THROUGH    (0x01 << VM_CALL_ARGS_BLOCKARG_THROUGH_bit)
 #define VM_CALL_FCALL           (0x01 << VM_CALL_FCALL_bit)
 #define VM_CALL_VCALL           (0x01 << VM_CALL_VCALL_bit)
 #define VM_CALL_ARGS_SIMPLE     (0x01 << VM_CALL_ARGS_SIMPLE_bit)

Updated by Eregon (Benoit Daloze) almost 7 years ago

Just my opinion, but I think the shortcut syntax is going to cause more confusion than it would help.
& as argument without a & as a parameter looks very weird to me (it's like it in other languages, I would strongly advise against such magic).
I argue adding this makes Ruby less readable as a language.

This only saves one character in the definition, and it's not hard to name a block: &b or &block,
that's what the Ruby code out there uses and is clear because of it.
Moreover, it's inconsistent with arguments which must be named to be passed (except via zsuper):
foo(*) does not work as a call, and def meth(*) means ignore arguments, which is meaningless for blocks (just omitting &b is enough to ignore a block).

This also seems to conflict with one of the nicer proposals for short block notations like
enum.map(&.to_s.upcase.ljust(3)), a much more general and useful feature than this shortcut,
which only applies for a few methods forwarding the block and saves 2 character per forwarding method.

Also, the original poster started with "since capturing a block into a proc is slow: foo(&block)".
There is no such reason anymore, and I see no clear motivation of why such a special edge-case syntax is worth adding.

From the developer meeting notes, [Feature #11256] anonymous block forwarding:
Matz: it’s a good property to avoid naming variables. Also I personaly like it. However prompting such name-less programming might let people write cryptic codes.
Matz: hmm… Made up my mind. Accepted.

I agree it is cryptic and moreover argue it has little use, is inconsistent with other arguments and likely to cause syntax restrictions for other features.

@bughit (bug hit) Could you explain your motivation for this shortcut now that the performance is no longer a concern?
I agree foo{yield} was not nice, but is it worth to have foo(&) foo(1, 2, &) over foo(&b) foo(1, 2, &b) ?

@matz (Yukihiro Matsumoto) Could you detail your opinion and reconsider?

Actions #13

Updated by bughit (bug hit) almost 7 years ago

Eregon (Benoit Daloze) wrote:

@bughit (bug hit) Could you explain your motivation for this shortcut now that the performance is no longer a concern?
I agree foo{yield} was not nice, but is it worth to have foo(&) foo(1, 2, &) over foo(&b) foo(1, 2, &b) ?

ko1 convinced matz to accept this and he provided his reasons, simplicity of notation, not requiring an otherwise pointless variable. Which makes sense to me.

Also keep in mind that lazy proc allocation is an implementation detail. Conceptually when you declare a block params it still looks like you're doing unnecessary work of instantiating a proc object which you have no intention of using.

Every method has an invisible, nameless, optional block, and a naked & seems like an intuitive, logical way to forward it.

Actions #14

Updated by naruse (Yui NARUSE) almost 7 years ago

  • Target version deleted (2.6)

Updated by jeremyevans0 (Jeremy Evans) about 3 years ago

Looks like @matz (Yukihiro Matsumoto) approved this, but it was never committed. I've submitted a pull request for this that is based on the patches provided by @mame (Yusuke Endoh) and @nobu (Nobuyoshi Nakada): https://github.com/ruby/ruby/pull/5051

Updated by mame (Yusuke Endoh) almost 3 years ago

@matz (Yukihiro Matsumoto) accepted this, but said that the formal argument must explicitly receive a anonymous block parameter, i.e.

def foo(&) = bar(&) # OK
def foo = bar(&)    # NG

In the future, matz wants to make it more explicit whether a method accepts a block or not.

@jeremyevans0 (Jeremy Evans) Could you remove my diff from your PR?

Updated by jeremyevans0 (Jeremy Evans) almost 3 years ago

mame (Yusuke Endoh) wrote in #note-16:

@matz (Yukihiro Matsumoto) accepted this, but said that the formal argument must explicitly receive a anonymous block parameter, i.e.

def foo(&) = bar(&) # OK
def foo = bar(&)    # NG

In the future, matz wants to make it more explicit whether a method accepts a block or not.

@jeremyevans0 (Jeremy Evans) Could you remove my diff from your PR?

Sure, I updated the pull request. This made the pull request much simpler: https://github.com/ruby/ruby/pull/5051

Updated by bughit (bug hit) almost 3 years ago

mame (Yusuke Endoh) wrote in #note-16:

@matz (Yukihiro Matsumoto) accepted this, but said that the formal argument must explicitly receive a anonymous block parameter, i.e.

def foo(&) = bar(&) # OK
def foo = bar(&)    # NG

In the future, matz wants to make it more explicit whether a method accepts a block or not.

Where is the naked ampersand in def foo(&) described?

Don't see it here: https://docs.ruby-lang.org/en/master/doc/syntax/methods_rdoc.html

Does it mean the block is required?

What is the reason for the restriction?

The block may or may not be passed to def foo, so what is wrong with forwarding it or its absence with &? It reduces the utility of this feature, for what gain?

Updated by jeremyevans0 (Jeremy Evans) almost 3 years ago

bughit (bug hit) wrote in #note-18:

mame (Yusuke Endoh) wrote in #note-16:

@matz (Yukihiro Matsumoto) accepted this, but said that the formal argument must explicitly receive a anonymous block parameter, i.e.

def foo(&) = bar(&) # OK
def foo = bar(&)    # NG

In the future, matz wants to make it more explicit whether a method accepts a block or not.

Where is the naked ampersand in def foo(&) described?

Don't see it here: https://docs.ruby-lang.org/en/master/doc/syntax/methods_rdoc.html

This hasn't been committed yet, so you wouldn't see it on the website yet. The pull request already includes updates to the documentation: https://github.com/ruby/ruby/pull/5051/files#diff-d2caf8d6be0d2ccf90ff06a49bd1b4e4738588b4d5332954d57640e6fce0fd88

Does it mean the block is required?

No, just as &block doesn't mean the block is required.

What is the reason for the restriction?

The block may or may not be passed to def foo, so what is wrong with forwarding it or its absence with &? It reduces the utility of this feature, for what gain?

In the future, matz wants to make it more explicit whether a method accepts a block or not. I assume that means that we could see future changes in Ruby that better align with requiring & as a parameter, even if currently it seems reasonable that it would not be required.

In terms of gain currently, if & is required, the code to implement the feature is much simpler. Ruby is already very complex internally. If we can reduce the amount of complexity added, that is a good reason.

Actions #20

Updated by jeremyevans (Jeremy Evans) almost 3 years ago

  • Status changed from Assigned to Closed

Applied in changeset git|4adb012926f8bd6011168327d8832cf19976de40.


Anonymous block forwarding allows a method to forward a passed
block to another method without having to provide a name for the
block parameter.

Implements [Feature #11256]

Co-authored-by: Yusuke Endoh
Co-authored-by: Nobuyoshi Nakada

Updated by bughit (bug hit) almost 3 years ago

jeremyevans0 (Jeremy Evans) wrote in #note-19:

Does it mean the block is required?

No, just as &block doesn't mean the block is required.

&block serves a purpose in the code, binding the block to a named proc.

A naked & is just documentation then. It has not material effect. It makes no sense to force such a noop modifications of method signatures to be able to forward blocks anonymously. If the block can be present it should be forwardable.

What is the reason for the restriction?

The block may or may not be passed to def foo, so what is wrong with forwarding it or its absence with &? It reduces the utility of this feature, for what gain?

In the future, matz wants to make it more explicit whether a method accepts a block or not.

That only makes sense if def foo comes to mean the block is not allowed. It seems doubtful that will ever happen and until then anon forwarding should be possible without & in the signature.

If we can reduce the amount of complexity added, that is a good reason.

The result of this complexity reduction is an incoherent restriction that cannot be justified logically. Considering that the more complex proper implementation already exists, it's worth keeping it.

Updated by Eregon (Benoit Daloze) almost 3 years ago

bughit (bug hit) wrote in #note-21:

A naked & is just documentation then. It has not material effect. It makes no sense to force such a noop modifications of method signatures to be able to forward blocks anonymously. If the block can be present it should be forwardable.

It's extremely useful documentation/reading help that the method accepts a block.
If people have to scan for & in the whole method to know if a method accepts a block, it's really annoying (a yield keyword is much easier to spot).
I strongly agree with matz on this, methods taking a block should be clear about it, including in the parameters.

The result of this complexity reduction is an incoherent restriction that cannot be justified logically. Considering that the more complex proper implementation already exists, it's worth keeping it.

matz said no.
Please have some understanding that people implementing Ruby might know better about this and complexity.

Updated by bughit (bug hit) almost 3 years ago

I strongly agree with matz on this, methods taking a block should be clear about it, including in the parameters.

I never said I disagree. Requiring a naked & in the signature to signal "block allowed" and perhaps &! for block required would be better. But "block allowed" will break lots of code so I don't see it happening. So as long as "block allowed" is the default, forwarding should be possible. ... forwards blocks with 0 indication that the method takes a block, should it be illegal without a naked & in the signature?

Please have some understanding that people implementing Ruby might know better about this and complexity.

This is my feature request, so please don't tell me not to advocate for my vision.

Updated by mame (Yusuke Endoh) almost 3 years ago

bughit (bug hit) wrote in #note-21:

That only makes sense if def foo comes to mean the block is not allowed.

From what I understand, matz is for this. Passing a block to def foo might be prohibited in the future (I guess it is very far even if possible, though). If a method receives a block, it should have a &block parameter or ... argument forwarding. For the compatibility, a method whose body has yield or super keywords might be also allowed.

Actually @ko1 (Koichi Sasada) tried it in #15554. He withdrew the proposal because he have discovered a code pattern that makes false positives inevitable, but if matz accepts the incompatibility, the proposal might be a good first step.

Matz said he will create a ticket to discuss it, so please wait for him about the details.

Actions #25

Updated by nobu (Nobuyoshi Nakada) over 2 years ago

  • Related to Bug #18828: [Ripper] Anonymous parameter forwarding failures are not checked added
Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0