[GR-37597] Copy the CRuby bug/behavior that the ruby2_keyword flag on…

… a Hash is not reset for caller(*args) -> callee(*args) calls

PullRequest: truffleruby/3279

spec/mspec/lib/mspec/expectations/expectations.rb

@@ -32,4 +32,8 @@ def self.fail_predicate(receiver, predicate, args, block, result, expectation)
     result_to_s = MSpec.format(result)
     raise SpecExpectationNotMetError, "Expected #{receiver_to_s}#{predicate_to_s}#{args_to_s}\n#{expectation} but was #{result_to_s}"
   end
+
+  def self.fail_single_arg_predicate(receiver, predicate, arg, result, expectation)
+    fail_predicate(receiver, predicate, [arg], nil, result, expectation)
+  end
 end

spec/mspec/lib/mspec/matchers/base.rb

@@ -16,15 +16,24 @@ def initialize(actual)
   end
 
   def ==(expected)
-    method_missing(:==, expected)
+    result = @actual == expected
+    unless result
+      ::SpecExpectation.fail_single_arg_predicate(@actual, :==, expected, result, "to be truthy")
+    end
   end
 
   def !=(expected)
-    method_missing(:!=, expected)
+    result = @actual != expected
+    unless result
+      ::SpecExpectation.fail_single_arg_predicate(@actual, :!=, expected, result, "to be truthy")
+    end
   end
 
   def equal?(expected)
-    method_missing(:equal?, expected)
+    result = @actual.equal?(expected)
+    unless result
+      ::SpecExpectation.fail_single_arg_predicate(@actual, :equal?, expected, result, "to be truthy")
+    end
   end
 
   def method_missing(name, *args, &block)
@@ -41,15 +50,24 @@ def initialize(actual)
   end
 
   def ==(expected)
-    method_missing(:==, expected)
+    result = @actual == expected
+    if result
+      ::SpecExpectation.fail_single_arg_predicate(@actual, :==, expected, result, "to be falsy")
+    end
   end
 
   def !=(expected)
-    method_missing(:!=, expected)
+    result = @actual != expected
+    if result
+      ::SpecExpectation.fail_single_arg_predicate(@actual, :!=, expected, result, "to be falsy")
+    end
   end
 
   def equal?(expected)
-    method_missing(:equal?, expected)
+    result = @actual.equal?(expected)
+    if result
+      ::SpecExpectation.fail_single_arg_predicate(@actual, :equal?, expected, result, "to be falsy")
+    end
   end
 
   def method_missing(name, *args, &block)

spec/ruby/core/module/ruby2_keywords_spec.rb

@@ -3,15 +3,14 @@
 
 ruby_version_is "2.7" do
   describe "Module#ruby2_keywords" do
-    it "marks the final hash argument as keyword hash" do
-      obj = Object.new
-
-      obj.singleton_class.class_exec do
-        def foo(*a) a.last end
-        ruby2_keywords :foo
+    class << self
+      ruby2_keywords def mark(*args)
+        args
       end
+    end
 
-      last = obj.foo(1, 2, a: "a")
+    it "marks the final hash argument as keyword hash" do
+      last = mark(1, 2, a: "a").last
       Hash.ruby2_keywords_hash?(last).should == true
     end
 
@@ -21,74 +20,121 @@ def foo(*a) a.last end
         def regular(*args)
           args.last
         end
-
-        ruby2_keywords def foo(*args)
-          args.last
-        end
       end
 
       h = {a: 1}
       ruby_version_is "3.0" do
         obj.regular(**h).should.equal?(h)
       end
 
-      last = obj.foo(**h)
+      last = mark(**h).last
       Hash.ruby2_keywords_hash?(last).should == true
       Hash.ruby2_keywords_hash?(h).should == false
 
-      last2 = obj.foo(**last) # last is already marked
+      last2 = mark(**last).last # last is already marked
       Hash.ruby2_keywords_hash?(last2).should == true
       Hash.ruby2_keywords_hash?(last).should == true
       last2.should_not.equal?(last)
       Hash.ruby2_keywords_hash?(h).should == false
     end
 
-    it "makes a copy and unmark at the call site when calling with marked *args" do
+    it "makes a copy and unmark the Hash when calling a method taking (arg)" do
       obj = Object.new
       obj.singleton_class.class_exec do
-        ruby2_keywords def foo(*args)
-          args
-        end
-
         def single(arg)
           arg
         end
+      end
 
