Revert "Avoid hash allocation for certain proc calls"

This reverts commit abc04e898b627ab37fa9dd5e330f239768778d8b.

This caused problems in a Rails test.

test/ruby/test_allocation.rb

@@ -2,16 +2,10 @@
 require 'test/unit'
 
 class TestAllocation < Test::Unit::TestCase
-  def munge_checks(checks)
-    checks
-  end
-
   def check_allocations(checks)
     dups = checks.split("\n").reject(&:empty?).tally.select{|_,v| v > 1}
     raise "duplicate checks:\n#{dups.keys.join("\n")}" unless dups.empty?
 
-    checks = munge_checks(checks)
-
     assert_separately([], <<~RUBY)
       $allocations = [0, 0]
       $counts = {}
@@ -555,8 +549,7 @@ class TestAllocation < Test::Unit::TestCase
 
     def test_nested_anonymous_splat_and_anonymous_keyword_splat_parameters
       check_allocations(<<~RUBY)
-        def self.t(*, **#{block}); end
+        def self.anon_splat_and_anon_keyword_splat(*, **#{block}); t(*, **) end; def self.t(*, **#{block}); end
-        def self.anon_splat_and_anon_keyword_splat(*, **#{block}); t(*, **) end
 
         check_allocations(1, 1, "anon_splat_and_anon_keyword_splat(1, a: 2#{block})")
         check_allocations(1, 0, "anon_splat_and_anon_keyword_splat(1, *empty_array, a: 2#{block})")
@@ -646,8 +639,7 @@ class TestAllocation < Test::Unit::TestCase
 
     def test_nested_argument_forwarding
       check_allocations(<<~RUBY)
-        def self.t(...) end
+        def self.argument_forwarding(...); t(...) end; def self.t(...) end
-        def self.argument_forwarding(...); t(...) end
 
         check_allocations(0, 0, "argument_forwarding(1, a: 2#{block})")
         check_allocations(0, 0, "argument_forwarding(1, *empty_array, a: 2#{block})")
@@ -774,69 +766,4 @@ class TestAllocation < Test::Unit::TestCase
       end
     end
   end
-
-  class ProcCall < MethodCall
-    def munge_checks(checks)
-      return checks if @no_munge
-      sub = rep = nil
-      checks.split("\n").map do |line|
-        case line
-        when "singleton_class.send(:ruby2_keywords, :r2k)"
-          "r2k.ruby2_keywords"
-        when /\Adef self.([a-z0-9_]+)\((.*)\);(.*)end\z/
-          sub = $1 + '('
-          rep = $1 + '.('
-          "#{$1} = #{$1} = proc{ |#{$2}| #{$3} }"
-        when /check_allocations/
-          line.gsub(sub, rep)
-        else
-          line
-        end
-      end.join("\n")
-    end
-
-    # Generic argument forwarding not supported in proc definitions
-    undef_method :test_argument_forwarding
-    undef_method :test_nested_argument_forwarding
-
-    # Proc anonymous arguments cannot be used directly
-    undef_method :test_nested_anonymous_splat_and_anonymous_keyword_splat_parameters
-
-    def test_no_array_allocation_with_splat_and_nonstatic_keywords
-      @no_munge = true
-
-      check_allocations(<<~RUBY)
-        keyword = keyword = proc{ |a: nil, b: nil #{block}| }
-
-        check_allocations(0, 1, "keyword.(*empty_array, a: empty_array#{block})") # LVAR
-        check_allocations(0, 1, "->{keyword.(*empty_array, a: empty_array#{block})}.call") # DVAR
-        check_allocations(0, 1, "$x = empty_array;  keyword.(*empty_array, a: $x#{block})") # GVAR
-        check_allocations(0, 1, "@x = empty_array; keyword.(*empty_array, a: @x#{block})") # IVAR
-        check_allocations(0, 1, "self.class.const_set(:X, empty_array); keyword.(*empty_array, a: X#{block})") # CONST
-        check_allocations(0, 1, "keyword.(*empty_array, a: Object::X#{block})") # COLON2
-        check_allocations(0, 1, "keyword.(*empty_array, a: ::X#{block})") # COLON3
-        check_allocations(0, 1, "T = keyword; #{'B = block' unless block.empty?}; class Object; @@x = X; T.(*X, a: @@x#{', &B' unless block.empty?}) end") # CVAR
-        check_allocations(0, 1, "keyword.(*empty_array, a: empty_array, b: 1#{block})") # INTEGER
-        check_allocations(0, 1, "keyword.(*empty_array, a: empty_array, b: 1.0#{block})") # FLOAT
-        check_allocations(0, 1, "keyword.(*empty_array, a: empty_array, b: 1.0r#{block})") # RATIONAL
-        check_allocations(0, 1, "keyword.(*empty_array, a: empty_array, b: 1.0i#{block})") # IMAGINARY
-        check_allocations(0, 1, "keyword.(*empty_array, a: empty_array, b: 'a'#{block})") # STR
-        check_allocations(0, 1, "keyword.(*empty_array, a: empty_array, b: :b#{block})") # SYM
-        check_allocations(0, 1, "keyword.(*empty_array, a: empty_array, b: /a/#{block})") # REGX
-        check_allocations(0, 1, "keyword.(*empty_array, a: self#{block})") # SELF
-        check_allocations(0, 1, "keyword.(*empty_array, a: empty_array, b: nil#{block})") # NIL
-        check_allocations(0, 1, "keyword.(*empty_array, a: empty_array, b: true#{block})") # TRUE
-        check_allocations(0, 1, "keyword.(*empty_array, a: empty_array, b: false#{block})") # FALSE
-        check_allocations(0, 1, "keyword.(*empty_array, a: ->{}#{block})") # LAMBDA
-        check_allocations(0, 1, "keyword.(*empty_array, a: $1#{block})") # NTH_REF
-        check_allocations(0, 1, "keyword.(*empty_array, a: $`#{block})") # BACK_REF
-      RUBY
-    end
-
-    class WithBlock < self
-      def block
-        ', &block'
-      end
-    end
-  end
 end

vm_args.c

@@ -571,22 +571,6 @@ check_kwrestarg(VALUE keyword_hash, unsigned int *kw_flag)
     }
 }
 
-static void
-flatten_rest_args(rb_execution_context_t * const ec, struct args_info *args, VALUE * const locals, unsigned int *ci_flag)
-{
-    const VALUE *argv = RARRAY_CONST_PTR(args->rest);
-    int j, i=args->argc, rest_len = RARRAY_LENINT(args->rest)-1;
-    args->argc += rest_len;
-    if (rest_len) {
-        CHECK_VM_STACK_OVERFLOW(ec->cfp, rest_len+1);
-        for (i, j=0; rest_len > 0; rest_len--, i++, j++) {
-            locals[i] = argv[j];
-        }
-    }
-    args->rest = Qfalse;
-    *ci_flag &= ~VM_CALL_ARGS_SPLAT;
-}
-
 static int
 setup_parameters_complex(rb_execution_context_t * const ec, const rb_iseq_t * const iseq,
                          struct rb_calling_info *const calling,
@@ -743,24 +727,27 @@ setup_parameters_complex(rb_execution_context_t * const ec, const rb_iseq_t * co
                 args->rest_dupped = false;
 
                 if (ignore_keyword_hash_p(rest_last, iseq, &kw_flag, &converted_keyword_hash)) {
-                    if (ISEQ_BODY(iseq)->param.flags.has_rest) {
+                    if (ISEQ_BODY(iseq)->param.flags.has_rest || arg_setup_type != arg_setup_method) {
                         // Only duplicate/modify splat array if it will be used
                         arg_rest_dup(args);
                         rb_ary_pop(args->rest);
                     }
-                    else if (arg_setup_type == arg_setup_block && !ISEQ_BODY(iseq)->param.flags.has_kwrest) {
-                        // Avoid hash allocation for empty hashes
-                        // Copy rest elements except empty keyword hash directly to VM stack
-                        flatten_rest_args(ec, args, locals, &ci_flag);
-                        keyword_hash = Qnil;
-                        kw_flag = 0;
-                    }
                     given_argc--;
                 }
                 else if (!ISEQ_BODY(iseq)->param.flags.has_rest) {
                     // Avoid duping rest when not necessary
                     // Copy rest elements and converted keyword hash directly to VM stack
-                    flatten_rest_args(ec, args, locals, &ci_flag);
+                    const VALUE *argv = RARRAY_CONST_PTR(args->rest);
+                    int j, i=args->argc, rest_len = RARRAY_LENINT(args->rest)-1;
+                    args->argc += rest_len;
+                    if (rest_len) {
+                        CHECK_VM_STACK_OVERFLOW(ec->cfp, rest_len+1);
+                        for (i, j=0; rest_len > 0; rest_len--, i++, j++) {
+                            locals[i] = argv[j];
+                        }
+                    }
+                    args->rest = Qfalse;
+                    ci_flag &= ~VM_CALL_ARGS_SPLAT;
 
                     if (ISEQ_BODY(iseq)->param.flags.has_kw || ISEQ_BODY(iseq)->param.flags.has_kwrest) {
                         given_argc--;

vm_insnhelper.c

@@ -2739,11 +2739,9 @@ vm_caller_setup_keyword_hash(const struct rb_callinfo *ci, VALUE keyword_hash)
             keyword_hash = rb_hash_dup(rb_to_hash_type(keyword_hash));
         }
     }
-    else if (!IS_ARGS_KW_SPLAT_MUT(ci) && !RHASH_EMPTY_P(keyword_hash)) {
+    else if (!IS_ARGS_KW_SPLAT_MUT(ci)) {
         /* Convert a hash keyword splat to a new hash unless
          * a mutable keyword splat was passed.
-         * Skip allocating new hash for empty keyword splat, as empty
-         * keyword splat will be ignored by both callers.
          */
         keyword_hash = rb_hash_dup(keyword_hash);
     }