|
On Wed, 18 Apr 2012 15:31:10 +0200
brian.curtin <[hidden email]> wrote: > http://hg.python.org/cpython/rev/bf23a6c215f6 > changeset: 76388:bf23a6c215f6 > parent: 76385:6762b943ee59 > user: Brian Curtin <[hidden email]> > date: Wed Apr 18 08:30:51 2012 -0500 > summary: > Fix email post-commit review comments. > > Add INCREFs, fix args->kwargs, and a second args==NULL check was removed, > left over from a merger with another function. Instead, checking msg==NULL > does what that used to do in a roundabout way. I don't think INCREFs were necessary, actually. PyDict_SetItemString doesn't steal a reference. (and here we see why reference-stealing APIs are a nuisance: because you never know in advance whether a function will steal a reference or not, and you have to read the docs for each and every C API call you make) Regards Antoine. _______________________________________________ Python-Dev mailing list [hidden email] http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/lists%2B1324100855712-1801473%40n6.nabble.com |
|
On Wed, Apr 18, 2012 at 8:21 AM, Antoine Pitrou <[hidden email]> wrote:
> (and here we see why reference-stealing APIs are a nuisance: because > you never know in advance whether a function will steal a reference or > not, and you have to read the docs for each and every C API call you > make) +1 -eric _______________________________________________ Python-Dev mailing list [hidden email] http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/lists%2B1324100855712-1801473%40n6.nabble.com |
|
In reply to this post by Antoine Pitrou
Antoine Pitrou wrote:
> (and here we see why reference-stealing APIs are a nuisance: because > you never know in advance whether a function will steal a reference or > not, and you have to read the docs for each and every C API call you > make) Fortunately, they're very rare, so you don't encounter them often. Unfortunately, they're very rare, so you're all the more likely to forget about them and get bitten. Functions with ref-stealing APIs really ought to have a naming convention that makes them stand out and remind you to consult the documentation. -- Greg _______________________________________________ Python-Dev mailing list [hidden email] http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/lists%2B1324100855712-1801473%40n6.nabble.com |
|
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1 On 04/18/2012 06:48 PM, Greg Ewing wrote: > Functions with ref-stealing APIs really ought to have a naming > convention that makes them stand out and remind you to consult the > documentation. Maybe we should mandate that their names end with '_rtfm'. Tres. - -- =================================================================== Tres Seaver +1 540-429-0999 [hidden email] Palladion Software "Excellence by Design" http://palladion.com -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.10 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk+PTLwACgkQ+gerLs4ltQ5YgACg17rdlCVf8YJmGoYP2eANC8ya RhoAnimJr/5FzR59IELHAyhdXOO1c+uJ =uWHZ -----END PGP SIGNATURE----- _______________________________________________ Python-Dev mailing list [hidden email] http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/lists%2B1324100855712-1801473%40n6.nabble.com |
|
In reply to this post by Greg Ewing
On Thu, 2012-04-19 at 10:48 +1200, Greg Ewing wrote:
> Antoine Pitrou wrote: > > > (and here we see why reference-stealing APIs are a nuisance: because > > you never know in advance whether a function will steal a reference or > > not, and you have to read the docs for each and every C API call you > > make) > > Fortunately, they're very rare, so you don't encounter > them often. > > Unfortunately, they're very rare, so you're all the more > likely to forget about them and get bitten. > > Functions with ref-stealing APIs really ought to have > a naming convention that makes them stand out and remind > you to consult the documentation. attributes to gcc: http://gcc-python-plugin.readthedocs.org/en/latest/cpychecker.html#marking-functions-that-steal-references-to-their-arguments so you can write declarations like these: extern void bar(int i, PyObject *obj, int j, PyObject *other) CPYCHECKER_STEALS_REFERENCE_TO_ARG(2) CPYCHECKER_STEALS_REFERENCE_TO_ARG(4); There's a similar attribute for functions that return borrowed references: PyObject *foo(void) CPYCHECKER_RETURNS_BORROWED_REF; Perhaps we should add such attributes to the headers for Python 3.3? (perhaps with a different naming convention?) Hope this is helpful Dave _______________________________________________ Python-Dev mailing list [hidden email] http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/lists%2B1324100855712-1801473%40n6.nabble.com |
|
In reply to this post by Tres Seaver
On 19/04/12 11:22, Tres Seaver wrote:
> Maybe we should mandate that their names end with '_rtfm'. +1 -- Greg _______________________________________________ Python-Dev mailing list [hidden email] http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/lists%2B1324100855712-1801473%40n6.nabble.com |
|
In reply to this post by David Malcolm
On Wed, Apr 18, 2012 at 5:01 PM, David Malcolm <[hidden email]> wrote: On Thu, 2012-04-19 at 10:48 +1200, Greg Ewing wrote: +1 Adding these annotations and setting up a buildbot that builds using cpychecker would be a great. -gps
_______________________________________________ Python-Dev mailing list [hidden email] http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/lists%2B1324100855712-1801473%40n6.nabble.com |
|
In reply to this post by Antoine Pitrou
On Thu, Apr 19, 2012 at 12:21 AM, Antoine Pitrou <[hidden email]> wrote:
> I don't think INCREFs were necessary, actually. > PyDict_SetItemString doesn't steal a reference. Yes, I was tired when that checkin went by and my brain didn't register that the function was otherwise using borrowed refs for name and path, so it was also correct to use borrowed refs to Py_None. I should have been less cryptic and written out my full question "Should there be Py_INCREF's here?" rather than using the shorthand (i genuinely wasn't sure at the time, but that wasn't clear from what I wrote). > (and here we see why reference-stealing APIs are a nuisance: because > you never know in advance whether a function will steal a reference or > not, and you have to read the docs for each and every C API call you > make) Yeah, it would have been nice if there was an explicit hint in the API names when reference stealing was involved, but I guess it's far too late now :( Regards, Nick. -- Nick Coghlan | [hidden email] | Brisbane, Australia _______________________________________________ Python-Dev mailing list [hidden email] http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/lists%2B1324100855712-1801473%40n6.nabble.com |
|
In reply to this post by Gregory P. Smith-3
On Thu, Apr 19, 2012 at 11:04 AM, Gregory P. Smith <[hidden email]> wrote:
> +1 Adding these annotations and setting up a buildbot that builds using > cpychecker would be a great. Even without the extra annotations, running cpychecker on at least one of the buildbots might be helpful. I'm in the process of setting up a buildbot for RHEL 6, once I get it up and running normally, I'll look into what it would take to install and enable cpychecker for the builds. (Or, alternatively, I may make it a separate cron job, similar to the daily refcount leak detection run). Cheers, Nick. -- Nick Coghlan | [hidden email] | Brisbane, Australia _______________________________________________ Python-Dev mailing list [hidden email] http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/lists%2B1324100855712-1801473%40n6.nabble.com |
|
On 19 April 2012 02:20, Nick Coghlan <[hidden email]> wrote:
> On Thu, Apr 19, 2012 at 12:21 AM, Antoine Pitrou <[hidden email]> wrote: >> (and here we see why reference-stealing APIs are a nuisance: because >> you never know in advance whether a function will steal a reference or >> not, and you have to read the docs for each and every C API call you >> make) > > Yeah, it would have been nice if there was an explicit hint in the API > names when reference stealing was involved, but I guess it's far too > late now :( It's too late to change the fn names sure, but you could change the argument names in question for reference stealing apis with some kind of markup. That would make it fairly easy to write a script that did the checking for you : int PyTuple_SetItem(PyObject *p, Py_ssize_t pos, PyObject *stolen_o) Or better yet would be to mark the types : int PyTuple_SetItem(PyObject *p, Py_ssize_t pos, PyStolenObject* o) PyBorrowedObject* PyTuple_GetItem(PyObject *p, Py_ssize_t pos) PyStolenObject and PyBorrowedObject would just be typedefs to PyObject normally. But a consenting user could define PyENABLE_CHECKED_REFS before including Python.h which would given #if defined(PyENABLE_CHECKED_STOLEN_REFS) struct PyStolenObject; struct PyBorrowedObject; #define PyYesIKnowItsStolen(o) ((PyStolenObject*)o) #define PyYesIKnowItsBorrowed(o) ((PyObject*)o) #else typedef PyStolenObject PyObject; typedef PyBorrowedObject PyObject; #endif Forcing the user to use PyTuple_SetItem(p, pos, PyYesIKnowItsStolen(o)) PyObject * ref = PyYesIKnowItsBorrowed(PyTuple_GetItem(p, pos)); Or else it would fail to compile. The user could even add her own : PyStolenObject* IncRefBecauseIKnowItsStolen(PyObject* o) { PyIncRef(o); return (PyStolenObject*)o; } PyObject* IncRefBecauseIKnowItsBorrowed(PyBorrowedObject* o) { PyIncRef(o); return (PyObject*)o; } This would not require a special gcc build and would be available to anyone who wanted it. I use a similar, C++ based trick in my python extension code to avoid the whole issue of ref leaking, but I have to be careful at the point of calling the python api, having it automatic would be great. On a similar note, I have just implemented a wrapper around Python.h which runtime checks that the GIL is held around every call to the Python API or else fails very noisily. This was done because it turns out that wxPython had a ton of non-GIL calls to the API causing random sporadic set faults in our app. We now use it on several of our extensions. It doesn't require any changes to Python.h, you just need to add an include path before the python include path. Would there be any interest in this? Sam On 19 April 2012 02:25, Nick Coghlan <[hidden email]> wrote: > On Thu, Apr 19, 2012 at 11:04 AM, Gregory P. Smith <[hidden email]> wrote: >> +1 Adding these annotations and setting up a buildbot that builds using >> cpychecker would be a great. > > Even without the extra annotations, running cpychecker on at least one > of the buildbots might be helpful. > > I'm in the process of setting up a buildbot for RHEL 6, once I get it > up and running normally, I'll look into what it would take to install > and enable cpychecker for the builds. (Or, alternatively, I may make > it a separate cron job, similar to the daily refcount leak detection > run). > > Cheers, > Nick. > > -- > Nick Coghlan | [hidden email] | Brisbane, Australia > _______________________________________________ > Python-Dev mailing list > [hidden email] > http://mail.python.org/mailman/listinfo/python-dev > Unsubscribe: http://mail.python.org/mailman/options/python-dev/sam.partington%40gmail.com Python-Dev mailing list [hidden email] http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/lists%2B1324100855712-1801473%40n6.nabble.com |
|
Am 19.04.2012 12:42, schrieb Sam Partington:
> On 19 April 2012 02:20, Nick Coghlan <[hidden email]> wrote: >> On Thu, Apr 19, 2012 at 12:21 AM, Antoine Pitrou <[hidden email]> wrote: >>> (and here we see why reference-stealing APIs are a nuisance: because >>> you never know in advance whether a function will steal a reference or >>> not, and you have to read the docs for each and every C API call you >>> make) >> >> Yeah, it would have been nice if there was an explicit hint in the API >> names when reference stealing was involved, but I guess it's far too >> late now :( > > It's too late to change the fn names sure, but you could change the > argument names in question for reference stealing apis with some kind > of markup. While it may too late to change the names, it's not to late to remove these functions entirely. It will take some time, but it would be possible to add parallel APIs that neither borrow nor steal references, and have them preferred over the existing APIs. Then, with Python 4, the old APIs could go away. Regards, Martin _______________________________________________ Python-Dev mailing list [hidden email] http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/lists%2B1324100855712-1801473%40n6.nabble.com |
| Powered by Nabble | Edit this page |