-        def splat(*args)
-          args.last
-        end
+      h = { a: 1 }
+      args = mark(**h)
+      marked = args.last
+      Hash.ruby2_keywords_hash?(marked).should == true
+
+      after_usage = obj.single(*args)
+      after_usage.should == h
+      after_usage.should_not.equal?(h)
+      after_usage.should_not.equal?(marked)
+      Hash.ruby2_keywords_hash?(after_usage).should == false
+      Hash.ruby2_keywords_hash?(marked).should == true
+    end
 
+    it "makes a copy and unmark the Hash when calling a method taking (**kw)" do
+      obj = Object.new
+      obj.singleton_class.class_exec do
         def kwargs(**kw)
           kw
         end
       end
 
       h = { a: 1 }
-      args = obj.foo(**h)
+      args = mark(**h)
       marked = args.last
       Hash.ruby2_keywords_hash?(marked).should == true
 
-      after_usage = obj.single(*args)
+      after_usage = obj.kwargs(*args)
       after_usage.should == h
       after_usage.should_not.equal?(h)
       after_usage.should_not.equal?(marked)
       Hash.ruby2_keywords_hash?(after_usage).should == false
       Hash.ruby2_keywords_hash?(marked).should == true
+    end
+
+    # https://bugs.ruby-lang.org/issues/18625
+    it "does NOT copy the Hash when calling a method taking (*args)" do
+      obj = Object.new
+      obj.singleton_class.class_exec do
+        def splat(*args)
+          args.last
+        end
+
+        def splat1(arg, *args)
+          args.last
+        end
+
+        def proc_call(*args)
+          -> *args { args.last }.call(*args)
+        end
+      end
+
+      h = { a: 1 }
+      args = mark(**h)
+      marked = args.last
+      Hash.ruby2_keywords_hash?(marked).should == true
 
       after_usage = obj.splat(*args)
       after_usage.should == h
       after_usage.should_not.equal?(h)
-      after_usage.should_not.equal?(marked)
-      ruby_bug "#18625", ""..."3.3" do # might be fixed in 3.2
-        Hash.ruby2_keywords_hash?(after_usage).should == false
-      end
+      after_usage.should.equal?(marked) # https://bugs.ruby-lang.org/issues/18625
+      Hash.ruby2_keywords_hash?(after_usage).should == true # https://bugs.ruby-lang.org/issues/18625
       Hash.ruby2_keywords_hash?(marked).should == true
 
-      after_usage = obj.kwargs(*args)
+      args = mark(1, **h)
+      marked = args.last
+      after_usage = obj.splat1(*args)
       after_usage.should == h
       after_usage.should_not.equal?(h)
-      after_usage.should_not.equal?(marked)
-      Hash.ruby2_keywords_hash?(after_usage).should == false
+      after_usage.should.equal?(marked) # https://bugs.ruby-lang.org/issues/18625
+      Hash.ruby2_keywords_hash?(after_usage).should == true # https://bugs.ruby-lang.org/issues/18625
+      Hash.ruby2_keywords_hash?(marked).should == true
+
+      args = mark(**h)
+      marked = args.last
+      after_usage = obj.proc_call(*args)
+      after_usage.should == h
+      after_usage.should_not.equal?(h)
+      after_usage.should.equal?(marked) # https://bugs.ruby-lang.org/issues/18625
+      Hash.ruby2_keywords_hash?(after_usage).should == true # https://bugs.ruby-lang.org/issues/18625
+      Hash.ruby2_keywords_hash?(marked).should == true
+
+      args = mark(**h)
+      marked = args.last
+      after_usage = obj.send(:splat, *args)
+      after_usage.should == h
+      after_usage.should_not.equal?(h)
+      send_copies = RUBY_ENGINE == "ruby" # inconsistent with Proc#call above for CRuby
+      after_usage.equal?(marked).should == !send_copies
+      Hash.ruby2_keywords_hash?(after_usage).should == !send_copies
       Hash.ruby2_keywords_hash?(marked).should == true
     end
 

spec/ruby/language/keyword_arguments_spec.rb

@@ -302,6 +302,40 @@ def m(*args)
         m(a: 1).should == [[{a: 1}], {}]
         m({a: 1}).should == [[{a: 1}], {}]
       end
