clear_weakref() in Objects/weakrefobject.c is Confusing
まだ誰も着手していません。
- 主要言語
- Python
- スター
- 77.2k
- フォーク
- 36k
- 平均マージ
- 1日 9時間
- マージ済み PR(30日)
- 558
説明
(inspired by https://github.com/python/cpython/pull/95302#discussion_r931261376)
In Objects/weakrefobject.c, the clear_weakref() function is used to:
- pop the
PyWeakReferenceobject off its referenced object'stp_weaklist(a linked list) - clear (but not call) the callback, if any
However, in the places it is used, it isn't clear what it's doing. When adding a new use of clear_weakref() in one of my PRs it was particularly confusing to a reviewer what the relationship was between clear_weakref() and an object's weakref list (i.e. tp_weaklist).
While we could simply rename it to "remove_and_clear_weakref", it may make sense to do the following:
- move the linked-list part to a separate
remove_weakref()that has an explicitPyWeakReference **parameter - (hence, remove the linked list parts from
clear_weakref()) - add
remove_weakref_from_referent()which callsGET_WEAKREFS_LISTPTR(self->wr_object)and callsremove_weakref()on it - add a call to it everywhere we already call
clear_weakref()
Other observations:
- currently we aren't reseting
self->hashorself->vectorcallto NULL inclear_weakref() - we may be leaking weakref objects
PyObject_ClearWeakRefs()when the weakref doesn't have a callback
コントリビューションガイド
はじめの一歩
- issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
- 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
- リポジトリをフォークし、ブランチを切って変更します。
- issue 番号を参照したプルリクエストを送ります。
調査の方向性
Objects/weakrefobject.c で clear_weakref()、その呼び出し元、および tp_weaklist の処理を読むことから始めてください。提案されている remove_weakref() と remove_weakref_from_referent() の分離を、リンク先の PR の議論と比較し、その後、hash、vectorcall、および weakref-leak の可能性に関する指摘を調査してください。混乱を招いている責務と残っている懸念のスコープが決定され、レビュー済みになれば完了です。
索引モデルが issue の本文から書いたものです。
評価
- 技術スタック
- c, python
- 領域
- backend
- issue の種類
- リファクタリング
- 難易度
- 4/5
- 見積もり時間
- 3〜5日
- 活発さ
- 停滞
- 明瞭さ
- おおむね明確
- 初心者へのやさしさ
- 35/100