Project

General

Profile

Actions

Bug #9223

closed

Hash#reject!.size does not reflect changes to the hash

Added by dmarcotte (Daniel Marcotte) almost 11 years ago. Updated almost 11 years ago.

Status:
Closed
Target version:
ruby -v:
ruby 2.0.0p353
[ruby-core:58914]

Description

Here's an example demonstrating the issue, comparing to the regular reject behavior:

h = {a: 'A', b: 'B'}
reject_enum = h.reject
reject_bang_enum = h.reject!
h[:c] = 'C'
p reject_enum.size # 3
p reject_bang_enum.size # 2

Updated by nobu (Nobuyoshi Nakada) almost 11 years ago

Seems inverse.

Hash#reject is equivalent to the following ruby code:

def reject(&block)
dup.delete_if(&block)
end

That is, the change on the original receiver can't affect the result.

Actions #2

Updated by nobu (Nobuyoshi Nakada) almost 11 years ago

  • Status changed from Open to Closed
  • % Done changed from 0 to 100

This issue was solved with changeset r44047.
Daniel, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


hash.c: rb_hash_reject without dup

  • hash.c (rb_hash_reject): copy unrejected elements only to new hash,
    so that the change on the original receiver can affect.
    [ruby-core:58914] [Bug #9223]

Updated by tmm1 (Aman Karmani) almost 11 years ago

r44047 introduced a small change in behavior. Before, reject would copy ivars and default proc into the return hash:

h = {}
h.instance_variable_set(:@foo, 1)
p h.instance_variable_get(:@foo) #=> 1
p h.reject{true}.instance_variable_get(:@foo) #=> 1

Now, only the class is copied. This can cause subtle issues when a subclass of Hash is used (if it employs ivars).

Note that both the old and new behavior here are completely different than Hash#select, which always returns a plain Hash with no ivars.

Updated by nagachika (Tomoyuki Chikanaga) almost 11 years ago

  • Status changed from Closed to Assigned
  • Assignee set to matz (Yukihiro Matsumoto)

Hello,

r44137 introduce incompatible spec change of Hash#reject.
After r44137, Hash#reject return new hash and doesn't copied class, ivars, default value, taintedness from receiver.
ChangeLog says 'they had been copied just by accident.'
Even if it is the truth, hsh.reject is equivalent to hsh.dup.reject! for very long time and it is documented explicitly. There's a such description since ruby 1.8.1.
https://github.com/ruby/ruby/blob/v1_8_1/hash.c#L734

I think this change needs approval by matz, or at least by 2.1 release manager (naruse san).

Updated by naruse (Yui NARUSE) almost 11 years ago

  • Due date set to 12/16/2013
  • Category set to core
  • Priority changed from Normal to 7
  • Target version set to 2.1.0
  • % Done changed from 100 to 80

This blocks 2.1.0-rc

Actions #6

Updated by matz (Yukihiro Matsumoto) almost 11 years ago

I accept this behavior change. #reject should not copy instance variables etc. just like #select.

Matz.

Updated by naruse (Yui NARUSE) almost 11 years ago

NEWS says

+* Hash

    • incompatible changes:
    • Hash#reject now returns plain Hash object, that is the original object's
  •  subclass, instance variables, default value, and taintedness are no longer
    
  •  copied.
    

but test/ruby/test_hash.rb checks

assert_instance_of(Hash, h)

Updated by nobu (Nobuyoshi Nakada) almost 11 years ago

(13/12/17 1:27), naruse (Yui NARUSE) wrote:

+* Hash

    • incompatible changes:
    • Hash#reject now returns plain Hash object, that is the original object's
  •  subclass, instance variables, default value, and taintedness are no longer
    
  •  copied.
    

but test/ruby/test_hash.rb checks

assert_instance_of(Hash, h)

Right.
The result of Hash#reject is always an instance of Hash, not SubHash, now.

Better descriptions are always welcome, of course.

Updated by naruse (Yui NARUSE) almost 11 years ago

nobu (Nobuyoshi Nakada) wrote:

(13/12/17 1:27), naruse (Yui NARUSE) wrote:

+* Hash

    • incompatible changes:
    • Hash#reject now returns plain Hash object, that is the original object's
  •  subclass, instance variables, default value, and taintedness are no longer
    
  •  copied.
    

but test/ruby/test_hash.rb checks

assert_instance_of(Hash, h)

Right.
The result of Hash#reject is always an instance of Hash, not SubHash, now.

Better descriptions are always welcome, of course.

The change (SubClass) is not allowed for 2.1 because it breaks HashWithDifferentAccess.

Actions #10

Updated by nobu (Nobuyoshi Nakada) almost 11 years ago

  • Status changed from Assigned to Closed
  • % Done changed from 80 to 100

This issue was solved with changeset r44263.
Daniel, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


hash.c: revert

  • hash.c (rb_hash_reject): revert to deprecated behavior, with
    warnings, due to compatibility for HashWithDifferentAccess.
    [ruby-core:59154] [Bug #9223]
Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0