From c64ebe0530d9605d9db870604f2f17c62f7c06b7 Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Wed, 7 Oct 2026 16:55:46 +0000 Subject: [PATCH] [cpyrt] Also accept the old cppyy names for C++-side pythonization hooks Former cppyy users define static __cppyy_pythonize__ or __cppyy_explicit_pythonize__ members to pythonize their classes from C++. These were silently ignored after the rename to __cppjit_*. Accept the old names as a fallback, with the __cppjit_* names taking precedence. The lookup of the hooks lives in a dedicated "cppyy legacy API" section of Compatibility.h, guarded by the always-on CPPYY_LEGACY_API macro, so that all such compatibility shims are easy to find and to remove once there is a transition plan. Fixes root-project/root#23630. --- src/cpyrt/Compatibility.h | 56 ++++++++++++++++++++++++++++++++++++++ src/cpyrt/PyStrings.h | 3 ++ src/cpyrt/Pythonize.cxx | 21 +++++++------- test/cpp/pythonizables.cxx | 28 +++++++++++++++++++ test/cpp/pythonizables.h | 33 ++++++++++++++++++++++ test/test_pythonization.py | 3 ++ 6 files changed, 133 insertions(+), 11 deletions(-) diff --git a/src/cpyrt/Compatibility.h b/src/cpyrt/Compatibility.h index e9771d7e..90f5ad62 100644 --- a/src/cpyrt/Compatibility.h +++ b/src/cpyrt/Compatibility.h @@ -3,6 +3,7 @@ #include "Python.h" +#include "PyStrings.h" #include "cppjit_interop.h" #include @@ -43,6 +44,61 @@ inline std::string ObjToString(interop::TCppScope_t /*klass*/, return ""; } +//- cppyy legacy API ---------------------------------------------------------- +// Backwards compatibility with the API that former cppyy users have in their +// code. The macro is deliberately not a build option: it only exists so that +// the legacy code paths are easy to find, and to remove once there is a +// transition plan. Keep all such shims in this section. +#define CPPYY_LEGACY_API 1 + +// Name under which pyclass defines its explicit pythonization hook, looked up +// in its own __dict__ only (no fall-through to base classes). Prefers the +// __cppjit_* spelling over the old __cppyy_* one. Returns a borrowed reference, +// or nullptr with no error set if there is no such hook. +inline PyObject* ExplicitPythonizeHookName(PyObject* pyclass) { + // go through __dict__ instead of tp_getattr, see HasAttrDirect in + // Pythonize.cxx for the rationale + PyObject* name = nullptr; + PyObject* dct = PyObject_GetAttr(pyclass, PyStrings::gDict); + if (dct) { + PyObject* candidates[] = { + PyStrings::gExPythonize, +#if CPPYY_LEGACY_API + PyStrings::gCppyyExPythonize, +#endif + }; + for (PyObject* candidate : candidates) { + PyObject* attr = PyObject_GetItem(dct, candidate); + if (attr) { + Py_DECREF(attr); + name = candidate; + break; + } + PyErr_Clear(); + } + Py_DECREF(dct); + } + PyErr_Clear(); + return name; +} + +// Normal pythonization hook of pyclass, through regular attribute lookup (so +// it falls through to base classes). Prefers the __cppjit_* spelling over the +// old __cppyy_* one. Returns a new reference, or nullptr with no error set if +// there is no such hook. +inline PyObject* GetPythonizeHook(PyObject* pyclass) { + PyObject* func = PyObject_GetAttr(pyclass, PyStrings::gPythonize); +#if CPPYY_LEGACY_API + if (!func) { + PyErr_Clear(); + func = PyObject_GetAttr(pyclass, PyStrings::gCppyyPythonize); + } +#endif + if (!func) + PyErr_Clear(); + return func; +} + } // namespace cppjit::cpyrt::compat #endif // CPYRT_COMPATIBILITY_H diff --git a/src/cpyrt/PyStrings.h b/src/cpyrt/PyStrings.h index 6f0d3c8d..640704a6 100644 --- a/src/cpyrt/PyStrings.h +++ b/src/cpyrt/PyStrings.h @@ -78,6 +78,9 @@ namespace cppjit::cpyrt { \ X(gExPythonize, "__cppjit_explicit_pythonize__") \ X(gPythonize, "__cppjit_pythonize__") \ + /* old cppyy names, see the legacy API section in Compatibility.h */ \ + X(gCppyyExPythonize, "__cppyy_explicit_pythonize__") \ + X(gCppyyPythonize, "__cppyy_pythonize__") \ \ X(gArray, "__array__") \ X(gDType, "dtype") \ diff --git a/src/cpyrt/Pythonize.cxx b/src/cpyrt/Pythonize.cxx index 23a136dd..9a270e49 100644 --- a/src/cpyrt/Pythonize.cxx +++ b/src/cpyrt/Pythonize.cxx @@ -5,6 +5,7 @@ using namespace cppjit; #include "CPPFunction.h" #include "CPPInstance.h" #include "CPPOverload.h" +#include "Compatibility.h" #include "Converters.h" #include "CustomPyTypes.h" #include "LowLevelViews.h" @@ -2164,21 +2165,19 @@ bool cpyrt::Pythonize(PyObject* pyclass, interop::TCppScope_t scope) { // preferred if present // - normal pythonization: only called if explicit isn't present, falls // through to base classes + // The lookups live in Compatibility.h because they also accept the old + // cppyy names of these hooks. bool bUserOk = true; PyObject* res = nullptr; PyObject* pyname = PyUnicode_FromString(name.c_str()); - if (HasAttrDirect(pyclass, PyStrings::gExPythonize)) { - res = PyObject_CallMethodObjArgs(pyclass, PyStrings::gExPythonize, pyclass, - pyname, nullptr); + if (PyObject* exPythonize = compat::ExplicitPythonizeHookName(pyclass)) { + res = PyObject_CallMethodObjArgs(pyclass, exPythonize, pyclass, pyname, + nullptr); + bUserOk = (bool)res; + } else if (PyObject* func = compat::GetPythonizeHook(pyclass)) { + res = PyObject_CallFunctionObjArgs(func, pyclass, pyname, nullptr); + Py_DECREF(func); bUserOk = (bool)res; - } else { - PyObject* func = PyObject_GetAttr(pyclass, PyStrings::gPythonize); - if (func) { - res = PyObject_CallFunctionObjArgs(func, pyclass, pyname, nullptr); - Py_DECREF(func); - bUserOk = (bool)res; - } else - PyErr_Clear(); } if (!bUserOk) { Py_DECREF(pyname); diff --git a/test/cpp/pythonizables.cxx b/test/cpp/pythonizables.cxx index c6c7eade..77e89487 100644 --- a/test/cpp/pythonizables.cxx +++ b/test/cpp/pythonizables.cxx @@ -110,3 +110,31 @@ std::string pyzables::WithCallback2::klass_name{"not set"}; int pyzables::WithCallback3::get_int() { return 2 * m_int; } void pyzables::WithCallback3::set_int(int i) { m_int = 2 * i; } + +pyzables::WithCppyyCallback1::WithCppyyCallback1(int i) : m_int(i) {} + +int pyzables::WithCppyyCallback1::get_int() { return m_int; } +void pyzables::WithCppyyCallback1::set_int(int i) { m_int = i; } + +void pyzables::WithCppyyCallback1::__cppyy_explicit_pythonize__( + PyObject* klass, const std::string& name) { + replace_method_name(klass, "get_int", "GetInt"); + replace_method_name(klass, "set_int", "SetInt"); + klass_name = name; +} + +std::string pyzables::WithCppyyCallback1::klass_name{"not set"}; + +pyzables::WithCppyyCallback2::WithCppyyCallback2(int i) : m_int(i) {} + +int pyzables::WithCppyyCallback2::get_int() { return m_int; } +void pyzables::WithCppyyCallback2::set_int(int i) { m_int = i; } + +void pyzables::WithCppyyCallback2::__cppyy_pythonize__( + PyObject* klass, const std::string& name) { + replace_method_name(klass, "get_int", "GetInt"); + replace_method_name(klass, "set_int", "SetInt"); + klass_name = name; +} + +std::string pyzables::WithCppyyCallback2::klass_name{"not set"}; diff --git a/test/cpp/pythonizables.h b/test/cpp/pythonizables.h index 3ec1c6f5..8eba94fe 100644 --- a/test/cpp/pythonizables.h +++ b/test/cpp/pythonizables.h @@ -161,4 +161,37 @@ class WithCallback3 : public WithCallback2 { void set_int(int i); }; +// same as WithCallback1 and WithCallback2, but with the old cppyy names +class WithCppyyCallback1 { +public: + WithCppyyCallback1(int i); + +public: + int get_int(); + void set_int(int i); + +private: + int m_int; + +public: + static void __cppyy_explicit_pythonize__(PyObject* klass, const std::string&); + static std::string klass_name; +}; + +class WithCppyyCallback2 { +public: + WithCppyyCallback2(int i); + +public: + int get_int(); + void set_int(int i); + +private: + int m_int; + +public: + static void __cppyy_pythonize__(PyObject* klass, const std::string&); + static std::string klass_name; +}; + } // namespace pyzables diff --git a/test/test_pythonization.py b/test/test_pythonization.py index f8235460..98d0b44c 100644 --- a/test/test_pythonization.py +++ b/test/test_pythonization.py @@ -275,6 +275,9 @@ def test09_cpp_side_pythonization(self): for kls in [ cppjit.gbl.pyzables.WithCallback1, cppjit.gbl.pyzables.WithCallback2, + # backwards compatibility with the old cppyy names + cppjit.gbl.pyzables.WithCppyyCallback1, + cppjit.gbl.pyzables.WithCppyyCallback2, ]: w = kls(42) assert hasattr(w, "GetInt")