[RFA,01/14] Introduce py-ref.h
Commit Message
This patch introduces class gdbpy_reference, which is a sort of smart
pointer that owns a single Python reference to a PyObject. This class
acts a bit like unique_ptr, but also a bit like shared_ptr (in that
copies do what you might expect); I considered going solely with
unique_ptr but it seemed quite strange to have a unique_ptr that
actually manages a shared resource.
Subsequent patches use this new class to simplify logic in the Python
layer.
2016-11-06 Tom Tromey <tom@tromey.com>
* python/py-ref.h: New file.
---
gdb/ChangeLog | 4 ++
gdb/python/py-ref.h | 142 ++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 146 insertions(+)
create mode 100644 gdb/python/py-ref.h
Comments
On Mon, 07 Nov 2016 06:47:23 +0100, Tom Tromey wrote:
> + /* Copy another instance. */
> + gdbpy_reference &operator= (const gdbpy_reference &other)
> + {
> + Py_XDECREF (m_obj);
> + m_obj = other.m_obj;
> + if (m_obj != NULL)
> + Py_INCREF (m_obj);
> + return *this;
> + }
> +
> + /* Transfer ownership from another instance. */
> + gdbpy_reference &operator= (gdbpy_reference &&other)
> + {
> + Py_XDECREF (m_obj);
> + m_obj = other.m_obj;
> + other.m_obj = NULL;
> + return *this;
> + }
These two methods stale-reference / deallocate during self-assignments:
gdbpyref = gdbpyref;
gdbpyref = std::move (gdbpyref);
std::unique_ptr<> and std::shared_ptr<> behave correctly in such cases.
Jan
>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
Jan> These two methods stale-reference / deallocate during self-assignments:
Jan> gdbpyref = gdbpyref;
Jan> gdbpyref = std::move (gdbpyref);
Jan> std::unique_ptr<> and std::shared_ptr<> behave correctly in such cases.
I'll fix them.
Or maybe remove them, I can't remember if I ended up needing these.
Tom
On 11/07/2016 05:47 AM, Tom Tromey wrote:
> +/* An instance of this class holds a reference to a PyObject. When
> + the object is destroyed, the PyObject is decref'd.
> +
> + Normally an instance is constructed using a PyObject*. This sort
> + of initialization lets this class manage the lifetime of that
> + reference.
> +
> + Assignment and copy construction will make a new reference as
> + appropriate. Assignment from a plain PyObject* is disallowed to
> + avoid confusion about whether this acquires a new reference;
> + instead use the "reset" method -- which, like the PyObject*
> + constructor, transfers ownership.
I think this should say something about supporting explicitly
wrapping a NULL PyObject *, and how to check whether there's
a managed object.
> +*/
> +class gdbpy_reference
> +{
> + public:
> +
> + /* Create a new NULL instance. */
> + gdbpy_reference ()
> + : m_obj (NULL)
> + {
> + }
> +
> + /* Create a new NULL instance. */
> + gdbpy_reference (nullptr_t)
> + : m_obj (NULL)
> + {
> + }
I don't think this overload is needed, since there's no
ambiguity without it. If you don't provide it, initializing
with nullptr selects the one below, with the same result.
Otherwise, shouldn't this overload be explicit too?
> +
> + /* Create a new instance. OBJ is a reference, management of which
> + is now transferred to this class. */
> + explicit gdbpy_reference (PyObject *obj)
> + : m_obj (obj)
> + {
> + }
> + /* Equality. */
> + bool operator== (const PyObject *other) const
> + {
> + return m_obj == other;
> + }
> +
> + /* Inequality. */
> + bool operator!= (const PyObject *other) const
> + {
> + return m_obj != other;
> + }
This supports:
gdbpy_reference == PyObject *
Seems to me we should support these too:
gdbpy_reference == gdbpy_reference
PyObject * == gdbpy_reference
Thanks,
Pedro Alves
@@ -1,3 +1,7 @@
+2016-11-06 Tom Tromey <tom@tromey.com>
+
+ * python/py-ref.h: New file.
+
2016-10-29 Manish Goregaokar <manish@mozilla.com>
* rust-exp.y: Parse `sizeof(exp)` as `UNOP_SIZEOF`
new file mode 100644
@@ -0,0 +1,142 @@
+/* Python reference-holding class
+
+ Copyright (C) 2016 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#ifndef GDB_PYTHON_REF_H
+#define GDB_PYTHON_REF_H
+
+/* An instance of this class holds a reference to a PyObject. When
+ the object is destroyed, the PyObject is decref'd.
+
+ Normally an instance is constructed using a PyObject*. This sort
+ of initialization lets this class manage the lifetime of that
+ reference.
+
+ Assignment and copy construction will make a new reference as
+ appropriate. Assignment from a plain PyObject* is disallowed to
+ avoid confusion about whether this acquires a new reference;
+ instead use the "reset" method -- which, like the PyObject*
+ constructor, transfers ownership.
+*/
+class gdbpy_reference
+{
+ public:
+
+ /* Create a new NULL instance. */
+ gdbpy_reference ()
+ : m_obj (NULL)
+ {
+ }
+
+ /* Create a new NULL instance. */
+ gdbpy_reference (nullptr_t)
+ : m_obj (NULL)
+ {
+ }
+
+ /* Create a new instance. OBJ is a reference, management of which
+ is now transferred to this class. */
+ explicit gdbpy_reference (PyObject *obj)
+ : m_obj (obj)
+ {
+ }
+
+ /* Copy another instance. */
+ gdbpy_reference (const gdbpy_reference &other)
+ : m_obj (other.m_obj)
+ {
+ if (m_obj != NULL)
+ Py_INCREF (m_obj);
+ }
+
+ /* Transfer ownership from another instance. */
+ gdbpy_reference (gdbpy_reference &&other)
+ : m_obj (other.m_obj)
+ {
+ other.m_obj = NULL;
+ }
+
+ /* Destroy this instance. */
+ ~gdbpy_reference ()
+ {
+ Py_XDECREF (m_obj);
+ }
+
+ /* Copy another instance. */
+ gdbpy_reference &operator= (const gdbpy_reference &other)
+ {
+ Py_XDECREF (m_obj);
+ m_obj = other.m_obj;
+ if (m_obj != NULL)
+ Py_INCREF (m_obj);
+ return *this;
+ }
+
+ /* Transfer ownership from another instance. */
+ gdbpy_reference &operator= (gdbpy_reference &&other)
+ {
+ Py_XDECREF (m_obj);
+ m_obj = other.m_obj;
+ other.m_obj = NULL;
+ return *this;
+ }
+
+ /* Change this instance's referent. OBJ is a reference, management
+ of which is now transferred to this class. */
+ void reset (PyObject *obj)
+ {
+ Py_XDECREF (m_obj);
+ m_obj = obj;
+ }
+
+ /* Return this instance's referent. In Python terms this is a
+ borrowed pointer. */
+ PyObject *get ()
+ {
+ return m_obj;
+ }
+
+ /* Return this instance's referent, and stop managing this
+ reference. The caller is now responsible for the ownership of
+ the reference. */
+ PyObject *release ()
+ {
+ PyObject *result = m_obj;
+
+ m_obj = NULL;
+ return result;
+ }
+
+ /* Equality. */
+ bool operator== (const PyObject *other) const
+ {
+ return m_obj == other;
+ }
+
+ /* Inequality. */
+ bool operator!= (const PyObject *other) const
+ {
+ return m_obj != other;
+ }
+
+ private:
+
+ PyObject *m_obj;
+};
+
+#endif /* GDB_PYTHON_REF_H */