mirror of
https://github.com/ruby/ruby.git
synced 2022-11-09 12:17:21 -05:00
hash.c: Do not use the fast path (rb_yield_values) for lambda blocks
As a semantics, Hash#each yields a 2-element array (pairs of keys and
values). So, `{ a: 1 }.each(&->(k, v) { })` should raise an exception
due to lambda's arity check.
However, the optimization that avoids Array allocation by using
rb_yield_values for blocks whose arity is more than 1 (introduced at
b9d2960337
and some commits), seemed to
overlook the lambda case, and wrongly allowed the code above to work.
This change experimentally attempts to make it strict; now the code
above raises an ArgumentError. This is an incompatible change; if the
compatibility issue is bigger than our expectation, it may be reverted
(until Ruby 3.0 release).
[Bug #12706]
This commit is contained in:
parent
4be2a891cc
commit
47141797be
7 changed files with 81 additions and 14 deletions
7
NEWS.md
7
NEWS.md
|
@ -121,6 +121,13 @@ Excluding feature bug fixes.
|
|||
your plan to https://github.com/ruby/xmlrpc
|
||||
or https://github.com/ruby/net-telnet.
|
||||
|
||||
* EXPERIMENTAL: Hash#each consistently yields a 2-element array [[Bug #12706]]
|
||||
|
||||
* Now `{ a: 1 }.each(&->(k, v) { })` raises an ArgumentError
|
||||
due to lambda's arity check.
|
||||
* This is experimental; if it brings a big incompatibility issue,
|
||||
it may be reverted until 2.8/3.0 release.
|
||||
|
||||
## Stdlib compatibility issues
|
||||
|
||||
Excluding feature bug fixes.
|
||||
|
|
6
hash.c
6
hash.c
|
@ -3105,7 +3105,7 @@ static VALUE
|
|||
rb_hash_each_pair(VALUE hash)
|
||||
{
|
||||
RETURN_SIZED_ENUMERATOR(hash, 0, 0, hash_enum_size);
|
||||
if (rb_block_arity() > 1)
|
||||
if (rb_block_pair_yield_optimizable())
|
||||
rb_hash_foreach(hash, each_pair_i_fast, 0);
|
||||
else
|
||||
rb_hash_foreach(hash, each_pair_i, 0);
|
||||
|
@ -4446,7 +4446,7 @@ rb_hash_any_p(int argc, VALUE *argv, VALUE hash)
|
|||
/* yields pairs, never false */
|
||||
return Qtrue;
|
||||
}
|
||||
if (rb_block_arity() > 1)
|
||||
if (rb_block_pair_yield_optimizable())
|
||||
rb_hash_foreach(hash, any_p_i_fast, (VALUE)args);
|
||||
else
|
||||
rb_hash_foreach(hash, any_p_i, (VALUE)args);
|
||||
|
@ -5500,7 +5500,7 @@ env_each_pair(VALUE ehash)
|
|||
}
|
||||
FREE_ENVIRON(environ);
|
||||
|
||||
if (rb_block_arity() > 1) {
|
||||
if (rb_block_pair_yield_optimizable()) {
|
||||
for (i=0; i<RARRAY_LEN(ary); i+=2) {
|
||||
rb_yield_values(2, RARRAY_AREF(ary, i), RARRAY_AREF(ary, i+1));
|
||||
}
|
||||
|
|
|
@ -17,6 +17,7 @@ struct rb_iseq_struct; /* in vm_core.h */
|
|||
/* proc.c */
|
||||
VALUE rb_proc_location(VALUE self);
|
||||
st_index_t rb_hash_proc(st_index_t hash, VALUE proc);
|
||||
int rb_block_pair_yield_optimizable(void);
|
||||
int rb_block_arity(void);
|
||||
int rb_block_min_max_arity(int *max);
|
||||
VALUE rb_block_to_s(VALUE self, const struct rb_block *block, const char *additional_info);
|
||||
|
|
35
proc.c
35
proc.c
|
@ -1144,6 +1144,41 @@ block_setup(struct rb_block *block, VALUE block_handler)
|
|||
}
|
||||
}
|
||||
|
||||
int
|
||||
rb_block_pair_yield_optimizable(void)
|
||||
{
|
||||
int min, max;
|
||||
const rb_execution_context_t *ec = GET_EC();
|
||||
rb_control_frame_t *cfp = ec->cfp;
|
||||
VALUE block_handler = rb_vm_frame_block_handler(cfp);
|
||||
struct rb_block block;
|
||||
|
||||
if (block_handler == VM_BLOCK_HANDLER_NONE) {
|
||||
rb_raise(rb_eArgError, "no block given");
|
||||
}
|
||||
|
||||
block_setup(&block, block_handler);
|
||||
min = rb_vm_block_min_max_arity(&block, &max);
|
||||
|
||||
switch (vm_block_type(&block)) {
|
||||
case block_handler_type_symbol:
|
||||
return 0;
|
||||
|
||||
case block_handler_type_proc:
|
||||
{
|
||||
VALUE procval = block_handler;
|
||||
rb_proc_t *proc;
|
||||
GetProcPtr(procval, proc);
|
||||
if (proc->is_lambda) return 0;
|
||||
if (min != max) return 0;
|
||||
return min > 1;
|
||||
}
|
||||
|
||||
default:
|
||||
return min > 1;
|
||||
}
|
||||
}
|
||||
|
||||
int
|
||||
rb_block_arity(void)
|
||||
{
|
||||
|
|
|
@ -21,19 +21,37 @@ describe :hash_each, shared: true do
|
|||
ary.sort.should == ["a", "b", "c"]
|
||||
end
|
||||
|
||||
it "yields 2 values and not an Array of 2 elements when given a callable of arity 2" do
|
||||
obj = Object.new
|
||||
def obj.foo(key, value)
|
||||
ScratchPad << key << value
|
||||
ruby_version_is ""..."2.8" do
|
||||
it "yields 2 values and not an Array of 2 elements when given a callable of arity 2" do
|
||||
obj = Object.new
|
||||
def obj.foo(key, value)
|
||||
ScratchPad << key << value
|
||||
end
|
||||
|
||||
ScratchPad.record([])
|
||||
{ "a" => 1 }.send(@method, &obj.method(:foo))
|
||||
ScratchPad.recorded.should == ["a", 1]
|
||||
|
||||
ScratchPad.record([])
|
||||
{ "a" => 1 }.send(@method, &-> key, value { ScratchPad << key << value })
|
||||
ScratchPad.recorded.should == ["a", 1]
|
||||
end
|
||||
end
|
||||
|
||||
ScratchPad.record([])
|
||||
{ "a" => 1 }.send(@method, &obj.method(:foo))
|
||||
ScratchPad.recorded.should == ["a", 1]
|
||||
ruby_version_is "2.8" do
|
||||
it "yields an Array of 2 elements when given a callable of arity 2" do
|
||||
obj = Object.new
|
||||
def obj.foo(key, value)
|
||||
end
|
||||
|
||||
ScratchPad.record([])
|
||||
{ "a" => 1 }.send(@method, &-> key, value { ScratchPad << key << value })
|
||||
ScratchPad.recorded.should == ["a", 1]
|
||||
-> {
|
||||
{ "a" => 1 }.send(@method, &obj.method(:foo))
|
||||
}.should raise_error(ArgumentError)
|
||||
|
||||
-> {
|
||||
{ "a" => 1 }.send(@method, &-> key, value { })
|
||||
}.should raise_error(ArgumentError)
|
||||
end
|
||||
end
|
||||
|
||||
it "uses the same order as keys() and values()" do
|
||||
|
|
2
struct.c
2
struct.c
|
@ -821,7 +821,7 @@ rb_struct_each_pair(VALUE s)
|
|||
|
||||
RETURN_SIZED_ENUMERATOR(s, 0, 0, struct_enum_size);
|
||||
members = rb_struct_members(s);
|
||||
if (rb_block_arity() > 1) {
|
||||
if (rb_block_pair_yield_optimizable()) {
|
||||
for (i=0; i<RSTRUCT_LEN(s); i++) {
|
||||
VALUE key = rb_ary_entry(members, i);
|
||||
VALUE value = RSTRUCT_GET(s, i);
|
||||
|
|
|
@ -1841,4 +1841,10 @@ class TestHash < Test::Unit::TestCase
|
|||
h[obj2] = true
|
||||
assert_equal true, h[obj]
|
||||
end
|
||||
|
||||
def test_bug_12706
|
||||
assert_raise(ArgumentError) do
|
||||
{a: 1}.each(&->(k, v) {})
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
Loading…
Reference in a new issue