python / python/cpython

clear_weakref() in Objects/weakrefobject.c is Confusing

オープン
#95,348 コメント 2 件 リアクション 0 件 担当者 0 名 GitHub で見る

まだ誰も着手していません。

interpreter-core type-feature
主要言語
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 PyWeakReference object off its referenced object's tp_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 explicit PyWeakReference ** parameter
  • (hence, remove the linked list parts from clear_weakref())
  • add remove_weakref_from_referent() which calls GET_WEAKREFS_LISTPTR(self->wr_object) and calls remove_weakref() on it
  • add a call to it everywhere we already call clear_weakref()

Other observations:

  • currently we aren't reseting self->hash or self->vectorcall to NULL in clear_weakref()
  • we may be leaking weakref objects PyObject_ClearWeakRefs() when the weakref doesn't have a callback

コントリビューションガイド

コントリビューションガイドを開く

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. 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

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。