[GR-18163] Fix Hash#to_h called with a block and pass key and value t…

…o the block as separate arguments

PullRequest: truffleruby/4305

CHANGELOG.md

@@ -33,6 +33,7 @@ Compatibility:
 * Fix `Enumerable#reduce` to handle non-Symbol method name parameter (#2931, @andrykonchin).
 * Fix `RangeError` message to match CRuby for `Integer#chr` called with invalid codepoint argument (#2795, @andrykonchin).
 * Joni has been updated from 2.1.44 to 2.2.1 (@andrykonchin).
+* Fix `Hash#to_h` called with a block and pass key and value to the block as separate arguments (#3607, @andrykonchin).
 
 Performance:
 

spec/ruby/core/array/to_h_spec.rb

@@ -45,6 +45,12 @@
       [:a, :b].to_h { |k| [k, k.to_s] }.should == { a: 'a', b: 'b' }
     end
 
+    it "passes to a block each element as a single argument" do
+      ScratchPad.record []
+      [[:a, 1], [:b, 2]].to_h { |*args| ScratchPad << args; [args[0], args[1]] }
+      ScratchPad.recorded.sort.should == [[[:a, 1]], [[:b, 2]]]
+    end
+
     it "raises ArgumentError if block returns longer or shorter array" do
       -> do
         [:a, :b].to_h { |k| [k, k.to_s, 1] }

spec/ruby/core/enumerable/to_h_spec.rb

@@ -53,6 +53,14 @@ def enum.each(*args)
       @enum.to_h { |k| [k, k.to_s] }.should == { a: 'a', b: 'b' }
     end
 
+    it "passes to a block each element as a single argument" do
+      enum_of_arrays = EnumerableSpecs::EachDefiner.new([:a, 1], [:b, 2])
+
+      ScratchPad.record []
+      enum_of_arrays.to_h { |*args| ScratchPad << args; [args[0], args[1]] }
+      ScratchPad.recorded.sort.should == [[[:a, 1]], [[:b, 2]]]
+    end
+
     it "raises ArgumentError if block returns longer or shorter array" do
       -> do
         @enum.to_h { |k| [k, k.to_s, 1] }

spec/ruby/core/env/to_h_spec.rb

@@ -18,6 +18,18 @@
       ENV.to_h { |k, v| [k, v.upcase] }.should == { 'a' => "B", 'c' => "D" }
     end
 
+    it "passes to a block each pair's key and value as separate arguments" do
+      ENV.replace("a" => "b", "c" => "d")
+
+      ScratchPad.record []
+      ENV.to_h { |k, v| ScratchPad << [k, v]; [k, v] }
+      ScratchPad.recorded.sort.should == [["a", "b"], ["c", "d"]]
+
+      ScratchPad.record []
+      ENV.to_h { |*args| ScratchPad << args; [args[0], args[1]] }
+      ScratchPad.recorded.sort.should == [["a", "b"], ["c", "d"]]
+    end
+
     it "does not require the array elements to be strings" do
       ENV.replace("a" => "b", "c" => "d")
       ENV.to_h { |k, v| [k.to_sym, v.to_sym] }.should == { :a => :b, :c => :d }

spec/ruby/core/hash/to_h_spec.rb

@@ -37,6 +37,16 @@
       { a: 1, b: 2 }.to_h { |k, v| [k.to_s, v*v]}.should == { "a" => 1, "b" => 4 }
     end
 
+    it "passes to a block each pair's key and value as separate arguments" do
+      ScratchPad.record []
+      { a: 1, b: 2 }.to_h { |k, v| ScratchPad << [k, v]; [k, v] }
+      ScratchPad.recorded.sort.should == [[:a, 1], [:b, 2]]
+
+      ScratchPad.record []
+      { a: 1, b: 2 }.to_h { |*args| ScratchPad << args; [args[0], args[1]] }
+      ScratchPad.recorded.sort.should == [[:a, 1], [:b, 2]]
+    end
+
     it "raises ArgumentError if block returns longer or shorter array" do
       -> do
         { a: 1, b: 2 }.to_h { |k, v| [k.to_s, v*v, 1] }

spec/ruby/core/struct/to_h_spec.rb

@@ -21,6 +21,18 @@
       h.should == { "make" => "ford", "model" => "ranger", "year" => "" }
     end
 
+    it "passes to a block each pair's key and value as separate arguments" do
+      s = StructClasses::Ruby.new('3.2.4', 'macos')
+
+      ScratchPad.record []
+      s.to_h { |k, v| ScratchPad << [k, v]; [k, v] }
+      ScratchPad.recorded.sort.should == [[:platform, 'macos'], [:version, '3.2.4']]
+
+      ScratchPad.record []
+      s.to_h { |*args| ScratchPad << args; [args[0], args[1]] }
+      ScratchPad.recorded.sort.should == [[:platform, 'macos'], [:version, '3.2.4']]
+    end
+
     it "raises ArgumentError if block returns longer or shorter array" do
       -> do
         StructClasses::Car.new.to_h { |k, v| [k.to_s, "#{v}".downcase, 1] }

src/main/ruby/truffleruby/core/array.rb

@@ -1296,19 +1296,10 @@ def to_ary
   end
 
   def to_h
-    h = Hash.new
+    h = {}
     each_with_index do |elem, i|
       elem = yield(elem) if block_given?
-      unless elem.respond_to?(:to_ary)
-        raise TypeError, "wrong element type #{Primitive.class(elem)} at #{i} (expected array)"
-      end
-
-      ary = elem.to_ary
-      if ary.size != 2
-        raise ArgumentError, "wrong array length at #{i} (expected 2, was #{ary.size})"
-      end
-
-      h[ary[0]] = ary[1]
+      Truffle::HashOperations.assoc_key_value_pair_with_position(h, elem, i)
     end
     h
   end

src/main/ruby/truffleruby/core/enumerable.rb

@@ -314,20 +314,12 @@ def to_a(*args, **kwargs)
   end
   alias_method :entries, :to_a
 
-  def to_h(*arg)
+  def to_h(*args)
     h = {}
-    each_with_index(*arg) do |elem, i|
+    each(*args) do
+      elem = Primitive.single_block_arg
       elem = yield(elem) if block_given?
-      unless elem.respond_to?(:to_ary)
-        raise TypeError, "wrong element type #{Primitive.class(elem)} at #{i} (expected array)"
-      end
-
-      ary = elem.to_ary
-      if ary.size != 2
-        raise ArgumentError, "element has wrong array length (expected 2, was #{ary.size})"
-      end
-
-      h[ary[0]] = ary[1]
+      Truffle::HashOperations.assoc_key_value_pair(h, elem)
     end
     h
   end

src/main/ruby/truffleruby/core/env.rb

@@ -286,18 +286,9 @@ def to_h
     return to_hash unless block_given?
 
     h = {}
-    each_pair do |*elem|
-      elem = yield(elem)
-      unless elem.respond_to?(:to_ary)
-        raise TypeError, "wrong element type #{Primitive.class(elem)} (expected array)"
-      end
-
-      ary = elem.to_ary
-      if ary.size != 2
-        raise ArgumentError, "element has wrong array length (expected 2, was #{ary.size})"
-      end
-
-      h[ary[0]] = ary[1]
+    each_pair do |k, v|
+      pair = yield(k, v)
+      Truffle::HashOperations.assoc_key_value_pair(h, pair)
     end
     h
   end

src/main/ruby/truffleruby/core/hash.rb

@@ -355,7 +355,12 @@ def slice(*keys)
 
   def to_h
     if block_given?
-      super
+      h = {}
+      each do |k, v|
+        pair = yield(k, v)
+        Truffle::HashOperations.assoc_key_value_pair(h, pair)
+      end
+      h
     elsif instance_of? Hash
       self
     else

src/main/ruby/truffleruby/core/struct.rb

@@ -109,18 +109,9 @@ def select
 
   def to_h
     h = {}
-    each_pair.each_with_index do |elem, i|
-      elem = yield(elem) if block_given?
-      unless elem.respond_to?(:to_ary)
-        raise TypeError, "wrong element type #{Primitive.class(elem)} at #{i} (expected array)"
-      end
-
-      ary = elem.to_ary
-      if ary.size != 2
-        raise ArgumentError, "element has wrong array length (expected 2, was #{ary.size})"
-      end
-
-      h[ary[0]] = ary[1]
+    each_pair do |k, v|
+      pair = block_given? ? yield(k, v) : [k, v]
+      Truffle::HashOperations.assoc_key_value_pair(h, pair)
     end
     h
   end

src/main/ruby/truffleruby/core/truffle/hash_operations.rb

@@ -17,5 +17,35 @@ def self.hash_merge(current, other)
       end
       new_hash
     end
+
+    # MRI: rb_hash_set_pair (rb_ary_to_h also contains similar logic for Array#to_h)
+    def self.assoc_key_value_pair(hash, pair)
+      ary = Truffle::Type.rb_check_convert_type pair, Array, :to_ary
+
+      unless ary
+        raise TypeError, "wrong element type #{Primitive.class(pair)} (expected array)"
+      end
+
+      if ary.size != 2
+        raise ArgumentError, "element has wrong array length (expected 2, was #{ary.size})"
+      end
+
+      hash[ary[0]] = ary[1]
+    end
+
+    # MRI: extracted from rb_ary_to_h, is similar to rb_hash_set_pair
+    def self.assoc_key_value_pair_with_position(hash, pair, index)
+      ary = Truffle::Type.rb_check_convert_type pair, Array, :to_ary
+
+      unless ary
+        raise TypeError, "wrong element type #{Primitive.class(pair)} at #{index} (expected array)"
+      end
+
+      if ary.size != 2
+        raise ArgumentError, "wrong array length at #{index} (expected 2, was #{ary.size})"
+      end
+
+      hash[ary[0]] = ary[1]
+    end
   end
 end