Project

General

Profile

Actions

Bug #7300

closed

Hash#[] の挙動が 1.9.3 と異なっている

Added by hsbt (Hiroshi SHIBATA) over 11 years ago. Updated almost 5 years ago.

Status:
Closed
Target version:
ruby -v:
ruby 2.0.0dev (2012-11-07) [x86_64-darwin12.2.0]
[ruby-dev:46440]

Description

Hash[[nil]] を実行すると 1.9.3 では {} となるものが 2.0.0 では ArgumentError となります。

なかださんに相談してみたら、2.0.0 の動きが不正なものをチェックしてて正しい挙動と
いうことを教えてもらいましたが、2.0.0 では duplicate の警告だけにして、
次のリリースで消すという方が良さそうに思います。


Files

Updated by knu (Akinori MUSHA) over 11 years ago

これ、元のコードは何でしょうか。
もし文字通りnilなんて埋めてあったらコーディングミスなので、警告を出すくらいなら例外を出して直させるべきだと思います。

問題は、

Hash[enum.map {|x| x > 0 ? [x, x*x] : nil }]

のように動的にハッシュを作るときに意図的にnil(あるいは配列化できない任意の値)を使ってペアの生成をスキップする場合でしょうか。
これにしてもそんな仕様は最初から示されていないので、これを機に直してもらうのがいい気がします。「Hash[」で検索可能だし。

Hash[enum.map {|x| x > 0 ? [x, x*x] : nil }.compact] # minimum change
Hash[enum.flat_map {|x| x > 0 ? [x, x*x] : [] }] # 1.9+ only
{}.tap { |h| enum.each {|x| x > 0 and h[x] = x*x } }

Updated by knu (Akinori MUSHA) over 11 years ago

なんかいろいろ微妙にミスった。

Hash[enum.map {|x| x > 0 ? [x, x*x] : nil }.compact] # minimum change
Hash[*enum.flat_map {|x| x > 0 ? [x, x*x] : [] }] # 1.9+ only
{}.tap { |h| enum.each {|x| x > 0 and h[x] = x*x } }

です。

Updated by hsbt (Hiroshi SHIBATA) over 11 years ago

元のコードは以下の行になります。

https://github.com/apotonick/cells/blob/master/lib/cell/test_case.rb#L78

テストケースによっては Hash[[nil, nil, nil,...]] というような状態になるようです。
ライブラリ本体を直すべきであるということであれば、異論はありませんのでそのように動くつもりです。

Updated by mame (Yusuke Endoh) over 11 years ago

  • Status changed from Open to Assigned
  • Assignee set to nobu (Nobuyoshi Nakada)

柴田さん、またまたありがとうございます。

hsbt (Hiroshi SHIBATA) wrote:

元のコードは以下の行になります。

compact 足してよ、という感じで微妙なところですが、今回は警告に一票投じます (コードがすごく汚くなるとかでなければ) 。なかださん再検討お願いします。

余談ですが、「特に実用上困ったわけじゃないけど気がついた非互換 (RubySpec みたいな)」の報告もありがたいですが、「実コードに影響があった非互換」は心理的な優先度がだいぶ上がるので、その旨書いてもらえるととても嬉しいです。

--
Yusuke Endoh

Updated by hsbt (Hiroshi SHIBATA) over 11 years ago

遠藤さん、了解です。

2.0.0 互換報告は手元の Rails アプリケーションでチェックした結果なので、ほとんどが実運用に影響のある問題です。
今後はそのことを詳しく書くことにします。

Updated by nobu (Nobuyoshi Nakada) over 11 years ago

  • Description updated (diff)

Updated by nobu (Nobuyoshi Nakada) over 11 years ago

全面的に無視するのはなんとなく気がすすまないんですが、nilだけ警告にするというのはどうでしょうか。

Updated by hsbt (Hiroshi SHIBATA) over 11 years ago

redis-rb という ruby から redis を扱うライブラリにも同じコードがありました。

https://github.com/redis/redis-rb/blob/master/lib/redis.rb#L182

redis-rb では [nil, ["redis_version", "2.6.4"], ...] というような配列を Hash[] に渡して初期化しようとして落ちています。
配列が nil だけの時の [nil, nil, ...] に加えて [nil, ["valid", "valid"], ...] みたいな一部だけ nil というような配列の時でも警告をして通すような変更だと助かります。

Actions #9

Updated by nobu (Nobuyoshi Nakada) over 11 years ago

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

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


hash.c: warn for wrong elements

Updated by mame (Yusuke Endoh) over 5 years ago

1.9 との互換性のために Hash[[nil]] を許す件、2.0.0 のころから警告し続けているので、さすがにもう消しても大丈夫な気がします。

$ ruby -ve 'Hash[[nil]]'
ruby 2.6.0p0 (2018-12-25 revision 66547) [x86_64-linux]
-e:1: warning: wrong element type nil at 0 (expected array)
-e:1: warning: ignoring wrong elements is deprecated, remove them explicitly
-e:1: warning: this causes ArgumentError in the next release

次の 2.7 で消しませんか?パッチを付けておきます。

Updated by hsbt (Hiroshi SHIBATA) about 5 years ago

僕が決められるのかわかりませんが、5年以上経っているので消していいと思います。

Actions #12

Updated by hsbt (Hiroshi SHIBATA) about 5 years ago

  • Status changed from Open to Assigned
Actions #13

Updated by nobu (Nobuyoshi Nakada) almost 5 years ago

  • Description updated (diff)

Updated by nobu (Nobuyoshi Nakada) almost 5 years ago

  • Assignee changed from nobu (Nobuyoshi Nakada) to mame (Yusuke Endoh)
Actions #15

Updated by mame (Yusuke Endoh) almost 5 years ago

  • Status changed from Assigned to Closed

Applied in changeset git|d3f1c615c5b81319e422e9c92e1cb8ba82209fba.


hash.c (rb_hash_s_create): Reject Hash[[nil]]

The behavior of Hash[[nil]] #=> {} was a bug until 1.9.3, but had been
remained with a warning because some programs depended upon it.
Now, six years passed. We can remove the compatibility behavior.
[Bug #7300]

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0