Project

General

Profile

Actions

Bug #20388

closed

super + ruby2_keywords isn't working

Added by tenderlovemaking (Aaron Patterson) 8 months ago. Updated 8 months ago.


Description

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

Updated by tenderlovemaking (Aaron Patterson) 8 months ago

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)

Updated by jeremyevans0 (Jeremy Evans) 8 months ago

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.

Updated by jeremyevans0 (Jeremy Evans) 8 months ago

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);

Updated by jeremyevans0 (Jeremy Evans) 8 months ago

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

Actions #5

Updated by jeremyevans (Jeremy Evans) 8 months ago

  • 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]

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like1Like0