aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAlan Wu <XrXr@users.noreply.github.com>2023-09-07 14:56:07 -0400
committerAlan Wu <XrXr@users.noreply.github.com>2023-09-12 11:25:07 -0400
commit39ee3e22bd3d071c1c283b6b8dbd1af413342fb1 (patch)
treeda6a4f4c93d1887d477776b7f4575d5146f702e4
parentb90272b3b661bd2fd557332028b8feb568b6b9df (diff)
downloadruby-39ee3e22bd3d071c1c283b6b8dbd1af413342fb1.tar.gz
Make Kernel#lambda raise when given non-literal block
Previously, Kernel#lambda returned a non-lambda proc when given a non-literal block and issued a warning under the `:deprecated` category. With this change, Kernel#lambda will always return a lambda proc, if it returns without raising. Due to interactions with block passing optimizations, we previously had two separate code paths for detecting whether Kernel#lambda got a literal block. This change allows us to remove one path, the hack done with rb_control_frame_t::block_code introduced in 85a337f for supporting situations where Kernel#lambda returned a non-lambda proc. [Feature #19777] Co-authored-by: Takashi Kokubun <takashikkbn@gmail.com>
-rw-r--r--proc.c49
-rw-r--r--spec/ruby/core/kernel/lambda_spec.rb72
-rw-r--r--spec/ruby/core/proc/lambda_spec.rb8
-rw-r--r--test/ruby/test_lambda.rb26
-rw-r--r--test/ruby/test_proc.rb47
-rw-r--r--vm_args.c5
-rw-r--r--vm_core.h6
-rw-r--r--yjit/src/codegen.rs3
8 files changed, 79 insertions, 137 deletions
diff --git a/proc.c b/proc.c
index 9fd24a8fd4..0f4735b589 100644
--- a/proc.c
+++ b/proc.c
@@ -793,16 +793,8 @@ proc_new(VALUE klass, int8_t is_lambda)
break;
case block_handler_type_ifunc:
- 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);
- }
+ return rb_vm_make_proc_lambda(ec, VM_BH_TO_CAPT_BLOCK(block_handler), klass, is_lambda);
}
VM_UNREACHABLE(proc_new);
return Qnil;
@@ -857,31 +849,34 @@ rb_block_lambda(void)
}
static void
-f_lambda_warn(void)
+f_lambda_filter_non_literal(void)
{
rb_control_frame_t *cfp = GET_EC()->cfp;
VALUE block_handler = rb_vm_frame_block_handler(cfp);
- if (block_handler != VM_BLOCK_HANDLER_NONE) {
- switch (vm_block_handler_type(block_handler)) {
- case block_handler_type_iseq:
- if (RUBY_VM_PREVIOUS_CONTROL_FRAME(cfp)->ep == VM_BH_TO_ISEQ_BLOCK(block_handler)->ep) {
- return;
- }
- break;
- case block_handler_type_symbol:
+ if (block_handler == VM_BLOCK_HANDLER_NONE) {
+ // no block erorr raised else where
+ return;
+ }
+
+ switch (vm_block_handler_type(block_handler)) {
+ case block_handler_type_iseq:
+ if (RUBY_VM_PREVIOUS_CONTROL_FRAME(cfp)->ep == VM_BH_TO_ISEQ_BLOCK(block_handler)->ep) {
return;
- case block_handler_type_proc:
- if (rb_proc_lambda_p(VM_BH_TO_PROC(block_handler))) {
- return;
- }
- break;
- case block_handler_type_ifunc:
- break;
}
+ break;
+ case block_handler_type_symbol:
+ return;
+ case block_handler_type_proc:
+ if (rb_proc_lambda_p(VM_BH_TO_PROC(block_handler))) {
+ return;
+ }
+ break;
+ case block_handler_type_ifunc:
+ break;
}
- rb_warn_deprecated("lambda without a literal block", "the proc without lambda");
+ rb_raise(rb_eArgError, "the lambda method requires a literal block");
}
/*
@@ -895,7 +890,7 @@ f_lambda_warn(void)
static VALUE
f_lambda(VALUE _)
{
- f_lambda_warn();
+ f_lambda_filter_non_literal();
return rb_block_lambda();
}
diff --git a/spec/ruby/core/kernel/lambda_spec.rb b/spec/ruby/core/kernel/lambda_spec.rb
index 2f7abc749c..565536ac0d 100644
--- a/spec/ruby/core/kernel/lambda_spec.rb
+++ b/spec/ruby/core/kernel/lambda_spec.rb
@@ -26,42 +26,44 @@ describe "Kernel.lambda" do
l.lambda?.should be_true
end
- it "creates a lambda-style Proc if given a literal block via Kernel.public_send" do
- suppress_warning do
- l = Kernel.public_send(:lambda) { 42 }
- l.lambda?.should be_true
+ ruby_version_is ""..."3.3" do
+ it "creates a lambda-style Proc if given a literal block via Kernel.public_send" do
+ suppress_warning do
+ l = Kernel.public_send(:lambda) { 42 }
+ l.lambda?.should be_true
+ end
end
- end
- it "returns the passed Proc if given an existing Proc" do
- some_proc = proc {}
- l = suppress_warning {lambda(&some_proc)}
- l.should equal(some_proc)
- l.lambda?.should be_false
- end
+ it "returns the passed Proc if given an existing Proc" do
+ some_proc = proc {}
+ l = suppress_warning {lambda(&some_proc)}
+ l.should equal(some_proc)
+ l.lambda?.should be_false
+ end
- it "creates a lambda-style Proc when called with zsuper" do
- suppress_warning do
- l = KernelSpecs::LambdaSpecs::ForwardBlockWithZSuper.new.lambda { 42 }
- l.lambda?.should be_true
- l.call.should == 42
+ it "creates a lambda-style Proc when called with zsuper" do
+ suppress_warning do
+ l = KernelSpecs::LambdaSpecs::ForwardBlockWithZSuper.new.lambda { 42 }
+ l.lambda?.should be_true
+ l.call.should == 42
- lambda { l.call(:extra) }.should raise_error(ArgumentError)
+ lambda { l.call(:extra) }.should raise_error(ArgumentError)
+ end
end
- end
- it "returns the passed Proc if given an existing Proc through super" do
- some_proc = proc { }
- l = KernelSpecs::LambdaSpecs::SuperAmpersand.new.lambda(&some_proc)
- l.should equal(some_proc)
- l.lambda?.should be_false
- end
+ it "returns the passed Proc if given an existing Proc through super" do
+ some_proc = proc { }
+ l = KernelSpecs::LambdaSpecs::SuperAmpersand.new.lambda(&some_proc)
+ l.should equal(some_proc)
+ l.lambda?.should be_false
+ end
- it "does not create lambda-style Procs when captured with #method" do
- kernel_lambda = method(:lambda)
- l = suppress_warning {kernel_lambda.call { 42 }}
- l.lambda?.should be_false
- l.call(:extra).should == 42
+ it "does not create lambda-style Procs when captured with #method" do
+ kernel_lambda = method(:lambda)
+ l = suppress_warning {kernel_lambda.call { 42 }}
+ l.lambda?.should be_false
+ l.call(:extra).should == 42
+ end
end
it "checks the arity of the call when no args are specified" do
@@ -137,8 +139,16 @@ describe "Kernel.lambda" do
end
context "when called without a literal block" do
- it "warns when proc isn't a lambda" do
- -> { lambda(&proc{}) }.should complain("#{__FILE__}:#{__LINE__}: warning: lambda without a literal block is deprecated; use the proc without lambda instead\n")
+ ruby_version_is ""..."3.3" do
+ it "warns when proc isn't a lambda" do
+ -> { lambda(&proc{}) }.should complain("#{__FILE__}:#{__LINE__}: warning: lambda without a literal block is deprecated; use the proc without lambda instead\n")
+ end
+ end
+
+ ruby_version_is "3.3" do
+ it "raises when proc isn't a lambda" do
+ -> { lambda(&proc{}) }.should raise_error(ArgumentError, /the lambda method requires a literal block/)
+ end
end
it "doesn't warn when proc is lambda" do
diff --git a/spec/ruby/core/proc/lambda_spec.rb b/spec/ruby/core/proc/lambda_spec.rb
index b2d3f50350..5c3c38fc2a 100644
--- a/spec/ruby/core/proc/lambda_spec.rb
+++ b/spec/ruby/core/proc/lambda_spec.rb
@@ -14,9 +14,11 @@ describe "Proc#lambda?" do
Proc.new {}.lambda?.should be_false
end
- it "is preserved when passing a Proc with & to the lambda keyword" do
- suppress_warning {lambda(&->{})}.lambda?.should be_true
- suppress_warning {lambda(&proc{})}.lambda?.should be_false
+ ruby_version_is ""..."3.3" do
+ it "is preserved when passing a Proc with & to the lambda keyword" do
+ suppress_warning {lambda(&->{})}.lambda?.should be_true
+ suppress_warning {lambda(&proc{})}.lambda?.should be_false
+ end
end
it "is preserved when passing a Proc with & to the proc keyword" do
diff --git a/test/ruby/test_lambda.rb b/test/ruby/test_lambda.rb
index 9949fab8c7..7738034240 100644
--- a/test/ruby/test_lambda.rb
+++ b/test/ruby/test_lambda.rb
@@ -177,32 +177,6 @@ class TestLambdaParameters < Test::Unit::TestCase
RUBY
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
- prev_warning, Warning[:deprecated] = Warning[:deprecated], false
- f = pass_along {}
- refute_predicate(f, :lambda?, '[Bug #15620]')
- assert_nothing_raised(ArgumentError) { f.call(:extra_arg) }
- ensure
- Warning[:deprecated] = prev_warning
- end
-
- def test_create_non_lambda_for_proc_two_levels
- prev_warning, Warning[:deprecated] = Warning[:deprecated], false
- f = pass_along2 {}
- refute_predicate(f, :lambda?, '[Bug #15620]')
- assert_nothing_raised(ArgumentError) { f.call(:extra_arg) }
- ensure
- Warning[:deprecated] = prev_warning
- end
-
def test_instance_exec
bug12568 = '[ruby-core:76300] [Bug #12568]'
assert_nothing_raised(ArgumentError, bug12568) do
diff --git a/test/ruby/test_proc.rb b/test/ruby/test_proc.rb
index 2b3590d4d0..4245a7b785 100644
--- a/test/ruby/test_proc.rb
+++ b/test/ruby/test_proc.rb
@@ -289,7 +289,6 @@ class TestProc < Test::Unit::TestCase
assert_equal(false, l.lambda?)
assert_equal(false, l.curry.lambda?, '[ruby-core:24127]')
assert_equal(false, proc(&l).lambda?)
- assert_equal(false, assert_deprecated_warning {lambda(&l)}.lambda?)
assert_equal(false, Proc.new(&l).lambda?)
l = lambda {}
assert_equal(true, l.lambda?)
@@ -299,47 +298,21 @@ class TestProc < Test::Unit::TestCase
assert_equal(true, Proc.new(&l).lambda?)
end
- def self.helper_test_warn_lamda_with_passed_block &b
+ def helper_test_warn_lambda_with_passed_block &b
lambda(&b)
end
- def self.def_lambda_warning name, warn
- define_method(name, proc do
- prev = Warning[:deprecated]
- assert_warn warn do
- Warning[:deprecated] = true
- yield
- end
- ensure
- Warning[:deprecated] = prev
- end)
- end
-
- def_lambda_warning 'test_lambda_warning_normal', '' do
- lambda{}
- end
-
- def_lambda_warning 'test_lambda_warning_pass_lambda', '' do
- b = lambda{}
- lambda(&b)
- end
-
- def_lambda_warning 'test_lambda_warning_pass_symbol_proc', '' do
- lambda(&:to_s)
- end
-
- def_lambda_warning 'test_lambda_warning_pass_proc', /deprecated/ do
- b = proc{}
- lambda(&b)
- end
-
- def_lambda_warning 'test_lambda_warning_pass_block', /deprecated/ do
- helper_test_warn_lamda_with_passed_block{}
+ def test_lambda_warning_pass_proc
+ assert_raise(ArgumentError) do
+ b = proc{}
+ lambda(&b)
+ end
end
- def_lambda_warning 'test_lambda_warning_pass_block_symbol_proc', '' do
- # Symbol#to_proc returns lambda
- helper_test_warn_lamda_with_passed_block(&:to_s)
+ def test_lambda_warning_pass_block
+ assert_raise(ArgumentError) do
+ helper_test_warn_lambda_with_passed_block{}
+ end
end
def test_curry_ski_fib
diff --git a/vm_args.c b/vm_args.c
index a02e08d7fc..882966e432 100644
--- a/vm_args.c
+++ b/vm_args.c
@@ -885,10 +885,7 @@ 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) {
- 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;
+ return VM_CF_BLOCK_HANDLER(reg_cfp);
}
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 25629d3789..f3e5a04f72 100644
--- a/vm_core.h
+++ b/vm_core.h
@@ -1550,12 +1550,6 @@ 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)
{
diff --git a/yjit/src/codegen.rs b/yjit/src/codegen.rs
index 69b1c52c9c..3ae519bc81 100644
--- a/yjit/src/codegen.rs
+++ b/yjit/src/codegen.rs
@@ -5194,9 +5194,6 @@ fn gen_push_frame(
let block_handler = asm.load(
Opnd::mem(64, ep_opnd, SIZEOF_VALUE_I32 * VM_ENV_DATA_INDEX_SPECVAL)
);
-
- asm.store(Opnd::mem(64, CFP, RUBY_OFFSET_CFP_BLOCK_CODE), block_handler);
-
block_handler
}
BlockHandler::AlreadySet => 0.into(), // unused