[GR-34365] Implement full Ruby 3 keyword arguments semantics (#2453)

PullRequest: truffleruby/3231

.gitignore

@@ -71,7 +71,6 @@ test/mri/tests/cext-c/**/mkmf.log
 
 # Tests
 /mri_tests.txt
-/test/truffle/ecosystem/blog5/Gemfile.lock
 /test/truffle/ecosystem/blog6/Gemfile.lock
 /test/truffle/integration/gem-testing
 /truffleruby-gem-test-pack

CHANGELOG.md

@@ -23,6 +23,7 @@ Bug fixes:
 
 Compatibility:
 
+* Implement full Ruby 3 keyword arguments semantics (#2453, @eregon, @chrisseaton).
 * Implement `ruby_native_thread_p` for compatibility (#2556, @aardvark179).
 * Add `rb_argv0` for the `tk` gem. (#2556, @aardvark179).
 * Implement more correct conversion of array elements by `Array#pack`(#2503, #2504, @aardvark179).

ci.jsonnet

@@ -487,8 +487,8 @@ local composition_environment = utils.add_inclusion_tracking(part_definitions, "
   test_builds:
     {
       "ruby-lint": $.platform.linux + $.cap.gate + $.jdk.v11 + $.use.common + $.env.jvm + $.use.build + $.run.lint + { timelimit: "45:00" },
-      # Run specs on MRI to make sure new specs are compatible and have the needed version guards
-      "ruby-test-specs-mri": $.platform.linux + $.cap.gate + $.use.skip_docs + $.use.common + $.run.test_specs_mri + { timelimit: "45:00" },
+      # Run specs on CRuby to make sure new specs are compatible and have the needed version guards
+      "ruby-test-specs-on-cruby": $.platform.linux + $.cap.gate + $.use.skip_docs + $.use.common + $.run.test_specs_mri + { timelimit: "45:00" },
     } +
 
     {

lib/cext/ABI_check.txt

@@ -1 +1 @@
-3
+6

lib/mri/pp.rb

@@ -237,11 +237,7 @@ def seplist(list, sep=nil, iter_method=:each) # :yield: element
         else
           sep.call
         end
-        if defined?(::TruffleRuby)
-          yield(*v)
-        else
-          yield(*v, **{})
-        end
+        yield(*v, **{})
       }
     end
 

lib/truffle/pathname.rb

@@ -991,11 +991,11 @@ def zero?() FileTest.zero?(@path) end
 
 class Pathname    # * Dir *
   # See <tt>Dir.glob</tt>.  Returns or yields Pathname objects.
-  def Pathname.glob(*args) # :yield: pathname
+  def Pathname.glob(*args, **kwargs) # :yield: pathname
     if block_given?
-      Dir.glob(*args) { |f| yield self.new(f) }
+      Dir.glob(*args, **kwargs) { |f| yield self.new(f) }
     else
-      Dir.glob(*args).map { |f| self.new(f) }
+      Dir.glob(*args, **kwargs).map { |f| self.new(f) }
     end
   end
 

lib/truffle/truffle/cext.rb

@@ -832,11 +832,7 @@ def rb_str_encode(str, to, ecflags, ecopts)
       opts[:invalid] = :replace
     end
 
-    if opts.empty?
-      str.encode(to)
-    else
-      str.encode(to, opts)
-    end
+    str.encode(to, **opts)
   end
 
   def rb_str_conv_enc_opts(str, from, to, ecflags, ecopts)
@@ -879,6 +875,10 @@ def rb_funcallv(recv, meth, argv)
     Primitive.send_argv_without_cext_lock(recv, meth, argv, nil)
   end
 
+  def rb_funcallv_keywords(recv, meth, argv)
+    Primitive.send_argv_keywords_without_cext_lock(recv, meth, argv, nil)
+  end
+
   def rb_funcall(recv, meth, n, *args)
     Primitive.send_without_cext_lock(recv, meth, args, nil)
   end
@@ -1183,6 +1183,15 @@ def rb_class_new_instance(klass, args)
     obj
   end
 
+  def rb_class_new_instance_kw(klass, args)
+    *args, kwargs = args
+    kwargs = Truffle::Type.rb_convert_type kwargs, Hash, :to_hash
+
+    obj = klass.send(:__allocate__)
+    obj.send(:initialize, *args, **kwargs)
+    obj
+  end
+
   def rb_f_sprintf(args)
     sprintf(*args)
   end

mx.truffleruby/suite.py

@@ -110,7 +110,6 @@
                 "truffleruby-gem-test-pack",
                 "lib/json/java",
                 "lib/ruby",
-                "test/truffle/ecosystem/blog5",
                 "test/truffle/ecosystem/blog6",
                 "test/truffle/ecosystem/hello-world",
                 "test/truffle/ecosystem/rails-app",

spec/ruby/core/hash/ruby2_keywords_hash_spec.rb

@@ -40,6 +40,20 @@
       kw.should == h
     end
 
+    it "copies instance variables" do
+      h = {a: 1}
+      h.instance_variable_set(:@foo, 42)
+      kw = Hash.ruby2_keywords_hash(h)
+      kw.instance_variable_get(:@foo).should == 42
+    end
+
+    it "copies the hash internals" do
+      h = {a: 1}
+      kw = Hash.ruby2_keywords_hash(h)
+      h[:a] = 2
+      kw[:a].should == 1
+    end
+
     it "raises TypeError for non-Hash" do
       -> { Hash.ruby2_keywords_hash(nil) }.should raise_error(TypeError)
     end

spec/ruby/core/module/ruby2_keywords_spec.rb

@@ -15,6 +15,83 @@ def foo(*a) a.last end
       Hash.ruby2_keywords_hash?(last).should == true
     end
 
+    it "makes a copy of the hash and only marks the copy as keyword hash" do
+      obj = Object.new
+      obj.singleton_class.class_exec do
+        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)
+      Hash.ruby2_keywords_hash?(last).should == true
+      Hash.ruby2_keywords_hash?(h).should == false
+
+      last2 = obj.foo(**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
+      obj = Object.new
+      obj.singleton_class.class_exec do
+        ruby2_keywords def foo(*args)
+          args
+        end
+
+        def single(arg)
+          arg
+        end
+
+        def splat(*args)
+          args.last
+        end
+
+        def kwargs(**kw)
+          kw
+        end
+      end
+
+      h = { a: 1 }
+      args = obj.foo(**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
+
+      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.2" do
+        Hash.ruby2_keywords_hash?(after_usage).should == false
+      end
+      Hash.ruby2_keywords_hash?(marked).should == true
+
+      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
+
     it "applies to the underlying method and applies across aliasing" do
       obj = Object.new
 
@@ -80,7 +157,7 @@ def foo(*a) end
       }.should raise_error(NameError, /undefined method `not_existing'/)
     end
 
-    it "acceps String as well" do
+    it "accepts String as well" do
       obj = Object.new
 
       obj.singleton_class.class_exec do

spec/ruby/core/struct/new_spec.rb

@@ -62,8 +62,11 @@ def obj.to_str() "Foo" end
     -> { Struct.new(:animal, ['chris', 'evan'])    }.should raise_error(TypeError)
   end
 
-  it "raises a ArgumentError if passed a Hash with an unknown key" do
-    -> { Struct.new(:animal, { name: 'chris' }) }.should raise_error(ArgumentError)
+  it "raises a TypeError or ArgumentError if passed a Hash with an unknown key" do
+    # CRuby raises ArgumentError: unknown keyword: :name, but that seems a bug
+    -> { Struct.new(:animal, { name: 'chris' }) }.should raise_error(StandardError) { |e|
+      [ArgumentError, TypeError].should.include?(e.class)
+    }
   end
 
   it "raises ArgumentError when there is a duplicate member" do