Re: [Python-Dev] cpython: Add Py_RETURN_NOTIMPLEMENTED macro. Fixes #12724.
Le Thu, 11 Aug 2011 03:34:37 +0200, brian.curtin <python-checkins@python.org> a écrit :
http://hg.python.org/cpython/rev/77a65b078852 changeset: 71809:77a65b078852 parent: 71803:1b4fae183da3 user: Brian Curtin <brian@python.org> date: Wed Aug 10 20:05:21 2011 -0500 summary: Add Py_RETURN_NOTIMPLEMENTED macro. Fixes #12724.
It would sound more useful to have a generic Py_RETURN() macro rather than some specific forms for each and every common object. Regards Antoine.
On Thu, Aug 11, 2011 at 00:02, Antoine Pitrou <solipsis@pitrou.net> wrote:
Le Thu, 11 Aug 2011 03:34:37 +0200, brian.curtin <python-checkins@python.org> a écrit :
http://hg.python.org/cpython/rev/77a65b078852 changeset: 71809:77a65b078852 parent: 71803:1b4fae183da3 user: Brian Curtin <brian@python.org> date: Wed Aug 10 20:05:21 2011 -0500 summary: Add Py_RETURN_NOTIMPLEMENTED macro. Fixes #12724.
It would sound more useful to have a generic Py_RETURN() macro rather than some specific forms for each and every common object.
Since the macro is rather generic, sure, but the name should probably be better since it doesn't necessarily convene the fact that a INCREF has occurred. So maybe Py_INCREF_RETURN()?
Regards
Antoine.
_______________________________________________ Python-Dev mailing list Python-Dev@python.org http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/brett%40python.org
2011/8/14 Brett Cannon <brett@python.org>:
On Thu, Aug 11, 2011 at 00:02, Antoine Pitrou <solipsis@pitrou.net> wrote:
Le Thu, 11 Aug 2011 03:34:37 +0200, brian.curtin <python-checkins@python.org> a écrit :
http://hg.python.org/cpython/rev/77a65b078852 changeset: 71809:77a65b078852 parent: 71803:1b4fae183da3 user: Brian Curtin <brian@python.org> date: Wed Aug 10 20:05:21 2011 -0500 summary: Add Py_RETURN_NOTIMPLEMENTED macro. Fixes #12724.
It would sound more useful to have a generic Py_RETURN() macro rather than some specific forms for each and every common object.
Since the macro is rather generic, sure, but the name should probably be better since it doesn't necessarily convene the fact that a INCREF has occurred. So maybe Py_INCREF_RETURN()?
That nearly nullifies the space saving. I think that fact that it's a macro at all conveys that it does something else aside from "return x;". -- Regards, Benjamin
On Sun, Aug 14, 2011 at 19:36, Benjamin Peterson <benjamin@python.org>wrote:
2011/8/14 Brett Cannon <brett@python.org>:
On Thu, Aug 11, 2011 at 00:02, Antoine Pitrou <solipsis@pitrou.net>
Le Thu, 11 Aug 2011 03:34:37 +0200, brian.curtin <python-checkins@python.org> a écrit :
http://hg.python.org/cpython/rev/77a65b078852 changeset: 71809:77a65b078852 parent: 71803:1b4fae183da3 user: Brian Curtin <brian@python.org> date: Wed Aug 10 20:05:21 2011 -0500 summary: Add Py_RETURN_NOTIMPLEMENTED macro. Fixes #12724.
It would sound more useful to have a generic Py_RETURN() macro rather
wrote: than
some specific forms for each and every common object.
Since the macro is rather generic, sure, but the name should probably be better since it doesn't necessarily convene the fact that a INCREF has occurred. So maybe Py_INCREF_RETURN()?
That nearly nullifies the space saving. I think that fact that it's a macro at all conveys that it does something else aside from "return x;".
This is C code; space savings went out the window along with gc a long time ago. Yes, being a macro helps differentiate semantics that a longer name is probably not needed.
-- Regards, Benjamin
2011/8/14 Brett Cannon <brett@python.org>:
On Sun, Aug 14, 2011 at 19:36, Benjamin Peterson <benjamin@python.org> wrote:
2011/8/14 Brett Cannon <brett@python.org>:
On Thu, Aug 11, 2011 at 00:02, Antoine Pitrou <solipsis@pitrou.net> wrote:
Le Thu, 11 Aug 2011 03:34:37 +0200, brian.curtin <python-checkins@python.org> a écrit :
http://hg.python.org/cpython/rev/77a65b078852 changeset: 71809:77a65b078852 parent: 71803:1b4fae183da3 user: Brian Curtin <brian@python.org> date: Wed Aug 10 20:05:21 2011 -0500 summary: Add Py_RETURN_NOTIMPLEMENTED macro. Fixes #12724.
It would sound more useful to have a generic Py_RETURN() macro rather than some specific forms for each and every common object.
Since the macro is rather generic, sure, but the name should probably be better since it doesn't necessarily convene the fact that a INCREF has occurred. So maybe Py_INCREF_RETURN()?
That nearly nullifies the space saving. I think that fact that it's a macro at all conveys that it does something else aside from "return x;".
This is C code; space savings went out the window along with gc a long time ago.
I'm hanging on to it by a hair. :) -- Regards, Benjamin
On Mon, Aug 15, 2011 at 12:34 PM, Brett Cannon <brett@python.org> wrote:
On Thu, Aug 11, 2011 at 00:02, Antoine Pitrou <solipsis@pitrou.net> wrote:
It would sound more useful to have a generic Py_RETURN() macro rather than some specific forms for each and every common object.
Since the macro is rather generic, sure, but the name should probably be better since it doesn't necessarily convene the fact that a INCREF has occurred. So maybe Py_INCREF_RETURN()?
Aside from None and NotImplemented, do we really do the straight incref-and-return all that often? While I was initially attracted to the idea of a generic macro, the more I thought about it, the more it seemed like a magnet for reference leak bugs. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
Le lundi 15 août 2011 à 13:16 +1000, Nick Coghlan a écrit :
On Mon, Aug 15, 2011 at 12:34 PM, Brett Cannon <brett@python.org> wrote:
On Thu, Aug 11, 2011 at 00:02, Antoine Pitrou <solipsis@pitrou.net> wrote:
It would sound more useful to have a generic Py_RETURN() macro rather than some specific forms for each and every common object.
Since the macro is rather generic, sure, but the name should probably be better since it doesn't necessarily convene the fact that a INCREF has occurred. So maybe Py_INCREF_RETURN()?
Aside from None and NotImplemented, do we really do the straight incref-and-return all that often?
AFAICT, often with True and False: x = (some condition) ? Py_True : Py_False; Py_INCREF(x); return x; Regards Antoine.
On Mon, Aug 15, 2011 at 10:17 PM, Antoine Pitrou <solipsis@pitrou.net> wrote:
AFAICT, often with True and False:
x = (some condition) ? Py_True : Py_False; Py_INCREF(x); return x;
And that's an idiom that works better with a Py_RETURN macro than it would separate macros: Py_RETURN(cond ? Py_True : Py_False); OK, I'm persuaded that "Py_RETURN(Py_NotImplemented);" would be a better way to handle this change: +1 Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
On Aug 15, 2011, at 5:35 AM, Nick Coghlan wrote:
On Mon, Aug 15, 2011 at 10:17 PM, Antoine Pitrou <solipsis@pitrou.net> wrote:
AFAICT, often with True and False:
x = (some condition) ? Py_True : Py_False; Py_INCREF(x); return x;
And that's an idiom that works better with a Py_RETURN macro than it would separate macros:
Py_RETURN(cond ? Py_True : Py_False);
OK, I'm persuaded that "Py_RETURN(Py_NotImplemented);" would be a better way to handle this change: +1
I don't think that is worth it. There is some value to keeping the API consistent with the style that has been used in the past. So, I vote for Py_RETURN_NOTIMPLEMENTED. There's no real need to factor this any further. It's not hard and not important enough to introduce a new variation on return macros. Adding another return style makes the C API harder to learn and remember. If we we're starting from scratch, Py_RETURN(obj) would make sense. But we're not starting from scratch, so we should stick with the precedents. Raymond
On Mon, 15 Aug 2011 05:46:12 -0700 Raymond Hettinger <raymond.hettinger@gmail.com> wrote:
I don't think that is worth it. There is some value to keeping the API consistent with the style that has been used in the past. So, I vote for Py_RETURN_NOTIMPLEMENTED. There's no real need to factor this any further. It's not hard and not important enough to introduce a new variation on return macros.
Why is Py_RETURN_NOTIMPLEMENTED important at all? Regards Antoine.
On Aug 15, 2011, at 05:46 AM, Raymond Hettinger wrote:
I don't think that is worth it. There is some value to keeping the API consistent with the style that has been used in the past. So, I vote for Py_RETURN_NOTIMPLEMENTED. There's no real need to factor this any further. It's not hard and not important enough to introduce a new variation on return macros. Adding another return style makes the C API harder to learn and remember. If we we're starting from scratch, Py_RETURN(obj) would make sense. But we're not starting from scratch, so we should stick with the precedents.
I can see the small value in the convenience, but I tend to agree with Raymond here. I think we have to be careful about not descending into macro obfuscation world. -Barry
On Mon, 15 Aug 2011 09:49:43 -0400 Barry Warsaw <barry@python.org> wrote:
On Aug 15, 2011, at 05:46 AM, Raymond Hettinger wrote:
I don't think that is worth it. There is some value to keeping the API consistent with the style that has been used in the past. So, I vote for Py_RETURN_NOTIMPLEMENTED. There's no real need to factor this any further. It's not hard and not important enough to introduce a new variation on return macros. Adding another return style makes the C API harder to learn and remember. If we we're starting from scratch, Py_RETURN(obj) would make sense. But we're not starting from scratch, so we should stick with the precedents.
I can see the small value in the convenience, but I tend to agree with Raymond here. I think we have to be careful about not descending into macro obfuscation world.
How is Py_RETURN(Py_NotImplemented) more obfuscated than Py_RETURN_NOTIMPLEMENTED ???
On Mon, Aug 15, 2011 at 11:59 PM, Antoine Pitrou <solipsis@pitrou.net> wrote:
On Mon, 15 Aug 2011 09:49:43 -0400 Barry Warsaw <barry@python.org> wrote:
I can see the small value in the convenience, but I tend to agree with Raymond here. I think we have to be careful about not descending into macro obfuscation world.
How is Py_RETURN(Py_NotImplemented) more obfuscated than Py_RETURN_NOTIMPLEMENTED ???
Indeed, this entire discussion was started by the extension of the Py_RETURN_NONE idiom to also adopt Py_RETURN_NOTIMPLEMENTED. If the idiom is to be extended at all, why stop there? Why not cover the Py_RETURN_TRUE and Py_RETURN_FALSE cases as well? Or, we can add exactly one new macro that covers all 3 cases, and others besides. I haven't encountered any complaints about people failing to understand the difference between "return Py_None;" and "Py_RETURN_NONE;" and see no major reason why "return x;" vs "Py_RETURN(x);" would be significantly more confusing. Based on this thread, there are actually two options I'd be fine with: 1. Just revert it and leave Py_RETURN_NONE as a special snowflake 2. Properly generalise the incref-and-return idiom via a Py_RETURN macro Incrementally increasing complexity by adding a second instance of the dedicated macro approach is precisely what we *shouldn't* be doing. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
On Aug 16, 2011, at 08:32 AM, Nick Coghlan wrote:
Indeed, this entire discussion was started by the extension of the Py_RETURN_NONE idiom to also adopt Py_RETURN_NOTIMPLEMENTED.
If the idiom is to be extended at all, why stop there? Why not cover the Py_RETURN_TRUE and Py_RETURN_FALSE cases as well?
Or, we can add exactly one new macro that covers all 3 cases, and others besides. I haven't encountered any complaints about people failing to understand the difference between "return Py_None;" and "Py_RETURN_NONE;" and see no major reason why "return x;" vs "Py_RETURN(x);" would be significantly more confusing.
Based on this thread, there are actually two options I'd be fine with: 1. Just revert it and leave Py_RETURN_NONE as a special snowflake 2. Properly generalise the incref-and-return idiom via a Py_RETURN macro
Incrementally increasing complexity by adding a second instance of the dedicated macro approach is precisely what we *shouldn't* be doing.
My problem with Py_RETURN(x) is that it's not clear that it also does an incref, and without that, I think it's *more* confusing to use rather than just writing it out explicitly, Py_RETURN_NONE's historic existence notwithstanding. So I'd opt for #1, unless we can agree on a better color for the bikeshed. -Barry
On Mon, Aug 15, 2011 at 3:43 PM, Barry Warsaw <barry@python.org> wrote:
On Aug 16, 2011, at 08:32 AM, Nick Coghlan wrote:
Indeed, this entire discussion was started by the extension of the Py_RETURN_NONE idiom to also adopt Py_RETURN_NOTIMPLEMENTED.
If the idiom is to be extended at all, why stop there? Why not cover the Py_RETURN_TRUE and Py_RETURN_FALSE cases as well?
Or, we can add exactly one new macro that covers all 3 cases, and others besides. I haven't encountered any complaints about people failing to understand the difference between "return Py_None;" and "Py_RETURN_NONE;" and see no major reason why "return x;" vs "Py_RETURN(x);" would be significantly more confusing.
Based on this thread, there are actually two options I'd be fine with: 1. Just revert it and leave Py_RETURN_NONE as a special snowflake 2. Properly generalise the incref-and-return idiom via a Py_RETURN macro
Incrementally increasing complexity by adding a second instance of the dedicated macro approach is precisely what we *shouldn't* be doing.
My problem with Py_RETURN(x) is that it's not clear that it also does an incref, and without that, I think it's *more* confusing to use rather than just writing it out explicitly, Py_RETURN_NONE's historic existence notwithstanding.
So I'd opt for #1, unless we can agree on a better color for the bikeshed.
I dunno; if it *didn't* do an INCREF it would be a pretty pointless macro (just expanding to "return x") and I like reducing the clutter of a very common idiom. So I favor #2. -- --Guido van Rossum (python.org/~guido)
Barry Warsaw wrote:
On Aug 16, 2011, at 08:32 AM, Nick Coghlan wrote:
Based on this thread, there are actually two options I'd be fine with: 1. Just revert it and leave Py_RETURN_NONE as a special snowflake 2. Properly generalise the incref-and-return idiom via a Py_RETURN macro
Incrementally increasing complexity by adding a second instance of the dedicated macro approach is precisely what we *shouldn't* be doing.
My problem with Py_RETURN(x) is that it's not clear that it also does an incref, and without that, I think it's *more* confusing to use rather than just writing it out explicitly, Py_RETURN_NONE's historic existence notwithstanding.
So I'd opt for #1, unless we can agree on a better color for the bikeshed.
My apologies if this is just noise, but are there RETURN macros that don't do an INCREF? ~Ethan~
On Tue, Aug 16, 2011 at 9:13 AM, Ethan Furman <ethan@stoneleaf.us> wrote:
Barry Warsaw wrote:
So I'd opt for #1, unless we can agree on a better color for the bikeshed.
My apologies if this is just noise, but are there RETURN macros that don't do an INCREF?
No, Py_RETURN_NONE is the only previous example, and it was added to simplify the very common idiom of: Py_INCREF(Py_None); return Py_None; It was added originally because it helped to avoid *two* common bugs: return Py_None; # segfault waiting to happen return NULL; # Just plain wrong, but not picked up until tests are run and hence irritating I'd say NotImplemented is the second most common instance of that kind of direct incref-and-return (since operator methods need to return it to correctly support type coercion), although, as Antoine noted, Py_True and Py_False would be up there as well. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
On Mon, Aug 15, 2011 at 4:39 PM, Nick Coghlan <ncoghlan@gmail.com> wrote:
On Tue, Aug 16, 2011 at 9:13 AM, Ethan Furman <ethan@stoneleaf.us> wrote:
Barry Warsaw wrote:
So I'd opt for #1, unless we can agree on a better color for the bikeshed.
My apologies if this is just noise, but are there RETURN macros that don't do an INCREF?
No, Py_RETURN_NONE is the only previous example, and it was added to simplify the very common idiom of:
Py_INCREF(Py_None); return Py_None;
It was added originally because it helped to avoid *two* common bugs:
return Py_None; # segfault waiting to happen
return NULL; # Just plain wrong, but not picked up until tests are run and hence irritating
I'd say NotImplemented is the second most common instance of that kind of direct incref-and-return (since operator methods need to return it to correctly support type coercion), although, as Antoine noted, Py_True and Py_False would be up there as well.
I betcha if you extend your search to "return <variable>" preceded by "INCREF(variable)" you'll find a whole lot more examples. :-) -- --Guido van Rossum (python.org/~guido)
On 8/15/2011 9:49 AM, Barry Warsaw wrote:
On Aug 15, 2011, at 05:46 AM, Raymond Hettinger wrote:
I don't think that is worth it. There is some value to keeping the API consistent with the style that has been used in the past. So, I vote for Py_RETURN_NOTIMPLEMENTED. There's no real need to factor this any further. It's not hard and not important enough to introduce a new variation on return macros. Adding another return style makes the C API harder to learn and remember. If we we're starting from scratch, Py_RETURN(obj) would make sense. But we're not starting from scratch, so we should stick with the precedents.
I can see the small value in the convenience, but I tend to agree with Raymond here. I think we have to be careful about not descending into macro obfuscation world.
Coming fresh to the C-API, as I partly am, I would rather have exactly 1 generally useful macro that increments the refcount of an object and returns it. To me, multiple special-case, seldom-used macros are a better example of 'macro obfuscation'. -- Terry Jan Reedy
Nick Coghlan, 15.08.2011 14:35:
On Mon, Aug 15, 2011 at 10:17 PM, Antoine Pitrou<solipsis@pitrou.net> wrote:
AFAICT, often with True and False:
x = (some condition) ? Py_True : Py_False; Py_INCREF(x); return x;
And that's an idiom that works better with a Py_RETURN macro than it would separate macros:
Py_RETURN(cond ? Py_True : Py_False);
And that would do what exactly? Duplicate the evaluation of the condition? Stefan
On Mon, 15 Aug 2011 15:21:43 +0200 Stefan Behnel <stefan_ml@behnel.de> wrote:
Nick Coghlan, 15.08.2011 14:35:
On Mon, Aug 15, 2011 at 10:17 PM, Antoine Pitrou<solipsis@pitrou.net> wrote:
AFAICT, often with True and False:
x = (some condition) ? Py_True : Py_False; Py_INCREF(x); return x;
And that's an idiom that works better with a Py_RETURN macro than it would separate macros:
Py_RETURN(cond ? Py_True : Py_False);
And that would do what exactly? Duplicate the evaluation of the condition?
You don't need to. #define Py_RETURN (x) do { \ PyObject *_tmp = (x); \ Py_INCREF(_tmp); \ return _tmp; \ } while(0)
On Thu, Aug 11, 2011 at 3:02 AM, Antoine Pitrou <solipsis@pitrou.net> wrote: ..
Add Py_RETURN_NOTIMPLEMENTED macro. Fixes #12724.
It would sound more useful to have a generic Py_RETURN() macro rather than some specific forms for each and every common object.
Just my $0.02: I occasionally wish we had Py_RETURN_BOOL(1/0) instead of Py_RETURN_TRUE/FALSE, but I feel proposed Py_RETURN() is too ambiguous and should be called Py_RETURN_SINGLETON() or Py_RETURN_NEWREF(). Longer spelling, however makes it less attractive. Overall, I am -1 on Py_RETURN(). Introducing the second obvious way to spell Py_RETURN_NONE/TRUE/FALSE will clutter the API and novices may be misled into always using Py_RETURN(x) instead of return x attracting reference leaks.
participants (11)
-
Alexander Belopolsky -
Antoine Pitrou -
Barry Warsaw -
Benjamin Peterson -
Brett Cannon -
Ethan Furman -
Guido van Rossum -
Nick Coghlan -
Raymond Hettinger -
Stefan Behnel -
Terry Reedy