+
+      # https://bugs.ruby-lang.org/issues/18625
+      it "works with call(*ruby2_keyword_args) with missing ruby2_keywords in between due to CRuby bug #18625" do
+        class << self
+          def n(*args) # Note the missing ruby2_keywords here
+            target(*args)
+          end
+
+          ruby2_keywords def m(*args)
+            n(*args)
+          end
+        end
+
+        empty = {}
+        m(**empty).should == [[], {}]
+        Hash.ruby2_keywords_hash?(empty).should == false
+        m(empty).should == [[{}], {}]
+        Hash.ruby2_keywords_hash?(empty).should == false
+
+        m(a: 1).should == [[], {a: 1}]
+        m({a: 1}).should == [[{a: 1}], {}]
+
+        kw = {a: 1}
+
+        m(**kw).should == [[], {a: 1}]
+        m(**kw)[1].should == kw
+        m(**kw)[1].should_not.equal?(kw)
+        Hash.ruby2_keywords_hash?(kw).should == false
+        Hash.ruby2_keywords_hash?(m(**kw)[1]).should == false
+
+        m(kw).should == [[{a: 1}], {}]
+        m(kw)[0][0].should.equal?(kw)
+        Hash.ruby2_keywords_hash?(kw).should == false
+      end
     end
   end
 end

src/main/java/org/truffleruby/cext/CExtNodes.java

@@ -1093,7 +1093,7 @@ protected 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);
+                    frame, callingSelf, superMethod, EmptyArgumentsDescriptor.INSTANCE, args, nil, null);
         }
 
         @TruffleBoundary

src/main/java/org/truffleruby/core/basicobject/BasicObjectNodes.java

@@ -448,7 +448,7 @@ protected Object instanceExec(VirtualFrame frame, Object receiver, Object[] argu
                     block.declarationContext.getRefinements());
             var descriptor = RubyArguments.getDescriptor(frame);
             return callBlockNode.executeCallBlock(
-                    declarationContext, block, receiver, block.block, descriptor, arguments);
+                    declarationContext, block, receiver, block.block, descriptor, arguments, null);
         }
 
         @Specialization

src/main/java/org/truffleruby/core/method/MethodNodes.java

@@ -138,7 +138,7 @@ protected Object call(Frame callerFrame, RubyMethod method, Object[] rubyArgs, R
             final Object[] newArgs = RubyArguments.repack(rubyArgs, method.receiver);
             RubyArguments.setMethod(newArgs, internalMethod);
             assert RubyArguments.assertFrameArguments(newArgs);
-            return callInternalMethodNode.execute(callerFrame, internalMethod, method.receiver, newArgs);
+            return callInternalMethodNode.execute(callerFrame, internalMethod, method.receiver, newArgs, null);
         }
     }
 
@@ -360,7 +360,7 @@ public Object execute(VirtualFrame frame) {
             final Object originalBoundMethodReceiver = RubyArguments.getSelf(RubyArguments.getDeclarationFrame(frame));
             Object[] rubyArgs = RubyArguments.repack(frame.getArguments(), originalBoundMethodReceiver);
             RubyArguments.setMethod(rubyArgs, method);
-            return callInternalMethodNode.execute(frame, method, originalBoundMethodReceiver, rubyArgs);
+            return callInternalMethodNode.execute(frame, method, originalBoundMethodReceiver, rubyArgs, null);
         }
     }
 

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);
+        return callInternalMethodNode.execute(null, method, receiver, frameArgs, null);
     }
     // endregion
 

src/main/java/org/truffleruby/core/module/ModuleNodes.java

@@ -809,7 +809,7 @@ public Object classExec(ArgumentsDescriptor descriptor, RubyModule self, Object[
                     new FixedDefaultDefinee(self),
                     block.declarationContext.getRefinements());
 
-            return callBlockNode.executeCallBlock(declarationContext, block, self, block.block, descriptor, args);
+            return callBlockNode.executeCallBlock(declarationContext, block, self, block.block, descriptor, args, null);
         }
     }
 
@@ -2271,7 +2271,8 @@ protected RubyModule refine(RubyModule namespace, RubyModule moduleToRefine, Rub
                     refinement,
                     block.block,
                     EmptyArgumentsDescriptor.INSTANCE,
-                    EMPTY_ARGUMENTS);
+                    EMPTY_ARGUMENTS,
+                    null);
             return refinement;
         }
 

src/main/java/org/truffleruby/core/proc/ProcNodes.java

@@ -204,7 +204,8 @@ protected Object call(Frame callerFrame, RubyProc proc, Object[] rubyArgs, RootC
                     ProcOperations.getSelf(proc),
                     RubyArguments.getBlock(rubyArgs),
                     RubyArguments.getDescriptor(rubyArgs),
-                    RubyArguments.getRawArguments(rubyArgs));
+                    RubyArguments.getRawArguments(rubyArgs),
+                    null);
         }
     }