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>
This commit is contained in:
Alan Wu 2023-09-07 14:56:07 -04:00
Родитель b90272b3b6
Коммит 39ee3e22bd
8 изменённых файлов: 83 добавлений и 141 удалений

53
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:
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;
}
if (block_handler == VM_BLOCK_HANDLER_NONE) {
// no block erorr raised else where
return;
}
rb_warn_deprecated("lambda without a literal block", "the proc without lambda");
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:
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_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();
}

Просмотреть файл

@ -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 "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)
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
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 "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 "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
lambda { l.call(:extra) }.should raise_error(ArgumentError)
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 "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

Просмотреть файл

@ -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

Просмотреть файл

@ -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

Просмотреть файл

@ -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)
def test_lambda_warning_pass_proc
assert_raise(ArgumentError) do
b = proc{}
lambda(&b)
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{}
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

Просмотреть файл

@ -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);

Просмотреть файл

@ -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)
{

Просмотреть файл

@ -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