nobu	2013-12-11 16:01:29 +0900 (Wed, 11 Dec 2013)

  New Revision: 44137

  http://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=rev&revision=44137

  Log:
    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]

  Modified files:
    trunk/ChangeLog
    trunk/hash.c
    trunk/test/ruby/test_hash.rb
Index: ChangeLog
===================================================================
--- ChangeLog	(revision 44136)
+++ ChangeLog	(revision 44137)
@@ -1,3 +1,10 @@ https://github.com/ruby/ruby/blob/trunk/ChangeLog#L1
+Wed Dec 11 16:01:26 2013  Nobuyoshi Nakada  <nobu@r...>
+
+	* 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 <ruby@t...>
 
 	* compile.c (iseq_specialized_instruction): emit opt_aset instruction
Index: hash.c
===================================================================
--- hash.c	(revision 44136)
+++ hash.c	(revision 44137)
@@ -1095,37 +1095,39 @@ rb_hash_reject_bang(VALUE hash) https://github.com/ruby/ruby/blob/trunk/hash.c#L1095
 }
 
 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 <code>Hash#delete_if</code>, but works on (and returns) a
- *  copy of the <i>hsh</i>. Equivalent to
- *  <code><i>hsh</i>.dup.delete_if</code>.
+ *  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, https://github.com/ruby/ruby/blob/trunk/hash.c#L1156
 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) https://github.com/ruby/ruby/blob/trunk/hash.c#L1183
 
     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;
 }
 
Index: test/ruby/test_hash.rb
===================================================================
--- test/ruby/test_hash.rb	(revision 44136)
+++ test/ruby/test_hash.rb	(revision 44137)
@@ -539,6 +539,8 @@ class TestHash < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_hash.rb#L539
   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 https://github.com/ruby/ruby/blob/trunk/test/ruby/test_hash.rb#L558
     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 https://github.com/ruby/ruby/blob/trunk/test/ruby/test_hash.rb#L825
 
   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!

--
ML: ruby-changes@q...
Info: http://www.atdot.net/~ko1/quickml/