CODEWITHSUNDEEP
ruby
vovan
vovan

Reputation: 1520

Wrong output of Hash#keys method

Under certain conditions Hash#keys does not work correctly in Ruby before version 2.4

Demo code:

h = { a: 1, b: 2, c: 3 }
h.each do |k, v|
    h.delete(:a)
    p h
    p h.keys
    break
end

Ruby 2.3.8 output:

{:b=>2, :c=>3}
[:b]

Ruby 2.5.1 output:

{:b=>2, :c=>3}
[:b, :c]

I agree it is not good to modify hash when iterating. But I did not see the relation between the modification the hash and the work keys method.

Why is this happening?

Upvotes: 10

Views: 147

Answers (1)

Eric Duminil
Eric Duminil

Reputation: 54233

Interesting question. This isn't an answer yet, but it's too long for a comment and it could help others answer the question.

Which Rubies are affected?

I created a GitHub repository with a very simple spec:

describe Hash do
  it "should always know which keys are left" do
    h = { a: 1, b: 2, c: 3 }
    h.each do |k, v|
      h.delete :a
      expect(h.keys).to eq [:b, :c]
    end
  end
end

enter image description here Thanks to Travis, it's easy to see which Ruby versions have this bug:

  • Ruby 2.1
  • Ruby 2.2
  • Ruby 2.3

When did the bug appear?

When was the bug fixed?

I just spent an hour using git bisect and make install in order to find that the bug has been fixed in this commit (75775157).

Introduce table improvement by Vladimir Makarov .

[Feature #12142] See header of st.c for improvment details.

You can see all of code history here: https://github.com/vnmakarov/ruby/tree/hash_tables_with_open_addressing

This improvement is discussed at https://bugs.ruby-lang.org/issues/12142 with many people, especially with Yura Sokolov.

  • st.c: improve st_table.

  • include/ruby/st.h: ditto.

  • internal.h, numeric.c, hash.c (rb_dbl_long_hash): extract a function.

  • ext/-test-/st/foreach/foreach.c: catch up this change.

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@56650 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

It has been confirmed by @Vovan, who found this commit 1 minute before I did.

Upvotes: 7

Related Questions