Bug #7300
closedHash#[] の挙動が 1.9.3 と異なっている
Description
Hash[[nil]]
を実行すると 1.9.3 では {}
となるものが 2.0.0 では ArgumentError
となります。
なかださんに相談してみたら、2.0.0 の動きが不正なものをチェックしてて正しい挙動と
いうことを教えてもらいましたが、2.0.0 では duplicate の警告だけにして、
次のリリースで消すという方が良さそうに思います。
Files
Updated by knu (Akinori MUSHA) about 12 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) about 12 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) about 12 years ago
元のコードは以下の行になります。
https://github.com/apotonick/cells/blob/master/lib/cell/test_case.rb#L78
テストケースによっては Hash[[nil, nil, nil,...]]
というような状態になるようです。
ライブラリ本体を直すべきであるということであれば、異論はありませんのでそのように動くつもりです。
Updated by mame (Yusuke Endoh) about 12 years ago
- Status changed from Open to Assigned
- Assignee set to nobu (Nobuyoshi Nakada)
柴田さん、またまたありがとうございます。
hsbt (Hiroshi SHIBATA) wrote:
元のコードは以下の行になります。
compact 足してよ、という感じで微妙なところですが、今回は警告に一票投じます (コードがすごく汚くなるとかでなければ) 。なかださん再検討お願いします。
余談ですが、「特に実用上困ったわけじゃないけど気がついた非互換 (RubySpec みたいな)」の報告もありがたいですが、「実コードに影響があった非互換」は心理的な優先度がだいぶ上がるので、その旨書いてもらえるととても嬉しいです。
--
Yusuke Endoh mame@tsg.ne.jp
Updated by hsbt (Hiroshi SHIBATA) about 12 years ago
遠藤さん、了解です。
2.0.0 互換報告は手元の Rails アプリケーションでチェックした結果なので、ほとんどが実運用に影響のある問題です。
今後はそのことを詳しく書くことにします。
Updated by nobu (Nobuyoshi Nakada) about 12 years ago
- Description updated (diff)
Updated by nobu (Nobuyoshi Nakada) about 12 years ago
全面的に無視するのはなんとなく気がすすまないんですが、nil
だけ警告にするというのはどうでしょうか。
Updated by hsbt (Hiroshi SHIBATA) about 12 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
というような配列の時でも警告をして通すような変更だと助かります。
Updated by nobu (Nobuyoshi Nakada) about 12 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
- hash.c (rb_hash_s_create): just warn for wrong elements now.
[ruby-dev:46440] [Bug #7300]
Updated by mame (Yusuke Endoh) almost 6 years ago
- File remove-hash-constructor-compatibility.patch remove-hash-constructor-compatibility.patch added
- Status changed from Closed to Open
- Target version changed from 2.0.0 to 2.7
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) almost 6 years ago
僕が決められるのかわかりませんが、5年以上経っているので消していいと思います。
Updated by hsbt (Hiroshi SHIBATA) almost 6 years ago
- Status changed from Open to Assigned
Updated by nobu (Nobuyoshi Nakada) over 5 years ago
- Assignee changed from nobu (Nobuyoshi Nakada) to mame (Yusuke Endoh)
Updated by mame (Yusuke Endoh) over 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]