From b0cb54c0b3a564ec2bc2fc9f34e157daa4e72534 Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Sat, 28 Jun 2025 00:59:59 +0200 Subject: [PATCH] [Python] Avoid hack in setting `__reduce__` attribute of CPPInstance The CPPIntance type is immutable, and new attributes can't be set: ```txt import cppyy >>> setattr(cppyy._backend.CPPInstance, "test", 10) Traceback (most recent call last): File "", line 1, in setattr(cppyy._backend.CPPInstance, "test", 10) ~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ TypeError: cannot set 'test' attribute of immutable type 'cppyy.CPPInstance' >>> ``` In our pythonizations, we still want to monkey-patch a `__reduce__` method anyway, and to make Python not complain we to a hack by setting the attribute with the C Python API using `PyObject_GenericSetAttr`. This function doesn't normally do any safety checks, and our monkey patching works. However, the Python debug build is not having any of that, thanks to a new assert that was added a year ago: https://github.com/python/cpython/blame/c419af9e277bea7dd78f4defefc752fe93b0b8ec/Objects/object.c#L1921 To make the ROOT Python interface work with debug builds of the Python interpreter, we therefore have to implement adding the reduce method properly. This commit suggests to achieve this by defining the `__reduce__` method in the immutable CPPInstance type within CPyCppyy, but in such a way that its implementation can be routed to ROOT via a private API. --- .../pyroot/cppyy/CPyCppyy/src/CPPInstance.cxx | 17 ++++++++++++ .../pyroot/cppyy/CPyCppyy/src/CPPInstance.h | 6 +++++ .../ROOT/_pythonization/_cppinstance.py | 5 ++-- .../pythonizations/src/CPPInstancePyz.cxx | 26 +++---------------- .../pythonizations/src/PyROOTModule.cxx | 2 +- 5 files changed, 29 insertions(+), 27 deletions(-) diff --git a/bindings/pyroot/cppyy/CPyCppyy/src/CPPInstance.cxx b/bindings/pyroot/cppyy/CPyCppyy/src/CPPInstance.cxx index f9f30d26d950c..98bcbad25d625 100644 --- a/bindings/pyroot/cppyy/CPyCppyy/src/CPPInstance.cxx +++ b/bindings/pyroot/cppyy/CPyCppyy/src/CPPInstance.cxx @@ -400,6 +400,21 @@ static PySequenceMethods op_as_sequence = { 0, // sq_inplace_repeat }; +std::function &CPPInstance::ReduceMethod() { + static std::function reducer; + return reducer; +} + +PyObject *op_reduce(PyObject *self, PyObject * /*args*/) +{ + auto &reducer = CPPInstance::ReduceMethod(); + if (!reducer) { + PyErr_SetString(PyExc_NotImplementedError, ""); + return nullptr; + } + return reducer(self); +} + //---------------------------------------------------------------------------- static PyMethodDef op_methods[] = { @@ -409,6 +424,8 @@ static PyMethodDef op_methods[] = { (char*)"dispatch to selected overload"}, {(char*)"__smartptr__", (PyCFunction)op_get_smartptr, METH_NOARGS, (char*)"get associated smart pointer, if any"}, + {(char*)"__reduce__", (PyCFunction)op_reduce, METH_NOARGS, + (char*)"reduce method for serialization"}, {(char*)"__reshape__", (PyCFunction)op_reshape, METH_O, (char*)"cast pointer to 1D array type"}, {(char*)nullptr, nullptr, 0, nullptr} diff --git a/bindings/pyroot/cppyy/CPyCppyy/src/CPPInstance.h b/bindings/pyroot/cppyy/CPyCppyy/src/CPPInstance.h index 85012f8ba5391..795632024ab22 100644 --- a/bindings/pyroot/cppyy/CPyCppyy/src/CPPInstance.h +++ b/bindings/pyroot/cppyy/CPyCppyy/src/CPPInstance.h @@ -15,6 +15,7 @@ #include "CallContext.h" // for Parameter // Standard +#include #include #include @@ -83,6 +84,11 @@ class CPPInstance { void CastToArray(Py_ssize_t sz); Py_ssize_t ArrayLength(); +// implementation of the __reduce__ method: doesn't wrap any function by +// default but can be re-assigned by libraries that add C++ object +// serialization support, like ROOT + static std::function &ReduceMethod(); + private: void CreateExtension(); void* GetExtendedObject(); diff --git a/bindings/pyroot/pythonizations/python/ROOT/_pythonization/_cppinstance.py b/bindings/pyroot/pythonizations/python/ROOT/_pythonization/_cppinstance.py index b5775fc7b0e6a..a16dedc295b79 100644 --- a/bindings/pyroot/pythonizations/python/ROOT/_pythonization/_cppinstance.py +++ b/bindings/pyroot/pythonizations/python/ROOT/_pythonization/_cppinstance.py @@ -8,13 +8,12 @@ # For the list of contributors see $ROOTSYS/README/CREDITS. # ################################################################################ + def pythonize_cppinstance(): - import cppyy from ROOT.libROOTPythonizations import AddCPPInstancePickling - klass = cppyy._backend.CPPInstance + AddCPPInstancePickling() - AddCPPInstancePickling(klass) # Instant pythonization (executed at `import ROOT` time), no need of a # decorator. CPPInstance is the base for cppyy instance proxies and thus needs diff --git a/bindings/pyroot/pythonizations/src/CPPInstancePyz.cxx b/bindings/pyroot/pythonizations/src/CPPInstancePyz.cxx index a7f3d19280e1e..021e0834776b3 100644 --- a/bindings/pyroot/pythonizations/src/CPPInstancePyz.cxx +++ b/bindings/pyroot/pythonizations/src/CPPInstancePyz.cxx @@ -62,7 +62,7 @@ PyObject *PyROOT::CPPInstanceExpand(PyObject * /*self*/, PyObject *args) /// Turn the object proxy instance into a character stream and return for /// pickle, together with the callable object that can restore the stream /// into the object proxy instance. -PyObject *op_reduce(PyObject *self, PyObject * /*args*/) +PyObject *op_reduce(PyObject *self) { // keep a borrowed reference around to the callable function for expanding; // because it is borrowed, it means that there can be no pickling during the @@ -120,28 +120,8 @@ PyObject *op_reduce(PyObject *self, PyObject * /*args*/) /// /// The C++ function op_reduce defined above is wrapped in a Python method /// so that it can be injected in CPPInstance -PyObject *PyROOT::AddCPPInstancePickling(PyObject * /*self*/, PyObject *args) +PyObject *PyROOT::AddCPPInstancePickling(PyObject * /*self*/, PyObject * /*args*/) { - PyObject *pyclass = PyTuple_GetItem(args, 0); - - const char *attr = "__reduce__"; - - PyMethodDef *pdef = new PyMethodDef(); - pdef->ml_name = attr; - pdef->ml_meth = (PyCFunction)op_reduce; - pdef->ml_flags = METH_NOARGS; - pdef->ml_doc = nullptr; - - PyObject *func = PyCFunction_New(pdef, nullptr); - PyObject *method = CustomInstanceMethod_New(func, nullptr, pyclass); - - // here PyObject_GenericSetAttr is used because CPPInstance does not allow - // attribute assignment using PyObject_SetAttr - // for more info refer to: - // https://bitbucket.org/wlav/cppyy/issues/110/user-defined-classes-in-c-dont-seem-to-be - PyObject_GenericSetAttr(pyclass, PyUnicode_FromString(attr), method); - Py_DECREF(method); - Py_DECREF(func); - + CPPInstance::ReduceMethod() = op_reduce; Py_RETURN_NONE; } diff --git a/bindings/pyroot/pythonizations/src/PyROOTModule.cxx b/bindings/pyroot/pythonizations/src/PyROOTModule.cxx index f1e4602572256..3bae603d6714b 100644 --- a/bindings/pyroot/pythonizations/src/PyROOTModule.cxx +++ b/bindings/pyroot/pythonizations/src/PyROOTModule.cxx @@ -126,7 +126,7 @@ PyObject *PyObjRefCounterAsStdAny(PyObject * /*self*/, PyObject *args) // Methods offered by the interface static PyMethodDef gPyROOTMethods[] = { - {(char *)"AddCPPInstancePickling", (PyCFunction)PyROOT::AddCPPInstancePickling, METH_VARARGS, + {(char *)"AddCPPInstancePickling", (PyCFunction)PyROOT::AddCPPInstancePickling, METH_NOARGS, (char *)"Add a custom pickling mechanism for Cppyy Python proxy objects"}, {(char *)"GetBranchAttr", (PyCFunction)PyROOT::GetBranchAttr, METH_VARARGS, (char *)"Allow to access branches as tree attributes"},