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 :
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:
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()?
On Sun, Aug 14, 2011 at 19:36, Benjamin Peterson <benjamin@python.org>wrote:
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.
On Mon, Aug 15, 2011 at 12:34 PM, Brett Cannon <brett@python.org> wrote:
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
On Mon, Aug 15, 2011 at 10:17 PM, Antoine Pitrou <solipsis@pitrou.net> wrote:
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:
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 Aug 15, 2011, at 05:46 AM, Raymond Hettinger 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. -Barry
On Mon, Aug 15, 2011 at 11:59 PM, Antoine Pitrou <solipsis@pitrou.net> 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. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
On Aug 16, 2011, at 08:32 AM, Nick Coghlan wrote:
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:
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)
On Tue, Aug 16, 2011 at 9:13 AM, Ethan Furman <ethan@stoneleaf.us> wrote:
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:
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:
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
On Thu, Aug 11, 2011 at 3:02 AM, Antoine Pitrou <solipsis@pitrou.net> wrote: ..
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.
On Thu, Aug 11, 2011 at 00:02, Antoine Pitrou <solipsis@pitrou.net> wrote:
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()?
On Sun, Aug 14, 2011 at 19:36, Benjamin Peterson <benjamin@python.org>wrote:
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.
On Mon, Aug 15, 2011 at 12:34 PM, Brett Cannon <brett@python.org> wrote:
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
On Mon, Aug 15, 2011 at 10:17 PM, Antoine Pitrou <solipsis@pitrou.net> wrote:
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:
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 Aug 15, 2011, at 05:46 AM, Raymond Hettinger 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. -Barry
On Mon, Aug 15, 2011 at 11:59 PM, Antoine Pitrou <solipsis@pitrou.net> 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. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
On Aug 16, 2011, at 08:32 AM, Nick Coghlan wrote:
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:
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)
On Tue, Aug 16, 2011 at 9:13 AM, Ethan Furman <ethan@stoneleaf.us> wrote:
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:
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:
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
On Thu, Aug 11, 2011 at 3:02 AM, Antoine Pitrou <solipsis@pitrou.net> wrote: ..
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