This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Why not use refcount of c.c_filename in PyAST_FromNodeObject()
Type: enhancement Stage: resolved
Components: Interpreter Core Versions: Python 3.9
process
Status: closed Resolution: rejected
Dependencies: Superseder:
Assigned To: Nosy List: serhiy.storchaka, shihai1991, vstinner
Priority: normal Keywords:

Created on 2019-10-28 15:08 by shihai1991, last changed 2022-04-11 14:59 by admin. This issue is now closed.

Messages (7)
msg355548 - (view) Author: Hai Shi (shihai1991) * (Python triager) Date: 2019-10-28 15:08
I don't know why don't use refcount
`
/* borrowed reference */
c.c_filename = filename;
`
in https://github.com/python/cpython/blob/master/Python/ast.c#L786-L787

like
`
Py_INCREF(filename);
c.c_filename = filename;
`
in https://github.com/python/cpython/blob/master/Python/compile.c#L333
msg355549 - (view) Author: Hai Shi (shihai1991) * (Python triager) Date: 2019-10-28 15:11
Due to victor add the desc(`/* borrowed reference */`)
so noisy+ victor ;)
msg355550 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-10-28 15:32
> Due to victor add the desc(`/* borrowed reference */`)

I added the comment, but the code was always like that.

In short, borrowed references are used to simply the implementation (avoid the need to DECREF on error) and/or for efficiency.

I don't understand the purpose of this issue. The bug tracker is not the right place to ask question. If you think that it's a bug, please explain how to trigger the bug.

I suggest to close the issue.
msg355551 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2019-10-28 15:38
Because there is no need to use refcount.

And there is no need to use refcount in compile.c too.
msg355569 - (view) Author: Hai Shi (shihai1991) * (Python triager) Date: 2019-10-28 17:06
> I don't understand the purpose of this issue. The bug tracker is not the right place to ask question. If you think that it's a bug, please explain how to trigger the bug.

Hi, victor. What i want to express is that the refcount operation of filename should be same between ast.c and compile.c.

I am not sure i express it clearly. 

If the refcount of filename in ast.c is redundant as Serhiy said, maybe we should remove it? 

Hm, If we think it's fine, just keep it, i will close it soon.
And pls forgive me adding a noisy bpo again ;(
msg355594 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2019-10-28 21:35
There is no bug. We can add Py_INCREF/Py_DECREF in ast.c. We can remove Py_INCREF/Py_DECREF from compile.c. But all this would just add code churn.
msg355610 - (view) Author: Hai Shi (shihai1991) * (Python triager) Date: 2019-10-29 04:06
Thanks for your patience, Serhiy.

IMHO, i called the moving operation `clean code`: Consistent understanding, Consistent code. And the `clean code` should be a part of `enhancement`.
History
Date User Action Args
2022-04-11 14:59:22adminsetgithub: 82799
2019-10-29 04:06:36shihai1991setmessages: + msg355610
2019-10-28 21:35:40serhiy.storchakasetstatus: open -> closed
resolution: rejected
messages: + msg355594

stage: resolved
2019-10-28 17:06:36shihai1991setmessages: + msg355569
2019-10-28 15:38:40serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg355551
2019-10-28 15:32:03vstinnersetmessages: + msg355550
2019-10-28 15:11:37shihai1991setnosy: + vstinner
messages: + msg355549
2019-10-28 15:08:30shihai1991create