[前][次][番号順一覧][スレッド一覧]

ruby-changes:59355

From: Alan <ko1@a...>
Date: Sat, 21 Dec 2019 23:09:19 +0900 (JST)
Subject: [ruby-changes:59355] 85a337f986 (master): Kernel#lambda: return forwarded block as non-lambda proc

https://git.ruby-lang.org/ruby.git/commit/?id=85a337f986

From 85a337f986fe6da99c7f8358f790f17b122b3903 Mon Sep 17 00:00:00 2001
From: Alan Wu <XrXr@u...>
Date: Sat, 13 Jul 2019 12:04:01 -0400
Subject: 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]

diff --git a/proc.c b/proc.c
index 54b044f..b4899d2 100644
--- a/proc.c
+++ b/proc.c
@@ -792,8 +792,16 @@ proc_new(VALUE klass, int8_t is_lambda, int8_t kernel) https://github.com/ruby/ruby/blob/trunk/proc.c#L792
 	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 b9412d4..03b501a 100644
--- a/test/ruby/test_lambda.rb
+++ b/test/ruby/test_lambda.rb
@@ -74,6 +74,26 @@ class TestLambdaParameters < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_lambda.rb#L74
     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 47ab8ee..444abf9 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 * https://github.com/ruby/ruby/blob/trunk/vm_args.c#L1204
             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 cd7bf74..12c3ac3 100644
--- a/vm_core.h
+++ b/vm_core.h
@@ -763,7 +763,7 @@ typedef struct rb_control_frame_struct { https://github.com/ruby/ruby/blob/trunk/vm_core.h#L763
     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)) https://github.com/ruby/ruby/blob/trunk/vm_core.h#L1494
 	      (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)
 {
-- 
cgit v0.10.2


--
ML: ruby-changes@q...
Info: http://www.atdot.net/~ko1/quickml/

[前][次][番号順一覧][スレッド一覧]