Fix Enumerable#tally with some arguments pattern [Feature #17744]

* Add test cases for Enumerable#tally with hash argument

* Add ruby/spec for Enumerable#tally with hash argument

* Fix Enumerable#tally does not update given frozen hash

* Add test cases for Enumerable#tally with hash convertible arguments

* Fix SEGV when Enumerable#tally takes non Hash convertible

* FIx cosmetic damage enum.c

enum.c

@@ -1069,10 +1069,14 @@ static VALUE
 enum_tally(int argc, VALUE *argv, VALUE obj)
 {
     VALUE hash;
-    if (rb_check_arity(argc, 0, 1))
-        hash = rb_check_hash_type(argv[0]);
-    else
+    if (rb_check_arity(argc, 0, 1)) {
+        hash = rb_convert_type(argv[0], T_HASH, "Hash", "to_hash");
+        rb_check_frozen(hash);
+    }
+    else {
         hash = rb_hash_new();
+    }
+
     return enum_hashify_into(obj, 0, 0, tally_i, hash);
 }
 

spec/ruby/core/enumerable/tally_spec.rb

@@ -45,6 +45,25 @@
       enum.tally({ 'foo' => 1 }).should == { 'foo' => 3, 'bar' => 1, 'baz' => 1}
     end
 
+    it "returns the given hash" do
+      enum = EnumerableSpecs::Numerous.new('foo', 'bar', 'foo', 'baz')
+      hash = { 'foo' => 1 }
+      enum.tally(hash).should equal(hash)
+    end
+
+    it "raises a FrozenError and does not udpate the given hash when the hash is frozen" do
+      enum = EnumerableSpecs::Numerous.new('foo', 'bar', 'foo', 'baz')
+      hash = { 'foo' => 1 }.freeze
+      -> { enum.tally(hash) }.should raise_error(FrozenError)
+      hash.should == { 'foo' => 1 }
+    end
+
+    it "does not call given block" do
+      enum = EnumerableSpecs::Numerous.new('foo', 'bar', 'foo', 'baz')
+      enum.tally({ 'foo' => 1 }) { |v| ScratchPad << v }
+      ScratchPad.recorded.should == []
+    end
+
     it "ignores the default value" do
       enum = EnumerableSpecs::Numerous.new('foo', 'bar', 'foo', 'baz')
       enum.tally(Hash.new(100)).should == { 'foo' => 2, 'bar' => 1, 'baz' => 1}

test/ruby/test_enum.rb

@@ -402,6 +402,34 @@ def test_tally
       @obj.tally({1 => ""})
     end
 
+    h = {1 => 2, 2 => 2, 3 => 1}
+    assert_same(h, @obj.tally(h))
+
+    h = {1 => 2, 2 => 2, 3 => 1}.freeze
+    assert_raise(FrozenError) do
+      @obj.tally(h)
+    end
+    assert_equal({1 => 2, 2 => 2, 3 => 1}, h)
+
+    hash_convertible = Object.new
+    def hash_convertible.to_hash
+      {1 => 3, 4 => "x"}
+    end
+    assert_equal({1 => 5, 2 => 2, 3 => 1, 4 => "x"}, @obj.tally(hash_convertible))
+
+    hash_convertible = Object.new
+    def hash_convertible.to_hash
+      {1 => 3, 4 => "x"}.freeze
+    end
+    assert_raise(FrozenError) do
+      @obj.tally(hash_convertible)
+    end
+    assert_equal({1 => 3, 4 => "x"}, hash_convertible.to_hash)
+
+    assert_raise(TypeError) do
+      @obj.tally(BasicObject.new)
+    end
+
     h = {1 => 2, 2 => 2, 3 => 1}
     assert_equal(h, @obj.tally(Hash.new(100)))
     assert_equal(h, @obj.tally(Hash.new {100}))