From 740535f843d65be45732e45b9fc07eadc4d63ba7 Mon Sep 17 00:00:00 2001 From: nobu Date: Wed, 11 Dec 2013 07:01:29 +0000 Subject: hash.c: reject should return a plain hash * hash.c (rb_hash_reject): return a plain hash, without copying the class, default value, instance variables, and taintedness. they had been copied just by accident. [ruby-core:59045] [Bug #9223] git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@44137 b2dd03c8-39d4-4d8f-98ff-823fe69b080e --- ChangeLog | 7 +++++++ hash.c | 35 ++++++++++++++++++++--------------- test/ruby/test_hash.rb | 37 ++++++++++++++++++++++++++++++++++++- 3 files changed, 63 insertions(+), 16 deletions(-) diff --git a/ChangeLog b/ChangeLog index edeab0d141..23da1f6a2d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,10 @@ +Wed Dec 11 16:01:26 2013 Nobuyoshi Nakada + + * hash.c (rb_hash_reject): return a plain hash, without copying + the class, default value, instance variables, and taintedness. + they had been copied just by accident. + [ruby-core:59045] [Bug #9223] + Wed Dec 11 15:36:15 2013 Aman Gupta * compile.c (iseq_specialized_instruction): emit opt_aset instruction diff --git a/hash.c b/hash.c index 4273aec45a..83c243fbf0 100644 --- a/hash.c +++ b/hash.c @@ -1095,37 +1095,39 @@ rb_hash_reject_bang(VALUE hash) } static int -reject_i(VALUE key, VALUE value, VALUE hash) +reject_i(VALUE key, VALUE value, VALUE result) { if (!RTEST(rb_yield_values(2, key, value))) { - rb_hash_aset(hash, key, value); + rb_hash_aset(result, key, value); } return ST_CONTINUE; } /* * call-seq: - * hsh.reject {| key, value | block } -> a_hash - * hsh.reject -> an_enumerator + * hsh.reject {|key, value| block} -> a_hash + * hsh.reject -> an_enumerator + * + * Returns a new hash consisting of entries for which the block returns false. * - * Same as Hash#delete_if, but works on (and returns) a - * copy of the hsh. Equivalent to - * hsh.dup.delete_if. + * If no block is given, an enumerator is returned instead. * + * h = { "a" => 100, "b" => 200, "c" => 300 } + * h.reject {|k,v| k < "b"} #=> {"b" => 200, "c" => 300} + * h.reject {|k,v| v > 100} #=> {"a" => 100} */ -static VALUE +VALUE rb_hash_reject(VALUE hash) { - VALUE ret; + VALUE result; RETURN_SIZED_ENUMERATOR(hash, 0, 0, hash_enum_size); - ret = hash_alloc(rb_obj_class(hash)); - OBJ_INFECT(ret, hash); + result = rb_hash_new(); if (!RHASH_EMPTY_P(hash)) { - rb_hash_foreach(hash, reject_i, ret); + rb_hash_foreach(hash, reject_i, result); } - return ret; + return result; } /* @@ -1154,8 +1156,9 @@ rb_hash_values_at(int argc, VALUE *argv, VALUE hash) static int select_i(VALUE key, VALUE value, VALUE result) { - if (RTEST(rb_yield_values(2, key, value))) + if (RTEST(rb_yield_values(2, key, value))) { rb_hash_aset(result, key, value); + } return ST_CONTINUE; } @@ -1180,7 +1183,9 @@ rb_hash_select(VALUE hash) RETURN_SIZED_ENUMERATOR(hash, 0, 0, hash_enum_size); result = rb_hash_new(); - rb_hash_foreach(hash, select_i, result); + if (!RHASH_EMPTY_P(hash)) { + rb_hash_foreach(hash, select_i, result); + } return result; } diff --git a/test/ruby/test_hash.rb b/test/ruby/test_hash.rb index 8c8ce6d7c3..208e628a6b 100644 --- a/test/ruby/test_hash.rb +++ b/test/ruby/test_hash.rb @@ -539,6 +539,8 @@ class TestHash < Test::Unit::TestCase end def test_reject + assert_equal({3=>4,5=>6}, @cls[1=>2,3=>4,5=>6].reject {|k, v| k + v < 7 }) + base = @cls[ 1 => 'one', 2 => false, true => 'true', 'cat' => 99 ] h1 = @cls[ 1 => 'one', 2 => false, true => 'true' ] h2 = @cls[ 2 => false, 'cat' => 99 ] @@ -556,7 +558,14 @@ class TestHash < Test::Unit::TestCase assert_equal(h3, h.reject {|k,v| v }) assert_equal(base, h) - assert_predicate(h.taint.reject {true}, :tainted?) + h.instance_variable_set(:@foo, :foo) + h.default = 42 + h.taint + h = h.reject {false} + assert_instance_of(Hash, h) + assert_not_predicate(h, :tainted?) + assert_nil(h.default) + assert_not_send([h, :instance_variable_defined?, :@foo]) end def test_reject! @@ -816,6 +825,32 @@ class TestHash < Test::Unit::TestCase def test_select assert_equal({3=>4,5=>6}, @cls[1=>2,3=>4,5=>6].select {|k, v| k + v >= 7 }) + + base = @cls[ 1 => 'one', '2' => false, true => 'true', 'cat' => 99 ] + h1 = @cls[ '2' => false, 'cat' => 99 ] + h2 = @cls[ 1 => 'one', true => 'true' ] + h3 = @cls[ 1 => 'one', true => 'true', 'cat' => 99 ] + + h = base.dup + assert_equal(h, h.select { true }) + assert_equal(@cls[], h.select { false }) + + h = base.dup + assert_equal(h1, h.select {|k,v| k.instance_of?(String) }) + + assert_equal(h2, h.select {|k,v| v.instance_of?(String) }) + + assert_equal(h3, h.select {|k,v| v }) + assert_equal(base, h) + + h.instance_variable_set(:@foo, :foo) + h.default = 42 + h.taint + h = h.select {true} + assert_instance_of(Hash, h) + assert_not_predicate(h, :tainted?) + assert_nil(h.default) + assert_not_send([h, :instance_variable_defined?, :@foo]) end def test_select! -- cgit v1.2.3