From: Pedro Alves <palves@redhat.com>
To: Tom Tromey <tom@tromey.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [RFA 00/14] add a smart pointer for PyObject*
Date: Fri, 11 Nov 2016 04:03:00 -0000 [thread overview]
Message-ID: <d91c0911-6f1f-f752-5eb0-f8877701b36e@redhat.com> (raw)
In-Reply-To: <87wpgaitam.fsf@tromey.com>
On 11/11/2016 03:34 AM, Tom Tromey wrote:
> Pedro> gdbpy_reference sort_func = module.GetAttrString ("execute_frame_filters");
>
> Pedro> instead of:
>
> Pedro> gdbpy_reference sort_func (PyObject_GetAttrString (module.get (),
> Pedro> "execute_frame_filters"));
>
> Pedro> etc.
>
> This particular one I chose not to do, on the theory that assignment
> from another gdbpy_reference does a Py_INCREF. That is, this addition
> would mean that one assignment operator claims ownership of an existing
> reference, while another assignment operator would create a new
> reference.
I think that theory is incorrect. If module.GetAttrString() returns a
gdbpy_reference, then RVO kicks in and no copy is made at all. If RVO
doesn't kick in, because the compiler fails to optimize, then it's the
move constructor that is called, since the return of
module.GetAttrString() is an rvalue in this context.
Likewise, when assigning to an existing variable:
gdbpy_reference sort_func;
...
sort_func = module.GetAttrString ("execute_frame_filters");
then it's the move assignment operator that is called.
Thus there should be no increfs here at all.
No chance of introducing bugs. I had that in mind here too,
but didn't spell it out, sorry about that.
This is like returning std::unique_ptr from functions (including
C++14 std::make_unique):
extern std::unique_ptr<...> make_something ();
std::unique_ptr<...> foo = make_something ();
The assignment only works because that's actually a move. Returning
std::unique_ptr makes sure that you don't try to assign the result of
the function call to a raw pointer directly.
The copy assignment operator is only called when you copy from
an lvalue, like e.g.:
gdbpy_reference ref1, ref2;
...
ref1 = ref2;
And in this case clearly you do want to incref.
There's another safety advantage to having functions return
gdbpy_references directly. Consider usages like:
if (module.GetAttrString ("execute_frame_filters") == NULL)
(I made that up, I assume there are cases like this, but I
didn't check.)
If GetAttrString returns a PyObject *, then this leaks. If instead
it returns a gdbpy_reference, it does not.
> Anyway, I didn't do this just because it requires more thought; and
> there were a lot of patches already; and I figured that it could be done
> later if desirable.
I hope I didn't came across as suggesting you'd do this now.
>
> Pedro> Also, somewhat related, I briefly looked at making our custom Python
> Pedro> types C++ classes before, so they could themselves hold other
> Pedro> C++ classes, std::string, etc., though I didn't find how to
> Pedro> coerce Python to use operator new with PyTypeObject types.
> Pedro> There must be some way though.
>
> I haven't looked at all into prior art for C++ Python APIs; which might
> be worth doing. One "easy" way to do it would be to store a pointer to
> a C++ object which would be "delete"d in the Python object-destruction
> callback (gross due to extra allocation of course). Another possible
> way would be placement new. But maybe there's something even nicer.
Yeah, definitely worth looking at what other APIs are doing.
Thanks,
Pedro Alves
next prev parent reply other threads:[~2016-11-11 4:03 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-11-07 6:00 Tom Tromey
2016-11-07 5:48 ` [RFA 01/14] Introduce py-ref.h Tom Tromey
2016-11-07 7:45 ` Jan Kratochvil
2016-11-07 15:48 ` Tom Tromey
2016-11-10 23:48 ` Pedro Alves
2016-11-12 17:31 ` Tom Tromey
2016-11-15 14:32 ` Pedro Alves
2016-11-16 23:18 ` Tom Tromey
2016-11-16 23:34 ` Pedro Alves
2016-11-07 5:48 ` [RFA 03/14] Use gdbpy_reference in py-type.c Tom Tromey
2016-11-07 5:48 ` [RFA 04/14] Use gdbpy_reference in gdbpy_string_to_argv Tom Tromey
2016-11-07 5:48 ` [RFA 08/14] Use gdbpy_reference in py-framefilter.c Tom Tromey
2016-11-07 5:48 ` [RFA 14/14] Use gdbpy_reference in gdbpy_lookup_symbol Tom Tromey
2016-11-07 5:48 ` [RFA 05/14] Use gdbpy_reference in py-function.c Tom Tromey
2016-11-07 5:48 ` [RFA 11/14] Use gdbpy_reference in py-prettyprint.c Tom Tromey
2016-11-07 5:48 ` [RFA 10/14] Use gdbpy_reference in call_doc_function Tom Tromey
2016-11-07 5:48 ` [RFA 13/14] Use gdbpy_reference in py-value.c Tom Tromey
2016-11-07 5:48 ` [RFA 06/14] Use gdbpy_reference in gdbpy_inferiors Tom Tromey
2016-11-07 5:48 ` [RFA 12/14] Use gdbpy_reference in python.c Tom Tromey
2016-11-07 5:48 ` [RFA 07/14] Use gdbpy_reference in gdbpy_breakpoints Tom Tromey
2016-11-07 5:56 ` [RFA 02/14] Change event code to use gdbpy_reference Tom Tromey
2016-11-11 0:09 ` Pedro Alves
2016-11-11 0:51 ` Pedro Alves
2016-11-07 5:57 ` [RFA 09/14] Use gdbpy_reference in py-linetable.c Tom Tromey
2016-11-08 4:07 ` [RFA 00/14] add a smart pointer for PyObject* Tom Tromey
2016-11-11 1:18 ` Pedro Alves
2016-11-11 3:34 ` Tom Tromey
2016-11-11 4:03 ` Pedro Alves [this message]
2016-11-11 5:49 ` Tom Tromey
2016-11-12 17:11 ` Tom Tromey
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d91c0911-6f1f-f752-5eb0-f8877701b36e@redhat.com \
--to=palves@redhat.com \
--cc=gdb-patches@sourceware.org \
--cc=tom@tromey.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox