From: Tom Tromey <tom@tromey.com>
To: Pedro Alves <palves@redhat.com>
Cc: Tom Tromey <tom@tromey.com>, gdb-patches@sourceware.org
Subject: Re: [RFA 00/14] add a smart pointer for PyObject*
Date: Fri, 11 Nov 2016 03:34:00 -0000 [thread overview]
Message-ID: <87wpgaitam.fsf@tromey.com> (raw)
In-Reply-To: <7f51de8d-912c-01d9-815f-85cc3c351f0c@redhat.com> (Pedro Alves's message of "Fri, 11 Nov 2016 01:18:39 +0000")
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
Pedro> I did wonder about shortening the name to "gdbpy_ref". That's how I end
Pedro> up reading "gdbpy_reference" in my mind after seeing it so many
Pedro> times. :-) You already picked a shorter name as header file
Pedro> name -- py-ref.h -- and that's typed way less often. :-)
Pedro> Anyway, certainly not very important. Just mentioning in case
Pedro> you had already considered but didn't know what others would think.
I wouldn't mind. I just erred on the side of verbosity.
Pedro> I wonder whether it'd be desirable to add more methods to
Pedro> gdbpy_reference, so you'd have code like:
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.
So, to avoid the confusion I made the PyObject* constructor explicit,
and I required the use of .reset(...) to claim ownership.
I'm open to basically any spelling of any of this. I just thought this
approach was most likely to avoid bugs.
Pedro> And maybe add some C++ classes that inherit from gdbpy_reference for
Pedro> specific Python types (e.g python tuple or list), which would provide
Pedro> methods that would only make sense for those types.
Pedro> Did you consider things like these? Or maybe you did but thought
Pedro> they'd obfuscate?
I did consider subclassing and also adding some convenience methods.
Other similar ideas came to mind as well, like providing wrapper
functions for all the Python APIs to try to ensure ownership sanity --
basically implementing David Malcolm's reference-checker work as a bunch
of templates or so.
I do think this would help; while the current approach definitely
improves the situation, it also leaves some bug-prone behavior. For
example, PyTuple_SET_ITEM steals a reference, so calls end up looking
like:
PyTuple_SET_ITEM (py_arg_tuple.get (), 0, py_value_obj.release ());
... using .get() rather than .release() would cause a bug (probably a
crash).
If we had a wrapper we could make calls look like:
mumble_tuple_set_item (py_arg_tuple, 0, py_value_obj);
... with the first argument being unwrapped using .get() and the third
with an implicit incref (or require std::move there). (An equivalent
using member functions could also be done.)
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.
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.
Tom
next prev parent reply other threads:[~2016-11-11 3:34 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 10/14] Use gdbpy_reference in call_doc_function Tom Tromey
2016-11-07 5:48 ` [RFA 07/14] Use gdbpy_reference in gdbpy_breakpoints Tom Tromey
2016-11-07 5:48 ` [RFA 12/14] Use gdbpy_reference in python.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 13/14] Use gdbpy_reference in py-value.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 03/14] Use gdbpy_reference in py-type.c 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 11/14] Use gdbpy_reference in py-prettyprint.c 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 14/14] Use gdbpy_reference in gdbpy_lookup_symbol Tom Tromey
2016-11-07 5:48 ` [RFA 08/14] Use gdbpy_reference in py-framefilter.c 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 [this message]
2016-11-11 4:03 ` Pedro Alves
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=87wpgaitam.fsf@tromey.com \
--to=tom@tromey.com \
--cc=gdb-patches@sourceware.org \
--cc=palves@redhat.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