python / python/cpython

clear_weakref() in Objects/weakrefobject.c is Confusing

Aberta
#95,348 2 comentários 0 reações 0 responsáveis Ver no GitHub

Ninguém assumiu esta issue ainda.

interpreter-core type-feature
Linguagem predominante
Python
Estrelas
77.2k
Forks
36k
Métricas de merge de PRs
Métricas de PR pendentes

Descrição

(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

Guia de contribuição

Abrir o guia de contribuição

Primeiros passos

  1. Leia a issue inteira e depois o guia de contribuição do projeto.
  2. Comente na issue dizendo que vai assumir — evita que duas pessoas façam o mesmo trabalho.
  3. Faça um fork do repositório e trabalhe em uma branch.
  4. Abra um pull request que referencie o número da issue.

Direção de pesquisa

Comece em Objects/weakrefobject.c lendo clear_weakref(), seus chamadores e o tratamento de tp_weaklist. Compare a separação proposta entre remove_weakref() e remove_weakref_from_referent() com a discussão do PR vinculado e, em seguida, investigue as observações sobre hash, vectorcall e possíveis weakref-leak. Está concluído quando o escopo das responsabilidades confusas e das preocupações restantes tiver sido decidido e revisado.

Escrita pelo modelo de indexação a partir do texto da issue.

Avaliação

Stack de tecnologia
c, python
Domínio
backend
Tipo de issue
Refatoração
Dificuldade
4/5
Tempo estimado
3-5 dias
Status de atividade
Estagnada
Clareza
Razoavelmente clara
Facilidade para iniciantes
35/100

Receba novas issues na sua caixa de entrada

Um resumo curto de issues do GitHub para quem está começando.