[issue28999] Use Py_RETURN_NONE and like

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Thank you Raymond. I didn't expect to get an approval from you and were ready 
to withdraw the patch. I myself were just +0 on pushing these changes.

Omitted changes in Modules/xx*.c and added few changes which Coccinelle missed.

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28999] Use Py_RETURN_NONE and like

[Roundup Robot]

Roundup Robot added the comment:

New changeset 2c724f45f23f by Serhiy Storchaka in branch 'default':
Issue #28999: Use Py_RETURN_NONE, Py_RETURN_TRUE and Py_RETURN_FALSE wherever
https://hg.python.org/cpython/rev/2c724f45f23f

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28999] Use Py_RETURN_NONE and like

[Roundup Robot]

Roundup Robot added the comment:

New changeset df87db35833e by Serhiy Storchaka in branch 'default':
Issue #28999: Use Py_RETURN_NONE, Py_RETURN_TRUE and Py_RETURN_FALSE wherever
https://hg.python.org/cpython/rev/df87db35833e

--
nosy: +python-dev

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28999] Use Py_RETURN_NONE and like

[INADA Naoki]

INADA Naoki added the comment:

Oh, I feel three LGTMs are positive signal.
As I commented on ML, I think "ask forgiveness than permission" is
realistic approach for patches like this.
But it's up to you.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28999] Use Py_RETURN_NONE and like

[Raymond Hettinger]

Raymond Hettinger added the comment:

I believe this is a safe change.  All of the replaced pairs on consecutive 
lines so there are no intervening operations.

For Modules/xxsubtype.c, I think the code should remain as-is.  It is intended 
to be the simplest possible example of how to make a subtype and its clarity is 
reduced by requiring that someone learn the macro.  That said, it would be 
reasonable to add a comment that this pair could also have been coded as 
Py_RETURN_NONE (i.e. use it as a teachable moment).

--
assignee:  -> serhiy.storchaka
nosy: +rhettinger

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28999] Use Py_RETURN_NONE and like

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

That is why I haven't pushed the patch. Thanks you for your LGTMs Victor and 
Inada, but this is not enough.

In general I think that it is better to use Py_RETURN_NONE than 
"Py_INCREF(Py_None); return Py_None;". Some replacements already was done in 
the past, and some currently reviewed patches include also such changes. This 
can distract attention from the main purpose of patches. I think that pushing 
all changes in one big commit will make less harm than add small changes in 
other patches here and there. Concinelle proved his reliability and it is easy 
to check these changes.

I'll push this patch if Raymond or other more conservative core developers 
accept it (or selected parts of it).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28999] Use Py_RETURN_NONE and like

[INADA Naoki]

INADA Naoki added the comment:

While patch looks safe, some developer may dislike such a large patch
without fixing real issue.

Anyway, Coccinelle seems very interesting tool for refactoring.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28999] Use Py_RETURN_NONE and like

[INADA Naoki]

INADA Naoki added the comment:

LGTM

--
nosy: +inada.naoki

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28999] Use Py_RETURN_NONE and like

[STINNER Victor]

STINNER Victor added the comment:

py_return.patch LGTM.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28999] Use Py_RETURN_NONE and like

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Resulting patch is huge. I don't think it can be pushed.

--
keywords: +patch
stage:  -> patch review
Added file: http://bugs.python.org/file45937/py_return.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28999] Use Py_RETURN_NONE and like

[Serhiy Storchaka]

New submission from Serhiy Storchaka:

In the comment to issue28765 patch Victor suggested to replace 
"Py_INCREF(Py_None); return Py_None;" with "Py_RETURN_NONE;".

Here is a Coccinelle [1] semantic patch that replaces all returns of new 
references to None, True or False with macros Py_RETURN_NONE, Py_RETURN_TRUE or 
Py_RETURN_FALSE correspondingly.

[1] http://coccinelle.lip6.fr/

--
components: Extension Modules, Interpreter Core
files: py_return.cocci
messages: 283480
nosy: haypo, serhiy.storchaka
priority: normal
severity: normal
status: open
title: Use Py_RETURN_NONE and like
type: enhancement
versions: Python 3.7
Added file: http://bugs.python.org/file45936/py_return.cocci

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com