Bug #20388: super + ruby2_keywords isn't working

• Status: Closed
• Assignee: jeremyevans0 (Jeremy Evans)
• Target version: -
• Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN

[tenderlovemaking (Aaron Patterson)]

class A
  def process action, ...
  end
end

class B < A
  def process method_name, *args
    args.freeze
    super
  end
  ruby2_keywords(:process)
end

p B.new.process(:foo, bar: :baz)

The above code started breaking at revision 4f77d8d3

[tenderlovemaking (Aaron Patterson)]

Sorry, I should be more specific. The code sample returns nil before 4f77d8d3, and since 4f77d8d3 it will raise an exception (it tries to mutate the args array)

[jeremyevans0 (Jeremy Evans)]

tenderlovemaking (Aaron Patterson) wrote in #note-1:

Sorry, I should be more specific. The code sample returns nil before 4f77d8d3, and since 4f77d8d3 it will raise an exception (it tries to mutate the args array)

Thanks for the report. I'll work on fixing this in the compiler.

[jeremyevans0 (Jeremy Evans)]

jeremyevans0 (Jeremy Evans) wrote in #note-2:

tenderlovemaking (Aaron Patterson) wrote in #note-1:

Sorry, I should be more specific. The code sample returns nil before 4f77d8d3, and since 4f77d8d3 it will raise an exception (it tries to mutate the args array)

Thanks for the report. I'll work on fixing this in the compiler.

Actually, compiler is fine, this is due to the anonymous splat optimization. Fix appears simple enough, I'll submit a PR shortly:

diff --git a/vm_args.c b/vm_args.c
index aa800319df..9df175eaa9 100644
--- a/vm_args.c
+++ b/vm_args.c
@@ -687,6 +687,9 @@ setup_parameters_complex(rb_execution_context_t * const ec, const rb_iseq_t * co
                 rest_last = rb_hash_dup(rest_last);
                 kw_flag |= VM_CALL_KW_SPLAT | VM_CALL_KW_SPLAT_MUT;

+                // Unset rest_dupped set by anon_rest as we may need to modify splat in this case
+                args->rest_dupped = false;
+
                 if (ignore_keyword_hash_p(rest_last, iseq, &kw_flag, &converted_keyword_hash)) {
                     arg_rest_dup(args);
                     rb_ary_pop(args->rest);

[jeremyevans0 (Jeremy Evans)]

jeremyevans0 (Jeremy Evans) wrote in #note-3:

jeremyevans0 (Jeremy Evans) wrote in #note-2:

tenderlovemaking (Aaron Patterson) wrote in #note-1:

Sorry, I should be more specific. The code sample returns nil before 4f77d8d3, and since 4f77d8d3 it will raise an exception (it tries to mutate the args array)

Thanks for the report. I'll work on fixing this in the compiler.

Actually, compiler is fine, this is due to the anonymous splat optimization. Fix appears simple enough, I'll submit a PR shortly:

PR: https://github.com/ruby/ruby/pull/10338

• Status changed from Open to Closed

Applied in changeset git|2dbcc123f4f605b51a3698d38ccd53ba6ef482ac.

---

Do not apply anon_rest optimization when passed array uses keyword-flagged hash

The optimization sets args->rest_dupped to avoid allocating an array,
but this is not safe if the splat array ends in a keyword flagged
hash. Unset args->rest_dupped in this case.

Fixes [Bug #20388]