classification
Title: Use Py_NewRef & Py_XNewRef where applicable
Type: enhancement Stage: resolved
Components: Interpreter Core Versions: Python 3.10
process
Status: closed Resolution: not a bug
Dependencies: Superseder:
Assigned To: Nosy List: Yonatan Goldschmidt, vstinner
Priority: normal Keywords:

Created on 2020-11-08 00:31 by Yonatan Goldschmidt, last changed 2020-11-17 12:01 by vstinner. This issue is now closed.

Messages (4)
msg380531 - (view) Author: Yonatan Goldschmidt (Yonatan Goldschmidt) * Date: 2020-11-08 00:31
Following https://bugs.python.org/issue42262, I think it'd be nice to convert existing C code to use the more concise Py_NewRef() and Py_XNewRef() where possible. Increases readability and less bug prone. 

Opening this new issue to track the conversion.
msg380576 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-11-09 08:35
> Following https://bugs.python.org/issue42262, I think it'd be nice to convert existing C code to use the more concise Py_NewRef() and Py_XNewRef() where possible. Increases readability and less bug prone. 

In general, we don't accept changes which are only coding style changes.

I don't see how Py_NewRef() or Py_XNewRef() is less error prone. In my experience, any change is more likely to introduce new bugs than leaving the code unchanged. See https://github.com/python/cpython/pull/23170 for a concrete case of a PR which converts code to Py_NewRef() / Py_XNewRef(): the PR introduced bugs (in early versions of the PR).
msg381199 - (view) Author: Yonatan Goldschmidt (Yonatan Goldschmidt) * Date: 2020-11-17 01:04
> I don't see how Py_NewRef() or Py_XNewRef() is less error prone. In my experience, any change is more likely to introduce new bugs than leaving the code unchanged.

Agree that inserting changes opens a door to introducing bugs.

However, the "end state" of having Py_NewRef() is desired, I think. It is more concise. It is less error prone because where you use it, you literally can't miss the "increment refcount" part when stealing a reference from another source. Py_INCREF has to come before/after stealing a ref, leaving room for error, IMHO.

> In general, we don't accept changes which are only coding style changes.

Didn't know that. Well if that's the case, then obviously there is no reason to work specifically on this conversion.
msg381226 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-11-17 12:01
> Didn't know that. Well if that's the case, then obviously there is no reason to work specifically on this conversion.

If people want to use Py_NewRef(), I suggest to do while modifying the code for another reason. But not propose PRs just to use Py_NewRef().
History
Date User Action Args
2020-11-17 12:01:37vstinnersetstatus: open -> closed
resolution: not a bug
messages: + msg381226

stage: resolved
2020-11-17 01:04:11Yonatan Goldschmidtsetmessages: + msg381199
2020-11-09 08:35:14vstinnersetmessages: + msg380576
2020-11-08 00:31:36Yonatan Goldschmidtcreate