From 0562c246ebdf071dfb85b2f7904574d38fee14f7 Mon Sep 17 00:00:00 2001 From: Ufuk Kayserilioglu Date: Thu, 7 Dec 2023 01:36:00 +0200 Subject: [PATCH] Add handling of implicit hash arguments Arguments that are passed as a hash need special consideration since in certain case they are not treated as keyword arguments. For example, if a call is passing `"a" => 1` as an argument, this will be turned into an implicit hash argument and not a keyword argument. The existing compiler checks to see if all hash nodes can be treated as keyword arguments. If they can, then it will treat them as keyword arguments. If not, then it will treat them as implicit hash arguments. This commit implements the same logic inside the Prism compiler. --- prism_compile.c | 51 +++++++++++++++++++++++++++------ test/ruby/test_compile_prism.rb | 10 +++++++ 2 files changed, 52 insertions(+), 9 deletions(-) diff --git a/prism_compile.c b/prism_compile.c index cc600b6ba683b3..3b0f020c14d4d9 100644 --- a/prism_compile.c +++ b/prism_compile.c @@ -993,16 +993,49 @@ pm_setup_args(pm_arguments_node_t *arguments_node, int *flags, struct rb_callinf break; } else { - *kw_arg = rb_xmalloc_mul_add(len, sizeof(VALUE), sizeof(struct rb_callinfo_kwarg)); - *flags = VM_CALL_KWARG; - (*kw_arg)->keyword_len = (int) len; + // We need to first figure out if all elements of the KeywordHashNode are AssocNodes + // with static literal keys. + // TODO: Figure this out from flags on the KeywordHashNode when Prism supports it + bool all_keys_static_literals = true; + + for (size_t i = 0; i < len; i++) { + pm_assoc_node_t *assoc = (pm_assoc_node_t *)keyword_arg->elements.nodes[i]; + pm_node_t *key = assoc->key; + + if (!key || !PM_NODE_TYPE_P(key, PM_ASSOC_NODE) || !pm_static_literal_p(key)) { + all_keys_static_literals = false; + break; + } + } - // TODO: Method callers like `foo(a => b)` - for (size_t i = 0; i < len; i++) { - pm_assoc_node_t *assoc = (pm_assoc_node_t *)keyword_arg->elements.nodes[i]; - (*kw_arg)->keywords[i] = pm_static_literal_value(assoc->key, scope_node, parser); - PM_COMPILE_NOT_POPPED(assoc->value); - } + if (all_keys_static_literals) { + // If they are all static literal keys then we can pass them as keyword arguments. + *kw_arg = rb_xmalloc_mul_add(len, sizeof(VALUE), sizeof(struct rb_callinfo_kwarg)); + *flags = VM_CALL_KWARG; + VALUE *keywords = (*kw_arg)->keywords; + (*kw_arg)->references = 0; + (*kw_arg)->keyword_len = (int)len; + + for (size_t i = 0; i < len; i++) { + pm_assoc_node_t *assoc = (pm_assoc_node_t *)keyword_arg->elements.nodes[i]; + pm_node_t *key = assoc->key; + keywords[i] = pm_static_literal_value(key, scope_node, parser); + PM_COMPILE_NOT_POPPED(assoc->value); + } + } else { + // If they aren't all static literal keys then we need to construct a new hash + // and pass that as an argument. + orig_argc++; + *flags |= VM_CALL_KW_SPLAT | VM_CALL_KW_SPLAT_MUT; + + for (size_t i = 0; i < len; i++) { + pm_assoc_node_t *assoc = (pm_assoc_node_t *)keyword_arg->elements.nodes[i]; + PM_COMPILE_NOT_POPPED(assoc->key); + PM_COMPILE_NOT_POPPED(assoc->value); + } + + ADD_INSN1(ret, &dummy_line_node, newhash, INT2FIX(len * 2)); + } } break; } diff --git a/test/ruby/test_compile_prism.rb b/test/ruby/test_compile_prism.rb index c2cb60251cd240..13dd866313e8cb 100644 --- a/test/ruby/test_compile_prism.rb +++ b/test/ruby/test_compile_prism.rb @@ -1329,6 +1329,16 @@ def self.foo(j) end foo(1) CODE + + assert_prism_eval(<<-CODE) + def self.prism_opt_var_trail_hash(a = nil, *b, c, **d); end + prism_opt_var_trail_hash("a") + prism_opt_var_trail_hash("a", c: 1) + prism_opt_var_trail_hash("a", "b") + prism_opt_var_trail_hash("a", "b", "c") + prism_opt_var_trail_hash("a", "b", "c", c: 1) + prism_opt_var_trail_hash("a", "b", "c", "c" => 0, c: 1) + CODE end def test_CallAndWriteNode