ruby2_keywords Hashes are now always correctly copied without the flag on CRuby 3.2+

eregon · eregon · commit eb10c5e08af0 · 2023-09-20T13:47:06.000+02:00
* Reverts "Copy the CRuby bug/behavior that the ruby2_keyword flag on a Hash is not reset for caller(*args) -> callee(*args) calls" * This reverts commit 103d4da. * See 103d4da5fc * 3363 -> 3242 for org.truffleruby.language.dispatch.RubyCallNode.execute (NI EE) * Avoids partial evaluating and allocating the nodes to copy the ruby2_keyword Hash per callee, instead it is only once per call site.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -12,6 +12,7 @@ Compatibility:
 
 Performance:
 
+* Optimize calls with `ruby2_keywords` forwarding by deciding it per call site instead of per callee thanks to [my fix in CRuby 3.2](https://bugs.ruby-lang.org/issues/18625) (@eregon).
 
 Changes:
 
diff --git a/spec/tags/core/module/ruby2_keywords_tags.txt b/spec/tags/core/module/ruby2_keywords_tags.txt
diff --git a/spec/tags/language/keyword_arguments_tags.txt b/spec/tags/language/keyword_arguments_tags.txt
@@ -1,2 +1 @@
 fails:Keyword arguments raises ArgumentError for missing keyword arguments even if there are extra ones
-fails:Keyword arguments delegation does not work with call(*ruby2_keyword_args) with missing ruby2_keywords in between
diff --git a/spec/truffle/parsing/fixtures/complex_numbers/with_float_imaginary_part.yaml b/spec/truffle/parsing/fixtures/complex_numbers/with_float_imaginary_part.yaml
@@ -20,7 +20,6 @@ ast: |
           methodName = "convert"
           notEmptyKeywordsProfile = false
           notRuby2KeywordsHashProfile = false
-          ruby2KeywordsHashProfile = false
       children:
           arguments = [
               IntegerFixnumLiteralNode
diff --git a/spec/truffle/parsing/fixtures/complex_numbers/with_integer_imaginary_part.yaml b/spec/truffle/parsing/fixtures/complex_numbers/with_integer_imaginary_part.yaml
@@ -20,7 +20,6 @@ ast: |
           methodName = "convert"
           notEmptyKeywordsProfile = false
           notRuby2KeywordsHashProfile = false
-          ruby2KeywordsHashProfile = false
       children:
           arguments = [
               IntegerFixnumLiteralNode
diff --git a/spec/truffle/parsing/fixtures/ensure/in_begin_end_block.yaml b/spec/truffle/parsing/fixtures/ensure/in_begin_end_block.yaml
@@ -50,7 +50,6 @@ ast: |
                                   methodName = "bar"
                                   notEmptyKeywordsProfile = false
                                   notRuby2KeywordsHashProfile = false
-                                  ruby2KeywordsHashProfile = false
                               children:
                                   receiver =
                                       SelfNode
diff --git a/spec/truffle/parsing/fixtures/ensure/in_module.yaml b/spec/truffle/parsing/fixtures/ensure/in_module.yaml
@@ -87,7 +87,6 @@ ast: |
                                                                   methodName = "bar"
                                                                   notEmptyKeywordsProfile = false
                                                                   notRuby2KeywordsHashProfile = false
-                                                                  ruby2KeywordsHashProfile = false
                                                               children:
                                                                   receiver =
                                                                       SelfNode
diff --git a/spec/truffle/parsing/fixtures/hashes/with_double_splat_operator/with_double_splat.yaml b/spec/truffle/parsing/fixtures/hashes/with_double_splat_operator/with_double_splat.yaml
@@ -25,7 +25,6 @@ ast: |
                       methodName = "foo"
                       notEmptyKeywordsProfile = false
                       notRuby2KeywordsHashProfile = false
-                      ruby2KeywordsHashProfile = false
                   children:
                       receiver =
                           SelfNode
diff --git a/spec/truffle/parsing/fixtures/hashes/with_double_splat_operator/with_double_splat_and_key_value_pairs.yaml b/spec/truffle/parsing/fixtures/hashes/with_double_splat_operator/with_double_splat_and_key_value_pairs.yaml
@@ -32,7 +32,6 @@ ast: |
                                   methodName = "foo"
                                   notEmptyKeywordsProfile = false
                                   notRuby2KeywordsHashProfile = false
-                                  ruby2KeywordsHashProfile = false
                               children:
                                   receiver =
                                       SelfNode
diff --git a/spec/truffle/parsing/fixtures/hashes/with_double_splat_operator/with_key_value_pairs_and_double_splat_operator.yaml b/spec/truffle/parsing/fixtures/hashes/with_double_splat_operator/with_key_value_pairs_and_double_splat_operator.yaml
@@ -46,7 +46,6 @@ ast: |
                                   methodName = "bar"
                                   notEmptyKeywordsProfile = false
                                   notRuby2KeywordsHashProfile = false
-                                  ruby2KeywordsHashProfile = false
                               children:
                                   receiver =
                                       SelfNode
diff --git a/spec/truffle/parsing/fixtures/hashes/with_double_splat_operator/with_key_value_pairs_double_splat_operator_and_key_value_pairs.yaml b/spec/truffle/parsing/fixtures/hashes/with_double_splat_operator/with_key_value_pairs_double_splat_operator_and_key_value_pairs.yaml
@@ -47,7 +47,6 @@ ast: |
                                   methodName = "bar"
                                   notEmptyKeywordsProfile = false
                                   notRuby2KeywordsHashProfile = false
-                                  ruby2KeywordsHashProfile = false
                               children:
                                   receiver =
                                       SelfNode
diff --git a/spec/truffle/parsing/fixtures/rescue/backtrace_optimization/disabled/when_method_call.yaml b/spec/truffle/parsing/fixtures/rescue/backtrace_optimization/disabled/when_method_call.yaml
@@ -38,7 +38,6 @@ ast: |
                                   methodName = "bar"
                                   notEmptyKeywordsProfile = false
                                   notRuby2KeywordsHashProfile = false
-                                  ruby2KeywordsHashProfile = false
                               children:
                                   receiver =
                                       SelfNode
diff --git a/spec/truffle/parsing/fixtures/rescue/backtrace_optimization/enabled/when_complex_literal.yaml b/spec/truffle/parsing/fixtures/rescue/backtrace_optimization/enabled/when_complex_literal.yaml
@@ -38,7 +38,6 @@ ast: |
                                   methodName = "convert"
                                   notEmptyKeywordsProfile = false
                                   notRuby2KeywordsHashProfile = false
-                                  ruby2KeywordsHashProfile = false
                               children:
                                   arguments = [
                                       IntegerFixnumLiteralNode
diff --git a/spec/truffle/parsing/fixtures/rescue/backtrace_optimization/enabled/when_rational_literal.yaml b/spec/truffle/parsing/fixtures/rescue/backtrace_optimization/enabled/when_rational_literal.yaml
@@ -38,7 +38,6 @@ ast: |
                                   methodName = "convert"
                                   notEmptyKeywordsProfile = false
                                   notRuby2KeywordsHashProfile = false
-                                  ruby2KeywordsHashProfile = false
                               children:
                                   arguments = [
                                       IntegerFixnumLiteralNode
diff --git a/spec/truffle/parsing/fixtures/rescue/capturing/with_attribute.yaml b/spec/truffle/parsing/fixtures/rescue/capturing/with_attribute.yaml
@@ -74,7 +74,6 @@ ast: |
                                                           methodName = "foo="
                                                           notEmptyKeywordsProfile = false
                                                           notRuby2KeywordsHashProfile = false
-                                                          ruby2KeywordsHashProfile = false
                                                       children:
                                                           arguments = [
                                                               DeadNode
@@ -97,7 +96,6 @@ ast: |
                                                                       methodName = "a"
                                                                       notEmptyKeywordsProfile = false
                                                                       notRuby2KeywordsHashProfile = false
-                                                                      ruby2KeywordsHashProfile = false
                                                                   children:
                                                                       receiver =
                                                                           SelfNode
diff --git a/spec/truffle/parsing/fixtures/rescue/capturing/with_element_reference.yaml b/spec/truffle/parsing/fixtures/rescue/capturing/with_element_reference.yaml
@@ -92,7 +92,6 @@ ast: |
                                                                       methodName = "a"
                                                                       notEmptyKeywordsProfile = false
                                                                       notRuby2KeywordsHashProfile = false
-                                                                      ruby2KeywordsHashProfile = false
                                                                   children:
                                                                       receiver =
                                                                           SelfNode
diff --git a/spec/truffle/parsing/fixtures/rescue/modifier/backtrace_optimization/disabled/when_method_call.yaml b/spec/truffle/parsing/fixtures/rescue/modifier/backtrace_optimization/disabled/when_method_call.yaml
@@ -34,7 +34,6 @@ ast: |
                                   methodName = "bar"
                                   notEmptyKeywordsProfile = false
                                   notRuby2KeywordsHashProfile = false
-                                  ruby2KeywordsHashProfile = false
                               children:
                                   receiver =
                                       SelfNode
diff --git a/spec/truffle/parsing/fixtures/rescue/modifier/backtrace_optimization/enabled/when_complex_literal.yaml b/spec/truffle/parsing/fixtures/rescue/modifier/backtrace_optimization/enabled/when_complex_literal.yaml
@@ -34,7 +34,6 @@ ast: |
                                   methodName = "convert"
                                   notEmptyKeywordsProfile = false
                                   notRuby2KeywordsHashProfile = false
-                                  ruby2KeywordsHashProfile = false
                               children:
                                   arguments = [
                                       IntegerFixnumLiteralNode
diff --git a/spec/truffle/parsing/fixtures/rescue/modifier/backtrace_optimization/enabled/when_rational_literal.yaml b/spec/truffle/parsing/fixtures/rescue/modifier/backtrace_optimization/enabled/when_rational_literal.yaml
@@ -34,7 +34,6 @@ ast: |
                                   methodName = "convert"
                                   notEmptyKeywordsProfile = false
                                   notRuby2KeywordsHashProfile = false
-                                  ruby2KeywordsHashProfile = false
                               children:
                                   arguments = [
                                       IntegerFixnumLiteralNode
diff --git a/spec/truffle/parsing/fixtures/strings/backtick_literal.yaml b/spec/truffle/parsing/fixtures/strings/backtick_literal.yaml
@@ -21,7 +21,6 @@ ast: |
           methodName = "`"
           notEmptyKeywordsProfile = false
           notRuby2KeywordsHashProfile = false
-          ruby2KeywordsHashProfile = false
       children:
           arguments = [
               StringLiteralNode
diff --git a/spec/truffle/parsing/fixtures/strings/backtick_literal_with_interpolation.yaml b/spec/truffle/parsing/fixtures/strings/backtick_literal_with_interpolation.yaml
@@ -21,7 +21,6 @@ ast: |
           methodName = "`"
           notEmptyKeywordsProfile = false
           notRuby2KeywordsHashProfile = false
-          ruby2KeywordsHashProfile = false
       children:
           arguments = [
               InterpolatedStringNode
diff --git a/spec/truffle/parsing/fixtures/strings/backtick_literal_with_interpolation_without_expression.yaml b/spec/truffle/parsing/fixtures/strings/backtick_literal_with_interpolation_without_expression.yaml
@@ -24,7 +24,6 @@ ast: |
           methodName = "`"
           notEmptyKeywordsProfile = false
           notRuby2KeywordsHashProfile = false
-          ruby2KeywordsHashProfile = false
       children:
           arguments = [
               InterpolatedStringNode
diff --git a/src/main/java/org/truffleruby/builtins/ReturnEnumeratorIfNoBlockNode.java b/src/main/java/org/truffleruby/builtins/ReturnEnumeratorIfNoBlockNode.java
@@ -53,7 +53,7 @@ public Object execute(VirtualFrame frame) {
             final Object[] rubyArgs = RubyArguments.repack(frame.getArguments(), receiver, 0, 1);
             RubyArguments.setArgument(rubyArgs, 0, methodSymbol);
 
-            return toEnumNode.execute(null, receiver, "to_enum", rubyArgs, PRIVATE, null);
+            return toEnumNode.execute(null, receiver, "to_enum", rubyArgs, PRIVATE);
         } else {
             return method.execute(frame);
         }
diff --git a/src/main/java/org/truffleruby/cext/CExtNodes.java b/src/main/java/org/truffleruby/cext/CExtNodes.java
@@ -1244,7 +1244,7 @@ Object callSuper(VirtualFrame frame, Object[] args,
             final InternalMethod superMethod = superMethodLookup.getMethod();
             // This C API only passes positional arguments, but maybe it should be influenced by ruby2_keywords hashes?
             return callSuperMethodNode.execute(
-                    frame, callingSelf, superMethod, EmptyArgumentsDescriptor.INSTANCE, args, nil, null);
+                    frame, callingSelf, superMethod, EmptyArgumentsDescriptor.INSTANCE, args, nil);
         }
 
         @TruffleBoundary
diff --git a/src/main/java/org/truffleruby/core/basicobject/BasicObjectNodes.java b/src/main/java/org/truffleruby/core/basicobject/BasicObjectNodes.java
@@ -398,8 +398,7 @@ Object instanceExec(VirtualFrame frame, Object receiver, Object[] arguments, Rub
                     new SingletonClassOfSelfDefaultDefinee(receiver),
                     block.declarationContext.getRefinements());
             var descriptor = RubyArguments.getDescriptor(frame);
-            return callBlockNode.executeCallBlock(
-                    declarationContext, block, receiver, nil, descriptor, arguments, null);
+            return callBlockNode.executeCallBlock(declarationContext, block, receiver, nil, descriptor, arguments);
         }
 
         @Specialization
@@ -422,8 +421,7 @@ Object instanceExec(ArgumentsDescriptor descriptor, Object self, Object[] argume
                     new SingletonClassOfSelfDefaultDefinee(self),
                     block.declarationContext.getRefinements());
 
-            return callBlockNode.executeCallBlock(
-                    declarationContext, block, self, nil, descriptor, arguments, null);
+            return callBlockNode.executeCallBlock(declarationContext, block, self, nil, descriptor, arguments);
         }
 
     }
@@ -561,7 +559,7 @@ Object send(Frame callerFrame, Object self, Object[] rubyArgs, RootCallTarget ta
                 @Cached NameToJavaStringNode nameToJavaString) {
             Object name = RubyArguments.getArgument(rubyArgs, 0);
             return dispatchNode.execute(callerFrame, self, nameToJavaString.execute(this, name),
-                    RubyArguments.repack(rubyArgs, self, 1), PRIVATE, null);
+                    RubyArguments.repack(rubyArgs, self, 1), PRIVATE);
         }
     }
 
diff --git a/src/main/java/org/truffleruby/core/kernel/KernelNodes.java b/src/main/java/org/truffleruby/core/kernel/KernelNodes.java
@@ -1396,8 +1396,7 @@ Object send(Frame callerFrame, Object self, Object[] rubyArgs, RootCallTarget ta
                 @Cached NameToJavaStringNode nameToJavaString) {
             Object name = RubyArguments.getArgument(rubyArgs, 0);
             Object[] newArgs = RubyArguments.repack(rubyArgs, self, 1);
-            return dispatchNode.execute(callerFrame, self, nameToJavaString.execute(this, name), newArgs, PUBLIC,
-                    null);
+            return dispatchNode.execute(callerFrame, self, nameToJavaString.execute(this, name), newArgs, PUBLIC);
         }
 
     }
diff --git a/src/main/java/org/truffleruby/core/klass/ClassNodes.java b/src/main/java/org/truffleruby/core/klass/ClassNodes.java
@@ -288,8 +288,7 @@ Object newInstance(Frame callerFrame, RubyClass rubyClass, Object[] rubyArgs, Ro
                 @Cached DispatchNode allocateNode,
                 @Cached DispatchNode initializeNode) {
             final Object instance = allocateNode.call(rubyClass, "__allocate__");
-            initializeNode.execute(null, instance, "initialize", RubyArguments.repack(rubyArgs, instance), PRIVATE,
-                    null);
+            initializeNode.execute(null, instance, "initialize", RubyArguments.repack(rubyArgs, instance), PRIVATE);
             return instance;
         }
 
diff --git a/src/main/java/org/truffleruby/core/method/MethodNodes.java b/src/main/java/org/truffleruby/core/method/MethodNodes.java
@@ -141,7 +141,7 @@ static Object callBoundMethod(Frame frame, InternalMethod internalMethod, Object
             final Object[] newArgs = RubyArguments.repack(callerRubyArgs, receiver);
             RubyArguments.setMethod(newArgs, internalMethod);
             assert RubyArguments.assertFrameArguments(newArgs);
-            return callInternalMethodNode.execute(frame, internalMethod, receiver, newArgs, null);
+            return callInternalMethodNode.execute(frame, internalMethod, receiver, newArgs);
         }
     }
 
diff --git a/src/main/java/org/truffleruby/core/method/RubyMethod.java b/src/main/java/org/truffleruby/core/method/RubyMethod.java
@@ -73,7 +73,7 @@ public Object execute(Object[] arguments,
         final Object[] convertedArguments = foreignToRubyArgumentsNode.executeConvert(arguments);
         final Object[] frameArgs = RubyArguments.pack(null, null, method, null, receiver, nil,
                 EmptyArgumentsDescriptor.INSTANCE, convertedArguments);
-        return callInternalMethodNode.execute(null, method, receiver, frameArgs, null);
+        return callInternalMethodNode.execute(null, method, receiver, frameArgs);
     }
     // endregion
 
diff --git a/src/main/java/org/truffleruby/core/module/ModuleNodes.java b/src/main/java/org/truffleruby/core/module/ModuleNodes.java
@@ -808,7 +808,7 @@ Object classExec(ArgumentsDescriptor descriptor, RubyModule self, Object[] args,
                     new FixedDefaultDefinee(self),
                     block.declarationContext.getRefinements());
 
-            return callBlockNode.executeCallBlock(declarationContext, block, self, nil, descriptor, args, null);
+            return callBlockNode.executeCallBlock(declarationContext, block, self, nil, descriptor, args);
         }
     }
 
@@ -2268,8 +2268,7 @@ RubyModule refine(RubyModule namespace, RubyModule moduleToRefine, RubyProc bloc
                     refinement,
                     nil,
                     EmptyArgumentsDescriptor.INSTANCE,
-                    EMPTY_ARGUMENTS,
-                    null);
+                    EMPTY_ARGUMENTS);
             return refinement;
         }
 
