Bug #13004: rb_get_kwargs はバグってないでしょうか？

• Status: Closed
• Assignee: -
• Target version: -
• Backport: 2.1: REQUIRED, 2.2: DONE, 2.3: DONE

[metanest (Makoto Kishimoto)]

きしもとです

拡張ライブラリで rb_get_kwargs を使おうとしたところ、いくつか変に
感じた点がありましたので、確認をお願いしたくこちらにメイルします。
もしバグでしたらチケットにまとめます。

(1) valuesを渡すと、全て変更（初期化）される。

extension(.ja).rdoc には、

                       ...  If an optional key is not present in
 +keyword_hash+, the corresponding element in +values+ is not changed.

                                         ..．省略可能キーワー
 ドがない場合は，values中の対応する要素は変更されません．

とあるが、コードでは、

1866     if (values) {
1867         for (j = 0; j < required + optional; j++) {
1868             values[j] = Qundef;
1869         }
1870     }

となっていて、valuesが非NULLであれば一律に Qundef で初期化している。

(2) "unknown keyword" ArgumentError を raise しそこねることがある。

該当部分のコードはこうなっていて、

1898     if (!rest && keyword_hash) {
1899         if (RHASH_SIZE(keyword_hash) > (unsigned int)j) {
1900             unknown_keyword_error(keyword_hash, table, required+optional);
1901         }
1902     }

ここで j には認識されたキーワード引数の個数が入っていて、valuesが
NULLの時にはkeyword_hashの中身が変更されず、恐らくその意図通りの
動作になるが、valuesが非NULLの場合はkeyword_hashからエントリが
除かれるため、認識できなかった引数があってもこのチェックを通り
抜けてしまうことがある（extract_kwargマクロも参照）。

1855 #define extract_kwarg(keyword, val) \
1856     (key = (st_data_t)(keyword), values ? \
1857      st_delete(rb_hash_tbl_raw(keyword_hash), &key, (val)) : \
1858      st_lookup(rb_hash_tbl_raw(keyword_hash), key, (val)))

(3) 使用されない要素はkeyword_hashに残されるだけで、別には保存されない。

extension(.ja).rdoc には、

 If +optional+ is negative, rest of +keyword_hash+ are stored in the
 next to optional +values+ as a new Hash, ...

 keyword_hashに使用されない要素がある場合は，optionalが負なら
 新しいHashとして省略可能引数の次に保存されますが，...

とあるが、該当するコードが見つからない。またもし、そのような動作を
するのであれば、ドキュメントのこの関数に関する記述の冒頭部に、
valuesのサイズは必ず「required+optional + 1」以上でなければ
ならないと書く必要があるように思う。

[nobu (Nobuyoshi Nakada)]

• Status changed from Open to Closed

Applied in changeset r56980.

---

extension.rdoc: fix rb_get_kwargs [ci skip]

• doc/extension.rdoc: [DOC] optional keyword arguments are
  defaulted to Qundef. ignored keys are kept in the hash but a
  new Hash is not created. [ruby-dev:49893] [Bug #13004]

[nobu (Nobuyoshi Nakada)]

• Description updated (diff)
• Backport changed from 2.1: UNKNOWN, 2.2: UNKNOWN, 2.3: UNKNOWN to 2.1: REQUIRED, 2.2: REQUIRED, 2.3: REQUIRED

[usa (Usaku NAKAMURA)]

• Backport changed from 2.1: REQUIRED, 2.2: REQUIRED, 2.3: REQUIRED to 2.1: REQUIRED, 2.2: DONE, 2.3: REQUIRED

ruby_2_2 r57215 merged revision(s) 56980,56981.

[nagachika (Tomoyuki Chikanaga)]

• Backport changed from 2.1: REQUIRED, 2.2: DONE, 2.3: REQUIRED to 2.1: REQUIRED, 2.2: DONE, 2.3: DONE

ruby_2_3 r57346 merged revision(s) 56980,56981.