classification
Title: Py_(X)SETREF macros
Type: enhancement Stage:
Components: Extension Modules, Interpreter Core Versions: Python 3.0, Python 2.6
process
Status: closed Resolution: rejected
Dependencies: Superseder:
Assigned To: Nosy List: _doublep, belopolsky, benjamin.peterson, exarkun, jcea, mark.dickinson, pitrou, rhettinger
Priority: normal Keywords: patch

Created on 2008-06-11 19:13 by pitrou, last changed 2010-04-08 21:18 by rhettinger. This issue is now closed.

Files
File name Uploaded Description Edit
py_setref.patch pitrou, 2008-06-11 19:13
py_setref.patch pitrou, 2008-06-28 19:36
Messages (12)
msg68009 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2008-06-11 19:13
This is an implementation of the Py_SETREF and Py_XSETREF macros
proposed in http://mail.python.org/pipermail/python-dev/2008-May/079862.html

As an example, I added a few conversions among the extension modules.
msg68010 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2008-06-11 19:28
FWIW, I also wanted to propose for Py_INCREF(op) to evaluate as (op), so
that it can be used as return or assignment value, e.g.:
    return Py_INCREF(result);
or:
    self->var = Py_INCREF(obj);

but it's perhaps a bit more controversial.
msg68012 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2008-06-11 19:57
Benjamin, the patch is against py3k, also it might also apply cleanly on
trunk...
msg68013 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-06-11 19:59
Sorry for the confusion. It seems to me this sort of thing would be
useful in 2.6, too, so I marked it.
msg68161 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2008-06-13 14:19
A comment on the patch:

Since object.h may be included from C++ extensions, you should not use a 
C++ keyword "new" as a variable name.
msg68898 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2008-06-28 19:36
This new patch avoids using temporary variables named "new", it also
adopts the "do { ... } while (0)" idiom for definition of the macros.
msg68904 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2008-06-28 20:11
-1 on the new macros.  The mnemonic doesn't work for me and the example 
code fragments are to my eyes less readable than before.  These add to 
the learning curve for reading and writing C extensions and provide 
nearly zero benefits.  

Assigning from an INCREF feels weird.  It is somewhat at odds with our 
coding style where we tend to stick with dirt simple C, trying to put 
operations on different lines rather than combining too many step in a 
single line.
msg68906 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2008-06-28 20:31
Le samedi 28 juin 2008 à 20:12 +0000, Raymond Hettinger a écrit :
> Raymond Hettinger <rhettinger@users.sourceforge.net> added the comment:
> 
> -1 on the new macros.  The mnemonic doesn't work for me and the example 
> code fragments are to my eyes less readable than before.  These add to 
> the learning curve for reading and writing C extensions and provide 
> nearly zero benefits.  

They might not be ideal, but I think they can be helpful to avoid
writing incorrect code out of laziness. There is already Py_CLEAR with
similar purposes.

> Assigning from an INCREF feels weird.  It is somewhat at odds with our 
> coding style where we tend to stick with dirt simple C, trying to put 
> operations on different lines rather than combining too many step in a 
> single line.

Ok.
msg70798 - (view) Author: Paul Pogonyshev (_doublep) Date: 2008-08-06 19:33
Just to note, I proposed similar macro on the mailing list under the
name Py_ASSIGN.
msg102636 - (view) Author: Jean-Paul Calderone (exarkun) * (Python committer) Date: 2010-04-08 17:44
The name suggests a different behavior to me - I assumed it would set the reference count to a specific value.  Maybe this is the kind of thing Raymond had in mind when he said "The mnemonic doesn't work for me".
msg102644 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-04-08 20:37
Py_ASSIGN could be a better name, but given the enthusiasm generated by this proposal, I think we might just as well close the issue.
msg102645 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2010-04-08 21:03
I agree with Raymond about the Py_INCREF.  I could see more uses for this macro without the Py_INCREF, especially for things like in-place arithmetic operations.  The following pattern appears a lot in code like Objects/longobject.c:

Old code:

/* add one to x */
temp = PyNumber_Add(x, one);
Py_DECREF(x);
x = temp;
if (x == NULL)
    return NULL;


With a non-INCREF version of Py_SETREF:

/* add one to x */
Py_SETREF(x, PyNumber_Add(x, one));
if (x == NULL)
    return NULL;
History
Date User Action Args
2010-04-08 21:18:04rhettingersetstatus: open -> closed
resolution: rejected
2010-04-08 21:03:04mark.dickinsonsetnosy: + mark.dickinson
messages: + msg102645
2010-04-08 20:37:09pitrousetmessages: + msg102644
2010-04-08 17:44:12exarkunsetnosy: + exarkun
messages: + msg102636
2008-09-02 17:16:18jceasetnosy: + jcea
2008-08-06 19:33:20_doublepsetnosy: + _doublep
messages: + msg70798
2008-06-28 20:31:53pitrousetmessages: + msg68906
2008-06-28 20:11:59rhettingersetnosy: + rhettinger
messages: + msg68904
2008-06-28 19:36:54pitrousetfiles: + py_setref.patch
messages: + msg68898
2008-06-13 14:19:08belopolskysetnosy: + belopolsky
messages: + msg68161
2008-06-11 20:00:05benjamin.petersonsetmessages: + msg68013
versions: + Python 3.0
2008-06-11 19:57:09pitrousetmessages: + msg68012
2008-06-11 19:52:02benjamin.petersonsetnosy: + benjamin.peterson
type: enhancement
versions: + Python 2.6, - Python 3.0
2008-06-11 19:28:20pitrousetmessages: + msg68010
2008-06-11 19:13:51pitroucreate