summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAlan Wu <XrXr@users.noreply.github.com>2019-07-13 12:04:01 -0400
committerAlan Wu <XrXr@users.noreply.github.com>2019-12-21 09:08:52 -0500
commit85a337f986fe6da99c7f8358f790f17b122b3903 (patch)
treef5c41137c2db802327cd4c405de7a922cbae7453
parentddb6023d64a8c96348b4e67603753e2916a04f28 (diff)
Kernel#lambda: return forwarded block as non-lambda proc
Before this commit, Kernel#lambda can't tell the difference between a directly passed literal block and one passed with an ampersand. A block passed with an ampersand is semantically speaking already a non-lambda proc. When Kernel#lambda receives a non-lambda proc, it should simply return it. Implementation wise, when the VM calls a method with a literal block, it places the code for the block on the calling control frame and passes a pointer (block handler) to the callee. Before this commit, the VM forwards block arguments by simply forwarding the block handler, which leaves the slot for block code unused when a control frame forwards its block argument. I use the vacant space to indicate that a frame has forwarded its block argument and inspect that in Kernel#lambda to detect forwarded blocks. This is a very ad-hoc solution and relies *heavily* on the way block passing works in the VM. However, it's the most self-contained solution I have. [Bug #15620]
Notes
Notes: Merged: https://github.com/ruby/ruby/pull/2289
-rw-r--r--proc.c10
-rw-r--r--test/ruby/test_lambda.rb20
-rw-r--r--vm_args.c5
-rw-r--r--vm_core.h8
4 files changed, 40 insertions, 3 deletions
diff --git a/proc.c b/proc.c
index 54b044f421..b4899d2f1f 100644
--- a/proc.c
+++ b/proc.c
@@ -792,8 +792,16 @@ proc_new(VALUE klass, int8_t is_lambda, int8_t kernel)
break;
case block_handler_type_ifunc:
- case block_handler_type_iseq:
return rb_vm_make_proc_lambda(ec, VM_BH_TO_CAPT_BLOCK(block_handler), klass, is_lambda);
+ case block_handler_type_iseq:
+ {
+ const struct rb_captured_block *captured = VM_BH_TO_CAPT_BLOCK(block_handler);
+ rb_control_frame_t *last_ruby_cfp = rb_vm_get_ruby_level_next_cfp(ec, cfp);
+ if (is_lambda && last_ruby_cfp && vm_cfp_forwarded_bh_p(last_ruby_cfp, block_handler)) {
+ is_lambda = false;
+ }
+ return rb_vm_make_proc_lambda(ec, captured, klass, is_lambda);
+ }
}
VM_UNREACHABLE(proc_new);
return Qnil;
diff --git a/test/ruby/test_lambda.rb b/test/ruby/test_lambda.rb
index b9412d4540..03b501a6c9 100644
--- a/test/ruby/test_lambda.rb
+++ b/test/ruby/test_lambda.rb
@@ -74,6 +74,26 @@ class TestLambdaParameters < Test::Unit::TestCase
assert_raise(ArgumentError, bug9605) {proc(&plus).call [1,2]}
end
+ def pass_along(&block)
+ lambda(&block)
+ end
+
+ def pass_along2(&block)
+ pass_along(&block)
+ end
+
+ def test_create_non_lambda_for_proc_one_level
+ f = pass_along {}
+ refute_predicate(f, :lambda?, '[Bug #15620]')
+ assert_nothing_raised(ArgumentError) { f.call(:extra_arg) }
+ end
+
+ def test_create_non_lambda_for_proc_two_levels
+ f = pass_along2 {}
+ refute_predicate(f, :lambda?, '[Bug #15620]')
+ assert_nothing_raised(ArgumentError) { f.call(:extra_arg) }
+ end
+
def test_instance_exec
bug12568 = '[ruby-core:76300] [Bug #12568]'
assert_nothing_raised(ArgumentError, bug12568) do
diff --git a/vm_args.c b/vm_args.c
index 47ab8ee66a..444abf9086 100644
--- a/vm_args.c
+++ b/vm_args.c
@@ -1204,7 +1204,10 @@ vm_caller_setup_arg_block(const rb_execution_context_t *ec, rb_control_frame_t *
return VM_BLOCK_HANDLER_NONE;
}
else if (block_code == rb_block_param_proxy) {
- return VM_CF_BLOCK_HANDLER(reg_cfp);
+ VM_ASSERT(!VM_CFP_IN_HEAP_P(GET_EC(), reg_cfp));
+ VALUE handler = VM_CF_BLOCK_HANDLER(reg_cfp);
+ reg_cfp->block_code = (const void *) handler;
+ return handler;
}
else if (SYMBOL_P(block_code) && rb_method_basic_definition_p(rb_cSymbol, idTo_proc)) {
const rb_cref_t *cref = vm_env_cref(reg_cfp->ep);
diff --git a/vm_core.h b/vm_core.h
index cd7bf74b5d..12c3ac3775 100644
--- a/vm_core.h
+++ b/vm_core.h
@@ -763,7 +763,7 @@ typedef struct rb_control_frame_struct {
const rb_iseq_t *iseq; /* cfp[2] */
VALUE self; /* cfp[3] / block[0] */
const VALUE *ep; /* cfp[4] / block[1] */
- const void *block_code; /* cfp[5] / block[2] */ /* iseq or ifunc */
+ const void *block_code; /* cfp[5] / block[2] */ /* iseq or ifunc or forwarded block handler */
VALUE *__bp__; /* cfp[6] */ /* outside vm_push_frame, use vm_base_ptr instead. */
#if VM_DEBUG_BP_CHECK
@@ -1494,6 +1494,12 @@ vm_block_handler_verify(MAYBE_UNUSED(VALUE block_handler))
(vm_block_handler_type(block_handler), 1));
}
+static inline int
+vm_cfp_forwarded_bh_p(const rb_control_frame_t *cfp, VALUE block_handler)
+{
+ return ((VALUE) cfp->block_code) == block_handler;
+}
+
static inline enum rb_block_type
vm_block_type(const struct rb_block *block)
{