diff --git a/compile.c b/compile.c index 3be7808a8d..a0754306c9 100644 --- a/compile.c +++ b/compile.c @@ -6284,6 +6284,21 @@ 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) @@ -6303,7 +6318,12 @@ setup_args_core(rb_iseq_t *iseq, LINK_ANCHOR *const args, const NODE *argn, len -= 1; } else { - compile_hash(iseq, args, kwnode, TRUE, FALSE); + 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); + } } } @@ -6312,8 +6332,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)); - if (*dup_rest) *dup_rest = 0; + ADD_INSN1(args, argn, splatarray, RBOOL(*dup_rest & SPLATARRAY_TRUE)); + if (*dup_rest & SPLATARRAY_TRUE) *dup_rest &= ~SPLATARRAY_TRUE; if (flag_ptr) *flag_ptr |= VM_CALL_ARGS_SPLAT; RUBY_ASSERT(flag_ptr == NULL || (*flag_ptr & VM_CALL_KW_SPLAT) == 0); return 1; @@ -6335,8 +6355,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)); - if (*dup_rest) *dup_rest = 0; + ADD_INSN1(args, argn, splatarray, RBOOL(*dup_rest & SPLATARRAY_TRUE)); + if (*dup_rest & SPLATARRAY_TRUE) *dup_rest &= ~SPLATARRAY_TRUE; argc += 1; } else if (!args_pushed) { @@ -6378,8 +6398,14 @@ 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; } @@ -6437,7 +6463,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 = 1, initial_dup_rest; + unsigned int dup_rest = SPLATARRAY_TRUE, initial_dup_rest; if (argn) { const NODE *check_arg = nd_type_p(argn, NODE_BLOCK_PASS) ? @@ -6447,7 +6473,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 = 0; + dup_rest = SPLATARRAY_FALSE; break; case(NODE_ARGSCAT): // avoid caller side array allocation for f(1, *arg) @@ -6461,20 +6487,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) { + if (dup_rest == SPLATARRAY_FALSE) { // 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 = 1; + dup_rest = SPLATARRAY_TRUE; 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 = 1; + dup_rest = SPLATARRAY_TRUE; break; } @@ -6487,9 +6513,9 @@ setup_args(rb_iseq_t *iseq, LINK_ANCHOR *const args, const NODE *argn, } } - 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; + 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; } } initial_dup_rest = dup_rest; diff --git a/prism_compile.c b/prism_compile.c index 044ef5083d..435737cb66 100644 --- a/prism_compile.c +++ b/prism_compile.c @@ -1536,9 +1536,13 @@ 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, VALUE *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, int *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; @@ -1576,9 +1580,18 @@ 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 @@ -1684,8 +1697,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); - if (*dup_rest == Qtrue) *dup_rest = Qfalse; + PUSH_INSN1(ret, location, splatarray, (*dup_rest & SPLATARRAY_TRUE) ? Qtrue : Qfalse); + if (*dup_rest & SPLATARRAY_TRUE) *dup_rest &= ~SPLATARRAY_TRUE; } // If this is the first spalt array seen and it's the last // parameter, we don't want splatarray to dup it. @@ -1846,7 +1859,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) { - VALUE dup_rest = Qtrue; + int dup_rest = SPLATARRAY_TRUE; const pm_node_list_t *arguments; size_t arguments_size; @@ -1863,23 +1876,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 = Qfalse; + dup_rest = SPLATARRAY_FALSE; 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 == Qfalse && index < elements->size; index++) { + for (size_t index = 0; dup_rest == SPLATARRAY_FALSE && 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 = Qtrue; + if (pm_setup_args_dup_rest_p(assoc->key) || pm_setup_args_dup_rest_p(assoc->value)) dup_rest = SPLATARRAY_TRUE; 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 = Qtrue; + if (assoc->value != NULL && pm_setup_args_dup_rest_p(assoc->value)) dup_rest = SPLATARRAY_TRUE; break; } default: @@ -1888,7 +1901,7 @@ pm_setup_args(const pm_arguments_node_t *arguments_node, const pm_node_t *block, } } - VALUE initial_dup_rest = dup_rest; + int initial_dup_rest = dup_rest; int argc; if (block && PM_NODE_TYPE_P(block, PM_BLOCK_ARGUMENT_NODE)) { @@ -1896,6 +1909,12 @@ pm_setup_args(const pm_arguments_node_t *arguments_node, const pm_node_t *block, // since the nature of the expression influences whether splat should // duplicate the array. bool regular_block_arg = true; + const pm_node_t *block_expr = ((const pm_block_argument_node_t *)block)->expression; + + if (block_expr && pm_setup_args_dup_rest_p(block_expr)) { + dup_rest = SPLATARRAY_TRUE | DUP_SINGLE_KW_SPLAT; + initial_dup_rest = dup_rest; + } DECL_ANCHOR(block_arg); INIT_ANCHOR(block_arg); diff --git a/test/ruby/test_call.rb b/test/ruby/test_call.rb index b7d62ae111..ffbda1fdb9 100644 --- a/test/ruby/test_call.rb +++ b/test/ruby/test_call.rb @@ -374,6 +374,21 @@ 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 = []