1berry/ruby
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Revert "Fix evaluation order issue in f(**h, &h.delete(key))"

Browse Source 
This reverts commit 07d3bf4832532ae7446c9a6924d79aed60a7a9a5.

No failures in the pull request CI, but there are now allocation
test failures.
This commit is contained in:
Jeremy Evans 2024-09-18 11:26:10 -07:00
parent 07d3bf4832
commit 9c12c39ed1
3 changed files with 25 additions and 84 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

54
compile.c
View File

@ -6284,21 +6284,6 @@ keyword_node_single_splat_p(NODE *kwnode)
RNODE_LIST(RNODE_LIST(node)->nd_next)->nd_next == NULL;
}
static void
compile_single_keyword_splat_mutable(rb_iseq_t *iseq, LINK_ANCHOR *const args, const NODE *argn,
NODE *kwnode, unsigned int *flag_ptr)
{
*flag_ptr |= VM_CALL_KW_SPLAT_MUT;
ADD_INSN1(args, argn, putspecialobject, INT2FIX(VM_SPECIAL_OBJECT_VMCORE));
ADD_INSN1(args, argn, newhash, INT2FIX(0));
compile_hash(iseq, args, kwnode, TRUE, FALSE);
ADD_SEND(args, argn, id_core_hash_merge_kwd, INT2FIX(2));
}
#define SPLATARRAY_FALSE 0
#define SPLATARRAY_TRUE 1
#define DUP_SINGLE_KW_SPLAT 2
static int
setup_args_core(rb_iseq_t *iseq, LINK_ANCHOR *const args, const NODE *argn,
unsigned int *dup_rest, unsigned int *flag_ptr, struct rb_callinfo_kwarg **kwarg_ptr)
@ -6318,12 +6303,7 @@ setup_args_core(rb_iseq_t *iseq, LINK_ANCHOR *const args, const NODE *argn,
len -= 1;
}
else {
if (keyword_node_single_splat_p(kwnode) && (*dup_rest & DUP_SINGLE_KW_SPLAT)) {
compile_single_keyword_splat_mutable(iseq, args, argn, kwnode, flag_ptr);
}
else {
compile_hash(iseq, args, kwnode, TRUE, FALSE);
}
compile_hash(iseq, args, kwnode, TRUE, FALSE);
}
}
@ -6332,8 +6312,8 @@ setup_args_core(rb_iseq_t *iseq, LINK_ANCHOR *const args, const NODE *argn,
case NODE_SPLAT: {
// f(*a)
NO_CHECK(COMPILE(args, "args (splat)", RNODE_SPLAT(argn)->nd_head));
ADD_INSN1(args, argn, splatarray, RBOOL(*dup_rest & SPLATARRAY_TRUE));
if (*dup_rest & SPLATARRAY_TRUE) *dup_rest &= ~SPLATARRAY_TRUE;
ADD_INSN1(args, argn, splatarray, RBOOL(*dup_rest));
if (*dup_rest) *dup_rest = 0;
if (flag_ptr) *flag_ptr |= VM_CALL_ARGS_SPLAT;
RUBY_ASSERT(flag_ptr == NULL || (*flag_ptr & VM_CALL_KW_SPLAT) == 0);
return 1;
@ -6355,8 +6335,8 @@ setup_args_core(rb_iseq_t *iseq, LINK_ANCHOR *const args, const NODE *argn,
}
if (nd_type_p(RNODE_ARGSCAT(argn)->nd_head, NODE_LIST)) {
ADD_INSN1(args, argn, splatarray, RBOOL(*dup_rest & SPLATARRAY_TRUE));
if (*dup_rest & SPLATARRAY_TRUE) *dup_rest &= ~SPLATARRAY_TRUE;
ADD_INSN1(args, argn, splatarray, RBOOL(*dup_rest));
if (*dup_rest) *dup_rest = 0;
argc += 1;
}
else if (!args_pushed) {
@ -6398,14 +6378,8 @@ setup_args_core(rb_iseq_t *iseq, LINK_ANCHOR *const args, const NODE *argn,
*flag_ptr |= VM_CALL_KW_SPLAT;
if (!keyword_node_single_splat_p(kwnode)) {
*flag_ptr |= VM_CALL_KW_SPLAT_MUT;
compile_hash(iseq, args, kwnode, TRUE, FALSE);
}
else if (*dup_rest & DUP_SINGLE_KW_SPLAT) {
compile_single_keyword_splat_mutable(iseq, args, argn, kwnode, flag_ptr);
}
else {
compile_hash(iseq, args, kwnode, TRUE, FALSE);
}
compile_hash(iseq, args, kwnode, TRUE, FALSE);
argc += 1;
}
@ -6463,7 +6437,7 @@ setup_args(rb_iseq_t *iseq, LINK_ANCHOR *const args, const NODE *argn,
unsigned int *flag, struct rb_callinfo_kwarg **keywords)
{
VALUE ret;
unsigned int dup_rest = SPLATARRAY_TRUE, initial_dup_rest;
unsigned int dup_rest = 1, initial_dup_rest;
if (argn) {
const NODE *check_arg = nd_type_p(argn, NODE_BLOCK_PASS) ?
@ -6473,7 +6447,7 @@ setup_args(rb_iseq_t *iseq, LINK_ANCHOR *const args, const NODE *argn,
switch(nd_type(check_arg)) {
case(NODE_SPLAT):
// avoid caller side array allocation for f(*arg)
dup_rest = SPLATARRAY_FALSE;
dup_rest = 0;
break;
case(NODE_ARGSCAT):
// avoid caller side array allocation for f(1, *arg)
@ -6487,20 +6461,20 @@ setup_args(rb_iseq_t *iseq, LINK_ANCHOR *const args, const NODE *argn,
nd_type_p(RNODE_ARGSPUSH(check_arg)->nd_body, NODE_HASH) &&
!RNODE_HASH(RNODE_ARGSPUSH(check_arg)->nd_body)->nd_brace);
if (dup_rest == SPLATARRAY_FALSE) {
if (!dup_rest) {
// require allocation for keyword key/value/splat that may modify splatted argument
NODE *node = RNODE_HASH(RNODE_ARGSPUSH(check_arg)->nd_body)->nd_head;
while (node) {
NODE *key_node = RNODE_LIST(node)->nd_head;
if (key_node && setup_args_dup_rest_p(key_node)) {
dup_rest = SPLATARRAY_TRUE;
dup_rest = 1;
break;
}
node = RNODE_LIST(node)->nd_next;
NODE *value_node = RNODE_LIST(node)->nd_head;
if (setup_args_dup_rest_p(value_node)) {
dup_rest = SPLATARRAY_TRUE;
dup_rest = 1;
break;
}
@ -6513,9 +6487,9 @@ setup_args(rb_iseq_t *iseq, LINK_ANCHOR *const args, const NODE *argn,
}
}
if (check_arg != argn && setup_args_dup_rest_p(RNODE_BLOCK_PASS(argn)->nd_body)) {
// for block pass that may modify splatted argument, dup rest and kwrest if given
dup_rest = SPLATARRAY_TRUE | DUP_SINGLE_KW_SPLAT;
if (!dup_rest && (check_arg != argn) && setup_args_dup_rest_p(RNODE_BLOCK_PASS(argn)->nd_body)) {
// require allocation for block pass that may modify splatted argument
dup_rest = 1;
}
}
initial_dup_rest = dup_rest;

40
prism_compile.c
View File

@ -1536,13 +1536,9 @@ pm_compile_hash_elements(rb_iseq_t *iseq, const pm_node_t *node, const pm_node_l
#undef FLUSH_CHUNK
}
#define SPLATARRAY_FALSE 0
#define SPLATARRAY_TRUE 1
#define DUP_SINGLE_KW_SPLAT 2
// This is details. Users should call pm_setup_args() instead.
static int
pm_setup_args_core(const pm_arguments_node_t *arguments_node, const pm_node_t *block, int *flags, const bool has_regular_blockarg, struct rb_callinfo_kwarg **kw_arg, int *dup_rest, rb_iseq_t *iseq, LINK_ANCHOR *const ret, pm_scope_node_t *scope_node, const pm_node_location_t *node_location)
pm_setup_args_core(const pm_arguments_node_t *arguments_node, const pm_node_t *block, int *flags, const bool has_regular_blockarg, struct rb_callinfo_kwarg **kw_arg, VALUE *dup_rest, rb_iseq_t *iseq, LINK_ANCHOR *const ret, pm_scope_node_t *scope_node, const pm_node_location_t *node_location)
{
const pm_node_location_t location = *node_location;
@ -1580,18 +1576,9 @@ pm_setup_args_core(const pm_arguments_node_t *arguments_node, const pm_node_t *b
// in this case, so mark the method as passing mutable
// keyword splat.
*flags |= VM_CALL_KW_SPLAT_MUT;
pm_compile_hash_elements(iseq, argument, elements, true, ret, scope_node);
}
else if (*dup_rest & DUP_SINGLE_KW_SPLAT) {
*flags |= VM_CALL_KW_SPLAT_MUT;
PUSH_INSN1(ret, location, putspecialobject, INT2FIX(VM_SPECIAL_OBJECT_VMCORE));
PUSH_INSN1(ret, location, newhash, INT2FIX(0));
pm_compile_hash_elements(iseq, argument, elements, true, ret, scope_node);
PUSH_SEND(ret, location, id_core_hash_merge_kwd, INT2FIX(2));
}
else {
pm_compile_hash_elements(iseq, argument, elements, true, ret, scope_node);
}
pm_compile_hash_elements(iseq, argument, elements, true, ret, scope_node);
}
else {
// We need to first figure out if all elements of the
@ -1697,8 +1684,8 @@ pm_setup_args_core(const pm_arguments_node_t *arguments_node, const pm_node_t *b
// foo(a, *b, c)
// ^^
if (index + 1 < arguments->size || has_regular_blockarg) {
PUSH_INSN1(ret, location, splatarray, (*dup_rest & SPLATARRAY_TRUE) ? Qtrue : Qfalse);
if (*dup_rest & SPLATARRAY_TRUE) *dup_rest &= ~SPLATARRAY_TRUE;
PUSH_INSN1(ret, location, splatarray, *dup_rest);
if (*dup_rest == Qtrue) *dup_rest = Qfalse;
}
// If this is the first spalt array seen and it's the last
// parameter, we don't want splatarray to dup it.
@ -1859,7 +1846,7 @@ pm_setup_args_dup_rest_p(const pm_node_t *node)
static int
pm_setup_args(const pm_arguments_node_t *arguments_node, const pm_node_t *block, int *flags, struct rb_callinfo_kwarg **kw_arg, rb_iseq_t *iseq, LINK_ANCHOR *const ret, pm_scope_node_t *scope_node, const pm_node_location_t *node_location)
{
int dup_rest = SPLATARRAY_TRUE;
VALUE dup_rest = Qtrue;
const pm_node_list_t *arguments;
size_t arguments_size;
@ -1876,23 +1863,23 @@ pm_setup_args(const pm_arguments_node_t *arguments_node, const pm_node_t *block,
// Start by assuming that dup_rest=false, then check each element of the
// hash to ensure we don't need to flip it back to true (in case one of
// the elements could potentially mutate the array).
dup_rest = SPLATARRAY_FALSE;
dup_rest = Qfalse;
const pm_keyword_hash_node_t *keyword_hash = (const pm_keyword_hash_node_t *) arguments->nodes[arguments_size - 1];
const pm_node_list_t *elements = &keyword_hash->elements;
for (size_t index = 0; dup_rest == SPLATARRAY_FALSE && index < elements->size; index++) {
for (size_t index = 0; dup_rest == Qfalse && index < elements->size; index++) {
const pm_node_t *element = elements->nodes[index];
switch (PM_NODE_TYPE(element)) {
case PM_ASSOC_NODE: {
const pm_assoc_node_t *assoc = (const pm_assoc_node_t *) element;
if (pm_setup_args_dup_rest_p(assoc->key) || pm_setup_args_dup_rest_p(assoc->value)) dup_rest = SPLATARRAY_TRUE;
if (pm_setup_args_dup_rest_p(assoc->key) || pm_setup_args_dup_rest_p(assoc->value)) dup_rest = Qtrue;
break;
}
case PM_ASSOC_SPLAT_NODE: {
const pm_assoc_splat_node_t *assoc = (const pm_assoc_splat_node_t *) element;
if (assoc->value != NULL && pm_setup_args_dup_rest_p(assoc->value)) dup_rest = SPLATARRAY_TRUE;
if (assoc->value != NULL && pm_setup_args_dup_rest_p(assoc->value)) dup_rest = Qtrue;
break;
}
default:
@ -1901,7 +1888,7 @@ pm_setup_args(const pm_arguments_node_t *arguments_node, const pm_node_t *block,
}
}
int initial_dup_rest = dup_rest;
VALUE initial_dup_rest = dup_rest;
int argc;
if (block && PM_NODE_TYPE_P(block, PM_BLOCK_ARGUMENT_NODE)) {
@ -1910,11 +1897,6 @@ pm_setup_args(const pm_arguments_node_t *arguments_node, const pm_node_t *block,
// duplicate the array.
bool regular_block_arg = true;
if (pm_setup_args_dup_rest_p(block)) {
dup_rest = SPLATARRAY_TRUE | DUP_SINGLE_KW_SPLAT;
initial_dup_rest = dup_rest;
}
DECL_ANCHOR(block_arg);
INIT_ANCHOR(block_arg);
pm_compile_node(iseq, block, block_arg, false, scope_node);

15
test/ruby/test_call.rb
View File

@ -374,21 +374,6 @@ class TestCall < Test::Unit::TestCase
assert_equal({splat_modified: false}, b)
end
def test_kwsplat_block_eval_order
def self.t(**kw, &b) [kw, b] end
pr = ->{}
h = {a: pr}
a = []
ary = t(**h, &h.delete(:a))
assert_equal([{a: pr}, pr], ary)
h = {a: pr}
ary = t(*a, **h, &h.delete(:a))
assert_equal([{a: pr}, pr], ary)
end
def test_kwsplat_block_order
o = Object.new
ary = []