diff --git a/src/main/java/org/truffleruby/core/proc/ProcNodes.java b/src/main/java/org/truffleruby/core/proc/ProcNodes.java
@@ -166,8 +166,7 @@ Object call(Frame callerFrame, RubyProc proc, Object[] rubyArgs, RootCallTarget
                     ProcOperations.getSelf(proc),
                     RubyArguments.getBlock(rubyArgs),
                     RubyArguments.getDescriptor(rubyArgs),
-                    RubyArguments.getRawArguments(rubyArgs),
-                    null);
+                    RubyArguments.getRawArguments(rubyArgs));
         }
     }
 
diff --git a/src/main/java/org/truffleruby/language/dispatch/DispatchMethodMissingNode.java b/src/main/java/org/truffleruby/language/dispatch/DispatchMethodMissingNode.java
@@ -35,28 +35,18 @@
 public abstract class DispatchMethodMissingNode extends RubyBaseNode {
 
     public abstract Object execute(Frame frame, Object receiver, String methodName, Object[] rubyArgs,
-            DispatchConfiguration config, LiteralCallNode literalCallNode);
+            DispatchConfiguration config);
 
     @Specialization(guards = "config.missingBehavior == RETURN_MISSING")
     static Object dispatchReturnMissing(
-            Frame frame,
-            Object receiver,
-            String methodName,
-            Object[] rubyArgs,
-            DispatchConfiguration config,
-            LiteralCallNode literalCallNode) {
+            Frame frame, Object receiver, String methodName, Object[] rubyArgs, DispatchConfiguration config) {
         return MISSING;
     }
 
     @InliningCutoff
     @Specialization(guards = { "config.missingBehavior == CALL_METHOD_MISSING", "isForeignObject(receiver)" })
     static Object dispatchForeign(
-            Frame frame,
-            Object receiver,
-            String methodName,
-            Object[] rubyArgs,
-            DispatchConfiguration config,
-            LiteralCallNode literalCallNode,
+            Frame frame, Object receiver, String methodName, Object[] rubyArgs, DispatchConfiguration config,
             @Cached CallForeignMethodNode callForeign) {
         final Object block = RubyArguments.getBlock(rubyArgs);
         final Object[] arguments = RubyArguments.getPositionalArguments(rubyArgs);
@@ -66,12 +56,7 @@ static Object dispatchForeign(
     @InliningCutoff
     @Specialization(guards = { "config.missingBehavior == CALL_METHOD_MISSING", "!isForeignObject(receiver)" })
     static Object dispatchMissingMethod(
-            Frame frame,
-            Object receiver,
-            String methodName,
-            Object[] rubyArgs,
-            DispatchConfiguration config,
-            LiteralCallNode literalCallNode,
+            Frame frame, Object receiver, String methodName, Object[] rubyArgs, DispatchConfiguration config,
             @Cached ToSymbolNode toSymbol,
             @Cached DispatchNode callMethodMissing,
             @Cached InlinedBranchProfile methodMissingMissingProfile,
@@ -82,7 +67,7 @@ static Object dispatchMissingMethod(
 
         RubyArguments.setArgument(newArgs, 0, symbolName);
         final Object result = callMethodMissing.execute(frame, receiver, "method_missing", newArgs,
-                DispatchConfiguration.PRIVATE_RETURN_MISSING_IGNORE_REFINEMENTS, literalCallNode);
+                DispatchConfiguration.PRIVATE_RETURN_MISSING_IGNORE_REFINEMENTS);
 
         if (result == MISSING) {
             methodMissingMissingProfile.enter(node);
diff --git a/src/main/java/org/truffleruby/language/dispatch/DispatchNode.java b/src/main/java/org/truffleruby/language/dispatch/DispatchNode.java
diff --git a/src/main/java/org/truffleruby/language/dispatch/LiteralCallNode.java b/src/main/java/org/truffleruby/language/dispatch/LiteralCallNode.java
diff --git a/src/main/java/org/truffleruby/language/dispatch/RubyCallNode.java b/src/main/java/org/truffleruby/language/dispatch/RubyCallNode.java
diff --git a/src/main/java/org/truffleruby/language/methods/CallInternalMethodNode.java b/src/main/java/org/truffleruby/language/methods/CallInternalMethodNode.java
diff --git a/src/main/java/org/truffleruby/language/methods/SymbolProcNode.java b/src/main/java/org/truffleruby/language/methods/SymbolProcNode.java
diff --git a/src/main/java/org/truffleruby/language/supercall/CallSuperMethodNode.java b/src/main/java/org/truffleruby/language/supercall/CallSuperMethodNode.java
diff --git a/src/main/java/org/truffleruby/language/supercall/SuperCallNode.java b/src/main/java/org/truffleruby/language/supercall/SuperCallNode.java
diff --git a/src/main/java/org/truffleruby/language/yield/CallBlockNode.java b/src/main/java/org/truffleruby/language/yield/CallBlockNode.java
diff --git a/src/main/java/org/truffleruby/language/yield/YieldExpressionNode.java b/src/main/java/org/truffleruby/language/yield/YieldExpressionNode.java
diff --git a/test/mri/tests/ruby/test_keyword.rb b/test/mri/tests/ruby/test_keyword.rb

Original file line number	Diff line number	Diff line change
`@@ -12,6 +12,7 @@ Compatibility:`
`12`	`12`
`13`	`13`	`Performance:`
`14`	`14`
	`15`	+* Optimize calls with `ruby2_keywords` forwarding by deciding it per call site instead of per callee thanks to [my fix in CRuby 3.2](https://bugs.ruby-lang.org/issues/18625) (@eregon).
`15`	`16`
`16`	`17`	`Changes:`
`17`	`18`
Original file line number	Diff line number	Diff line change
`@@ -1,2 +1 @@`
`1`	`1`	`fails:Keyword arguments raises ArgumentError for missing keyword arguments even if there are extra ones`
`2`		`-fails:Keyword arguments delegation does not work with call(*ruby2_keyword_args) with missing ruby2_keywords in between`