Use Py_NewRef & Py_XNewRef where applicable

[Jongy]

BPO	42287
Nosy	@vstinner, @Jongy

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2020-11-17.12:01:37.010>
created_at = <Date 2020-11-08.00:31:36.255>
labels = ['interpreter-core', 'type-feature', 'invalid', '3.10']
title = 'Use Py_NewRef & Py_XNewRef where applicable'
updated_at = <Date 2020-11-17.12:01:37.009>
user = 'https://github.com/Jongy'

bugs.python.org fields:

activity = <Date 2020-11-17.12:01:37.009>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2020-11-17.12:01:37.010>
closer = 'vstinner'
components = ['Interpreter Core']
creation = <Date 2020-11-08.00:31:36.255>
creator = 'Yonatan Goldschmidt'
dependencies = []
files = []
hgrepos = []
issue_num = 42287
keywords = []
message_count = 4.0
messages = ['380531', '380576', '381199', '381226']
nosy_count = 2.0
nosy_names = ['vstinner', 'Yonatan Goldschmidt']
pr_nums = []
priority = 'normal'
resolution = 'not a bug'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue42287'
versions = ['Python 3.10']

[Jongy]

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.

[vstinner]

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 #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).

[Jongy]

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.

[vstinner]

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().

[erlend-aasland]

Superseded by #99300

[vstinner]

Well, I change my mind since 2020 :-) I used Py_NewRef() in more and more code and now I can see that it makes the code easier to understand and easier to maintain.