From 75f61be6b767760d291e96a4120f7284dfea9920 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 24 Mar 2025 20:40:34 -0700 Subject: [PATCH 01/18] Backport of https://github.com/google/pybind11clif/pull/30099 --- CMakeLists.txt | 1 + include/pybind11/attr.h | 1 + .../detail/function_record_pyobject.h | 207 ++++++++++++++++++ include/pybind11/detail/internals.h | 30 +-- include/pybind11/functional.h | 11 +- include/pybind11/pybind11.h | 73 +++--- include/pybind11/pytypes.h | 12 + tests/extra_python_package/test_files.py | 1 + tests/test_pickling.py | 45 +++- 9 files changed, 293 insertions(+), 88 deletions(-) create mode 100644 include/pybind11/detail/function_record_pyobject.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 2161c92693..486fd3ef62 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -134,6 +134,7 @@ set(PYBIND11_HEADERS include/pybind11/detail/cpp_conduit.h include/pybind11/detail/descr.h include/pybind11/detail/dynamic_raw_ptr_cast_if_possible.h + include/pybind11/detail/function_record_pyobject.h include/pybind11/detail/init.h include/pybind11/detail/internals.h include/pybind11/detail/native_enum_data.h diff --git a/include/pybind11/attr.h b/include/pybind11/attr.h index ef2ca1709c..651949bfc1 100644 --- a/include/pybind11/attr.h +++ b/include/pybind11/attr.h @@ -192,6 +192,7 @@ struct argument_record { /// Internal data structure which holds metadata about a bound function (signature, overloads, /// etc.) +#define PYBIND11_DETAIL_FUNCTION_RECORD_ABI_ID "v1" // PLEASE UPDATE if the struct is changed. struct function_record { function_record() : is_constructor(false), is_new_style_constructor(false), is_stateless(false), diff --git a/include/pybind11/detail/function_record_pyobject.h b/include/pybind11/detail/function_record_pyobject.h new file mode 100644 index 0000000000..72c8c6c3b8 --- /dev/null +++ b/include/pybind11/detail/function_record_pyobject.h @@ -0,0 +1,207 @@ +// Copyright (c) 2024-2025 The Pybind Development Team. +// All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +// For background see the description of PR google/pybind11clif#30099. + +#pragma once + +#include +#include +#include + +#include "common.h" + +#include + +PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) +PYBIND11_NAMESPACE_BEGIN(detail) + +struct function_record_PyObject { + PyObject_HEAD + function_record *cpp_func_rec; +}; + +PYBIND11_NAMESPACE_BEGIN(function_record_PyTypeObject_methods) + +PyObject *tp_new_impl(PyTypeObject *type, PyObject *args, PyObject *kwds); +PyObject *tp_alloc_impl(PyTypeObject *type, Py_ssize_t nitems); +int tp_init_impl(PyObject *self, PyObject *args, PyObject *kwds); +void tp_dealloc_impl(PyObject *self); +void tp_free_impl(void *self); + +static PyObject *reduce_ex_impl(PyObject *self, PyObject *, PyObject *); + +PYBIND11_WARNING_PUSH +#if defined(__GNUC__) && __GNUC__ >= 8 +PYBIND11_WARNING_DISABLE_GCC("-Wcast-function-type") +#endif +static PyMethodDef tp_methods_impl[] + = {{"__reduce_ex__", (PyCFunction) reduce_ex_impl, METH_VARARGS | METH_KEYWORDS, nullptr}, + {nullptr, nullptr, 0, nullptr}}; +PYBIND11_WARNING_POP + +// Note that this name is versioned. +constexpr char tp_name_impl[] + = "pybind11_detail_function_record_" PYBIND11_DETAIL_FUNCTION_RECORD_ABI_ID + "_" PYBIND11_PLATFORM_ABI_ID; + +PYBIND11_NAMESPACE_END(function_record_PyTypeObject_methods) + +// Designated initializers are a C++20 feature: +// https://en.cppreference.com/w/cpp/language/aggregate_initialization#Designated_initializers +// MSVC rejects them unless /std:c++20 is used (error code C7555). +PYBIND11_WARNING_PUSH +PYBIND11_WARNING_DISABLE_CLANG("-Wmissing-field-initializers") +#if defined(__GNUC__) && __GNUC__ >= 8 +PYBIND11_WARNING_DISABLE_GCC("-Wmissing-field-initializers") +#endif +static PyTypeObject function_record_PyTypeObject = { + PyVarObject_HEAD_INIT(nullptr, 0) + /* const char *tp_name */ function_record_PyTypeObject_methods::tp_name_impl, + /* Py_ssize_t tp_basicsize */ sizeof(function_record_PyObject), + /* Py_ssize_t tp_itemsize */ 0, + /* destructor tp_dealloc */ function_record_PyTypeObject_methods::tp_dealloc_impl, + /* Py_ssize_t tp_vectorcall_offset */ 0, + /* getattrfunc tp_getattr */ nullptr, + /* setattrfunc tp_setattr */ nullptr, + /* PyAsyncMethods *tp_as_async */ nullptr, + /* reprfunc tp_repr */ nullptr, + /* PyNumberMethods *tp_as_number */ nullptr, + /* PySequenceMethods *tp_as_sequence */ nullptr, + /* PyMappingMethods *tp_as_mapping */ nullptr, + /* hashfunc tp_hash */ nullptr, + /* ternaryfunc tp_call */ nullptr, + /* reprfunc tp_str */ nullptr, + /* getattrofunc tp_getattro */ nullptr, + /* setattrofunc tp_setattro */ nullptr, + /* PyBufferProcs *tp_as_buffer */ nullptr, + /* unsigned long tp_flags */ Py_TPFLAGS_DEFAULT, + /* const char *tp_doc */ nullptr, + /* traverseproc tp_traverse */ nullptr, + /* inquiry tp_clear */ nullptr, + /* richcmpfunc tp_richcompare */ nullptr, + /* Py_ssize_t tp_weaklistoffset */ 0, + /* getiterfunc tp_iter */ nullptr, + /* iternextfunc tp_iternext */ nullptr, + /* struct PyMethodDef *tp_methods */ function_record_PyTypeObject_methods::tp_methods_impl, + /* struct PyMemberDef *tp_members */ nullptr, + /* struct PyGetSetDef *tp_getset */ nullptr, + /* struct _typeobject *tp_base */ nullptr, + /* PyObject *tp_dict */ nullptr, + /* descrgetfunc tp_descr_get */ nullptr, + /* descrsetfunc tp_descr_set */ nullptr, + /* Py_ssize_t tp_dictoffset */ 0, + /* initproc tp_init */ function_record_PyTypeObject_methods::tp_init_impl, + /* allocfunc tp_alloc */ function_record_PyTypeObject_methods::tp_alloc_impl, + /* newfunc tp_new */ function_record_PyTypeObject_methods::tp_new_impl, + /* freefunc tp_free */ function_record_PyTypeObject_methods::tp_free_impl, + /* inquiry tp_is_gc */ nullptr, + /* PyObject *tp_bases */ nullptr, + /* PyObject *tp_mro */ nullptr, + /* PyObject *tp_cache */ nullptr, + /* PyObject *tp_subclasses */ nullptr, + /* PyObject *tp_weaklist */ nullptr, + /* destructor tp_del */ nullptr, + /* unsigned int tp_version_tag */ 0, + /* destructor tp_finalize */ nullptr, +#if PY_VERSION_HEX >= 0x03080000 + /* vectorcallfunc tp_vectorcall */ nullptr, +#endif +}; +PYBIND11_WARNING_POP + +static bool function_record_PyTypeObject_PyType_Ready_first_call = true; + +inline void function_record_PyTypeObject_PyType_Ready() { + if (function_record_PyTypeObject_PyType_Ready_first_call) { + if (PyType_Ready(&function_record_PyTypeObject) < 0) { + throw error_already_set(); + } + function_record_PyTypeObject_PyType_Ready_first_call = false; + } +} + +inline bool is_function_record_PyObject(PyObject *obj) { + if (PyType_Check(obj) != 0) { + return false; + } + PyTypeObject *obj_type = Py_TYPE(obj); + // Fast path (pointer comparison). + if (obj_type == &function_record_PyTypeObject) { + return true; + } + // This works across extension modules. Note that tp_name is versioned. + if (strcmp(obj_type->tp_name, function_record_PyTypeObject.tp_name) == 0) { + return true; + } + return false; +} + +inline function_record *function_record_ptr_from_PyObject(PyObject *obj) { + if (is_function_record_PyObject(obj)) { + return ((detail::function_record_PyObject *) obj)->cpp_func_rec; + } + return nullptr; +} + +inline object function_record_PyObject_New() { + auto *py_func_rec = PyObject_New(function_record_PyObject, &function_record_PyTypeObject); + if (py_func_rec == nullptr) { + throw error_already_set(); + } + py_func_rec->cpp_func_rec = nullptr; // For clarity/purity. Redundant in practice. + return reinterpret_steal((PyObject *) py_func_rec); +} + +PYBIND11_NAMESPACE_BEGIN(function_record_PyTypeObject_methods) + +// Guard against accidents & oversights, in particular when porting to future Python versions. +inline PyObject *tp_new_impl(PyTypeObject *, PyObject *, PyObject *) { + pybind11_fail("UNEXPECTED CALL OF function_record_PyTypeObject_methods::tp_new_impl"); + // return nullptr; // Unreachable. +} + +inline PyObject *tp_alloc_impl(PyTypeObject *, Py_ssize_t) { + pybind11_fail("UNEXPECTED CALL OF function_record_PyTypeObject_methods::tp_alloc_impl"); + // return nullptr; // Unreachable. +} + +inline int tp_init_impl(PyObject *, PyObject *, PyObject *) { + pybind11_fail("UNEXPECTED CALL OF function_record_PyTypeObject_methods::tp_init_impl"); + // return -1; // Unreachable. +} + +// The implementation needs the definition of `class cpp_function`. +void tp_dealloc_impl(PyObject *self); + +inline void tp_free_impl(void *) { + pybind11_fail("UNEXPECTED CALL OF function_record_PyTypeObject_methods::tp_free_impl"); +} + +inline PyObject *reduce_ex_impl(PyObject *self, PyObject *, PyObject *) { + // Deliberately ignoring the arguments for simplicity (expected is `protocol: int`). + const function_record *rec = function_record_ptr_from_PyObject(self); + if (rec == nullptr) { + pybind11_fail( + "FATAL: function_record_PyTypeObject reduce_ex_impl(): cannot obtain cpp_func_rec."); + } + if (rec->name != nullptr && rec->name[0] != '\0' && rec->scope + && PyModule_Check(rec->scope.ptr()) != 0) { + object scope_module = get_scope_module(rec->scope); + if (scope_module) { + return make_tuple(reinterpret_borrow(PyEval_GetBuiltins())["eval"], + make_tuple(str("__import__('importlib').import_module('") + + scope_module + str("')"))) + .release() + .ptr(); + } + } + set_error(PyExc_RuntimeError, repr(self) + str(" is not pickleable.")); + return nullptr; +} + +PYBIND11_NAMESPACE_END(function_record_PyTypeObject_methods) + +PYBIND11_NAMESPACE_END(detail) +PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE) diff --git a/include/pybind11/detail/internals.h b/include/pybind11/detail/internals.h index 2fd1f6719c..30b1fe2f9e 100644 --- a/include/pybind11/detail/internals.h +++ b/include/pybind11/detail/internals.h @@ -37,11 +37,11 @@ /// further ABI-incompatible changes may be made before the ABI is officially /// changed to the new version. #ifndef PYBIND11_INTERNALS_VERSION -# define PYBIND11_INTERNALS_VERSION 8 +# define PYBIND11_INTERNALS_VERSION 9 #endif -#if PYBIND11_INTERNALS_VERSION < 8 -# error "PYBIND11_INTERNALS_VERSION 8 is the minimum for all platforms for pybind11v3." +#if PYBIND11_INTERNALS_VERSION < 9 +# error "PYBIND11_INTERNALS_VERSION 9 is the minimum for all platforms for pybind11v3." #endif PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) @@ -190,10 +190,6 @@ struct internals { // Unused if PYBIND11_SIMPLE_GIL_MANAGEMENT is defined: PyInterpreterState *istate = nullptr; - // Note that we have to use a std::string to allocate memory to ensure a unique address - // We want unique addresses since we use pointer equality to compare function records - std::string function_record_capsule_name = internals_function_record_capsule_name; - type_map native_enum_type_map; internals() = default; @@ -612,26 +608,6 @@ const char *c_str(Args &&...args) { return strings.front().c_str(); } -inline const char *get_function_record_capsule_name() { - // On GraalPy, pointer equality of the names is currently not guaranteed -#if !defined(GRAALVM_PYTHON) - return get_internals().function_record_capsule_name.c_str(); -#else - return nullptr; -#endif -} - -// Determine whether or not the following capsule contains a pybind11 function record. -// Note that we use `internals` to make sure that only ABI compatible records are touched. -// -// This check is currently used in two places: -// - An important optimization in functional.h to avoid overhead in C++ -> Python -> C++ -// - The sibling feature of cpp_function to allow overloads -inline bool is_function_record_capsule(const capsule &cap) { - // Pointer equality as we rely on internals() to ensure unique pointers - return cap.name() == get_function_record_capsule_name(); -} - PYBIND11_NAMESPACE_END(detail) /// Returns a named pointer that is shared among all extension modules (using the same diff --git a/include/pybind11/functional.h b/include/pybind11/functional.h index 22748f91f6..58e5b3645e 100644 --- a/include/pybind11/functional.h +++ b/include/pybind11/functional.h @@ -93,15 +93,8 @@ struct type_caster> { auto *cfunc_self = PyCFunction_GET_SELF(cfunc.ptr()); if (cfunc_self == nullptr) { PyErr_Clear(); - } else if (isinstance(cfunc_self)) { - auto c = reinterpret_borrow(cfunc_self); - - function_record *rec = nullptr; - // Check that we can safely reinterpret the capsule into a function_record - if (detail::is_function_record_capsule(c)) { - rec = c.get_pointer(); - } - + } else { + function_record *rec = function_record_ptr_from_PyObject(cfunc_self); while (rec != nullptr) { if (rec->is_stateless && same_type(typeid(function_type), diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index caca57b3e4..453d19a508 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -12,6 +12,7 @@ #include "detail/class.h" #include "detail/dynamic_raw_ptr_cast_if_possible.h" #include "detail/exception_translation.h" +#include "detail/function_record_pyobject.h" #include "detail/init.h" #include "detail/native_enum_data.h" #include "detail/using_smart_holder.h" @@ -21,6 +22,7 @@ #include "options.h" #include "typing.h" +#include #include #include #include @@ -598,20 +600,11 @@ class cpp_function : public function { if (rec->sibling) { if (PyCFunction_Check(rec->sibling.ptr())) { auto *self = PyCFunction_GET_SELF(rec->sibling.ptr()); - if (!isinstance(self)) { + chain = detail::function_record_ptr_from_PyObject(self); + if (chain && !chain->scope.is(rec->scope)) { + /* Never append a method to an overload chain of a parent class; + instead, hide the parent's overloads in this case */ chain = nullptr; - } else { - auto rec_capsule = reinterpret_borrow(self); - if (detail::is_function_record_capsule(rec_capsule)) { - chain = rec_capsule.get_pointer(); - /* Never append a method to an overload chain of a parent class; - instead, hide the parent's overloads in this case */ - if (!chain->scope.is(rec->scope)) { - chain = nullptr; - } - } else { - chain = nullptr; - } } } // Don't trigger for things like the default __init__, which are wrapper_descriptors @@ -631,21 +624,14 @@ class cpp_function : public function { = reinterpret_cast(reinterpret_cast(dispatcher)); rec->def->ml_flags = METH_VARARGS | METH_KEYWORDS; - capsule rec_capsule(unique_rec.release(), - detail::get_function_record_capsule_name(), - [](void *ptr) { destruct((detail::function_record *) ptr); }); + detail::function_record_PyTypeObject_PyType_Ready(); // Call-once initialization. + object py_func_rec = detail::function_record_PyObject_New(); + ((detail::function_record_PyObject *) py_func_rec.ptr())->cpp_func_rec + = unique_rec.release(); guarded_strdup.release(); - object scope_module; - if (rec->scope) { - if (hasattr(rec->scope, "__module__")) { - scope_module = rec->scope.attr("__module__"); - } else if (hasattr(rec->scope, "__name__")) { - scope_module = rec->scope.attr("__name__"); - } - } - - m_ptr = PyCFunction_NewEx(rec->def, rec_capsule.ptr(), scope_module.ptr()); + object scope_module = detail::get_scope_module(rec->scope); + m_ptr = PyCFunction_NewEx(rec->def, py_func_rec.ptr(), scope_module.ptr()); if (!m_ptr) { pybind11_fail("cpp_function::cpp_function(): Could not allocate function object"); } @@ -674,8 +660,9 @@ class cpp_function : public function { // chain. chain_start = rec; rec->next = chain; - auto rec_capsule = reinterpret_borrow(PyCFunction_GET_SELF(m_ptr)); - rec_capsule.set_pointer(unique_rec.release()); + auto *py_func_rec + = (detail::function_record_PyObject *) PyCFunction_GET_SELF(m_ptr); + py_func_rec->cpp_func_rec = unique_rec.release(); guarded_strdup.release(); } else { // Or end of chain (normal behavior) @@ -750,6 +737,8 @@ class cpp_function : public function { } } + friend void detail::function_record_PyTypeObject_methods::tp_dealloc_impl(PyObject *); + /// When a cpp_function is GCed, release any memory allocated by pybind11 static void destruct(detail::function_record *rec, bool free_strings = true) { // If on Python 3.9, check the interpreter "MICRO" (patch) version. @@ -799,13 +788,11 @@ class cpp_function : public function { /// Main dispatch logic for calls to functions bound using pybind11 static PyObject *dispatcher(PyObject *self, PyObject *args_in, PyObject *kwargs_in) { using namespace detail; - assert(isinstance(self)); + const function_record *overloads = function_record_ptr_from_PyObject(self); + assert(overloads != nullptr); /* Iterator over the list of potentially admissible overloads */ - const function_record *overloads = reinterpret_cast( - PyCapsule_GetPointer(self, get_function_record_capsule_name())), - *current_overload = overloads; - assert(overloads != nullptr); + const function_record *current_overload = overloads; /* Need to know how many arguments + keyword arguments there are to pick the right overload */ @@ -1249,6 +1236,17 @@ class cpp_function : public function { PYBIND11_NAMESPACE_BEGIN(detail) +PYBIND11_NAMESPACE_BEGIN(function_record_PyTypeObject_methods) + +// This implementation needs the definition of `class cpp_function`. +inline void tp_dealloc_impl(PyObject *self) { + auto *py_func_rec = (function_record_PyObject *) self; + cpp_function::destruct(py_func_rec->cpp_func_rec); + py_func_rec->cpp_func_rec = nullptr; +} + +PYBIND11_NAMESPACE_END(function_record_PyTypeObject_methods) + template <> struct handle_type_name { static constexpr auto name = const_name("Callable"); @@ -2465,14 +2463,7 @@ class class_ : public detail::generic_type { if (!func_self) { throw error_already_set(); } - if (!isinstance(func_self)) { - return nullptr; - } - auto cap = reinterpret_borrow(func_self); - if (!detail::is_function_record_capsule(cap)) { - return nullptr; - } - return cap.get_pointer(); + return detail::function_record_ptr_from_PyObject(func_self.ptr()); } }; diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 57f1279657..e01c7b0b96 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -1398,6 +1398,18 @@ class simple_collector; template class unpacking_collector; +inline object get_scope_module(handle scope) { + if (scope) { + if (hasattr(scope, "__module__")) { + return scope.attr("__module__"); + } + if (hasattr(scope, "__name__")) { + return scope.attr("__name__"); + } + } + return object(); +} + PYBIND11_NAMESPACE_END(detail) // TODO: After the deprecated constructors are removed, this macro can be simplified by diff --git a/tests/extra_python_package/test_files.py b/tests/extra_python_package/test_files.py index 2fa47c614a..86c37089a1 100644 --- a/tests/extra_python_package/test_files.py +++ b/tests/extra_python_package/test_files.py @@ -65,6 +65,7 @@ "include/pybind11/detail/cpp_conduit.h", "include/pybind11/detail/descr.h", "include/pybind11/detail/dynamic_raw_ptr_cast_if_possible.h", + "include/pybind11/detail/function_record_pyobject.h", "include/pybind11/detail/init.h", "include/pybind11/detail/internals.h", "include/pybind11/detail/native_enum_data.h", diff --git a/tests/test_pickling.py b/tests/test_pickling.py index d3551efc12..c3b5c63274 100644 --- a/tests/test_pickling.py +++ b/tests/test_pickling.py @@ -9,18 +9,41 @@ from pybind11_tests import pickling as m -def test_pickle_simple_callable(): +def test_assumptions(): + assert pickle.HIGHEST_PROTOCOL >= 0 + + +@pytest.mark.parametrize("protocol", range(pickle.HIGHEST_PROTOCOL + 1)) +def test_pickle_simple_callable(protocol): assert m.simple_callable() == 20220426 - if env.PYPY: - serialized = pickle.dumps(m.simple_callable) - deserialized = pickle.loads(serialized) - assert deserialized() == 20220426 - else: - # To document broken behavior: currently it fails universally with - # all C Python versions. - with pytest.raises(TypeError) as excinfo: - pickle.dumps(m.simple_callable) - assert re.search("can.*t pickle .*[Cc]apsule.* object", str(excinfo.value)) + serialized = pickle.dumps(m.simple_callable, protocol=protocol) + assert b"pybind11_tests.pickling" in serialized + assert b"simple_callable" in serialized + deserialized = pickle.loads(serialized) + assert deserialized() == 20220426 + assert deserialized is m.simple_callable + + # UNUSUAL: function record pickle roundtrip returns a module, not a function record object: + if not env.PYPY: + assert ( + pickle.loads(pickle.dumps(m.simple_callable.__self__, protocol=protocol)) + is m + ) + # This is not expected to create issues because the only purpose of + # `m.simple_callable.__self__` is to enable pickling: the only method it has is + # `__reduce_ex__`. Direct access for any other purpose is not supported. + # Note that `repr(m.simple_callable.__self__)` shows, e.g.: + # `` + # It is considered to be as much an implementation detail as the + # `pybind11::detail::function_record` C++ type is. + + # @rainwoodman suggested that the unusual pickle roundtrip behavior could be + # avoided by changing `reduce_ex_impl()` to produce, e.g.: + # `"__import__('importlib').import_module('pybind11_tests.pickling').simple_callable.__self__"` + # as the argument for the `eval()` function, and adding a getter to the + # `function_record_PyTypeObject` that returns `self`. However, the additional code complexity + # for this is deemed warranted only if the unusual pickle roundtrip behavior actually + # creates issues. @pytest.mark.parametrize("cls_name", ["Pickleable", "PickleableNew"]) From de67af66741b4edb8b985652e0a74868df89c2e2 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 24 Mar 2025 21:51:39 -0700 Subject: [PATCH 02/18] Add back `PYBIND11_WARNING_DISABLE_CLANG("-Wcast-function-type-mismatch")` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit macos-13 • brew install llvm ``` /Users/runner/work/pybind11/pybind11/include/pybind11/detail/function_record_pyobject.h:40:26: error: cast from 'PyObject *(*)(PyObject *, PyObject *, PyObject *)' (aka '_object *(*)(_object *, _object *, _object *)') to 'PyCFunction' (aka '_object *(*)(_object *, _object *)') converts to incompatible function type [-Werror,-Wcast-function-type-mismatch] 40 | = {{"__reduce_ex__", (PyCFunction) reduce_ex_impl, METH_VARARGS | METH_KEYWORDS, nullptr}, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ ``` --- include/pybind11/detail/function_record_pyobject.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/include/pybind11/detail/function_record_pyobject.h b/include/pybind11/detail/function_record_pyobject.h index 72c8c6c3b8..ca0e3fedce 100644 --- a/include/pybind11/detail/function_record_pyobject.h +++ b/include/pybind11/detail/function_record_pyobject.h @@ -36,6 +36,9 @@ PYBIND11_WARNING_PUSH #if defined(__GNUC__) && __GNUC__ >= 8 PYBIND11_WARNING_DISABLE_GCC("-Wcast-function-type") #endif +#if defined(__clang__) && !defined(__apple_build_version__) && __clang_major__ >= 19 +PYBIND11_WARNING_DISABLE_CLANG("-Wcast-function-type-mismatch") +#endif static PyMethodDef tp_methods_impl[] = {{"__reduce_ex__", (PyCFunction) reduce_ex_impl, METH_VARARGS | METH_KEYWORDS, nullptr}, {nullptr, nullptr, 0, nullptr}}; From e8e921da026accb33eb350198c89f5616940fae5 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Wed, 26 Mar 2025 09:01:39 -0700 Subject: [PATCH 03/18] Remove obsolete `PY_VERSION_HEX >= 0x03080000` (discovered by gh-henryiii) --- include/pybind11/detail/function_record_pyobject.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/include/pybind11/detail/function_record_pyobject.h b/include/pybind11/detail/function_record_pyobject.h index ca0e3fedce..fce08f3cf9 100644 --- a/include/pybind11/detail/function_record_pyobject.h +++ b/include/pybind11/detail/function_record_pyobject.h @@ -108,9 +108,7 @@ static PyTypeObject function_record_PyTypeObject = { /* destructor tp_del */ nullptr, /* unsigned int tp_version_tag */ 0, /* destructor tp_finalize */ nullptr, -#if PY_VERSION_HEX >= 0x03080000 /* vectorcallfunc tp_vectorcall */ nullptr, -#endif }; PYBIND11_WARNING_POP From 0da8312825bb9610f746cb0db648bbb54fd28c1b Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Thu, 27 Mar 2025 12:48:47 -0700 Subject: [PATCH 04/18] git merge --squash add_test_for_boost_histogram_situation --- include/pybind11/functional.h | 9 +++++++++ tests/test_callbacks.cpp | 31 +++++++++++++++++++++++++++++++ tests/test_callbacks.py | 12 ++++++++++++ 3 files changed, 52 insertions(+) diff --git a/include/pybind11/functional.h b/include/pybind11/functional.h index 58e5b3645e..4d8a4340f9 100644 --- a/include/pybind11/functional.h +++ b/include/pybind11/functional.h @@ -9,6 +9,7 @@ #pragma once +#define PYBIND11_HAS_TYPE_CASTER_STD_FUNCTION_FUNC_IS_STATELESS_WITH_EXACT_TYPE #define PYBIND11_HAS_TYPE_CASTER_STD_FUNCTION_SPECIALIZATIONS #include "pybind11.h" @@ -65,8 +66,15 @@ struct type_caster> { using retval_type = conditional_t::value, void_type, Return>; using function_type = Return (*)(Args...); +private: + bool func_is_stateless_with_exact_type_ = false; + public: + bool func_is_stateless_with_exact_type() const { return func_is_stateless_with_exact_type_; } + bool load(handle src, bool convert) { + func_is_stateless_with_exact_type_ = false; + if (src.is_none()) { // Defer accepting None to other overloads (if we aren't in convert mode): if (!convert) { @@ -103,6 +111,7 @@ struct type_caster> { function_type f; }; value = ((capture *) &rec->data)->f; + func_is_stateless_with_exact_type_ = true; return true; } rec = rec->next; diff --git a/tests/test_callbacks.cpp b/tests/test_callbacks.cpp index 20dbaa6b2c..a718bd6ddb 100644 --- a/tests/test_callbacks.cpp +++ b/tests/test_callbacks.cpp @@ -14,6 +14,30 @@ #include +namespace test_callbacks { +namespace boost_histogram { // See PR #5580 + +double custom_transform_double(double value) { return value * 3; } +int custom_transform_int(int value) { return value; } + +// Originally derived from +// https://github.com/scikit-hep/boost-histogram/blob/460ef90905d6a8a9e6dd3beddfe7b4b49b364579/include/bh_python/transform.hpp#L68-L85 +double apply_custom_transform(const py::object &src, double value) { + using raw_t = double(double); + + py::detail::make_caster> func_caster; + if (!func_caster.load(src, /*convert*/ false)) { + return -100; + } + if (!func_caster.func_is_stateless_with_exact_type()) { + return -200; + } + return static_cast &>(func_caster)(value); +} + +} // namespace boost_histogram +} // namespace test_callbacks + int dummy_function(int i) { return i + 1; } TEST_SUBMODULE(callbacks, m) { @@ -272,4 +296,11 @@ TEST_SUBMODULE(callbacks, m) { // rec_capsule with nullptr name py::capsule rec_capsule2(std::malloc(1), [](void *data) { std::free(data); }); m.add_object("custom_function2", PyCFunction_New(custom_def, rec_capsule2.ptr())); + + m.def("boost_histogram_custom_transform_double", + test_callbacks::boost_histogram::custom_transform_double); + m.def("boost_histogram_custom_transform_int", + test_callbacks::boost_histogram::custom_transform_int); + m.def("boost_histogram_apply_custom_transform", + test_callbacks::boost_histogram::apply_custom_transform); } diff --git a/tests/test_callbacks.py b/tests/test_callbacks.py index fbe56b9257..c43a9290ac 100644 --- a/tests/test_callbacks.py +++ b/tests/test_callbacks.py @@ -234,3 +234,15 @@ def test_callback_docstring(): m.test_tuple_unpacking.__doc__.strip() == "test_tuple_unpacking(arg0: Callable) -> object" ) + + +def test_boost_histogram_apply_custom_transform(): + ctd = m.boost_histogram_custom_transform_double + cti = m.boost_histogram_custom_transform_int + apply = m.boost_histogram_apply_custom_transform + assert apply(ctd, 5) == 15 + assert apply(cti, 0) == -200 + assert apply(None, 0) == -100 + assert apply(lambda value: value, 9) == -200 + assert apply({}, 0) == -100 + assert apply("", 0) == -100 From cb3b41e4db5b3f38464fb8c3f0bc35668c092968 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Thu, 27 Mar 2025 14:12:37 -0700 Subject: [PATCH 05/18] Revert "git merge --squash add_test_for_boost_histogram_situation" This reverts commit 0da8312825bb9610f746cb0db648bbb54fd28c1b. --- include/pybind11/functional.h | 9 --------- tests/test_callbacks.cpp | 31 ------------------------------- tests/test_callbacks.py | 12 ------------ 3 files changed, 52 deletions(-) diff --git a/include/pybind11/functional.h b/include/pybind11/functional.h index 4d8a4340f9..58e5b3645e 100644 --- a/include/pybind11/functional.h +++ b/include/pybind11/functional.h @@ -9,7 +9,6 @@ #pragma once -#define PYBIND11_HAS_TYPE_CASTER_STD_FUNCTION_FUNC_IS_STATELESS_WITH_EXACT_TYPE #define PYBIND11_HAS_TYPE_CASTER_STD_FUNCTION_SPECIALIZATIONS #include "pybind11.h" @@ -66,15 +65,8 @@ struct type_caster> { using retval_type = conditional_t::value, void_type, Return>; using function_type = Return (*)(Args...); -private: - bool func_is_stateless_with_exact_type_ = false; - public: - bool func_is_stateless_with_exact_type() const { return func_is_stateless_with_exact_type_; } - bool load(handle src, bool convert) { - func_is_stateless_with_exact_type_ = false; - if (src.is_none()) { // Defer accepting None to other overloads (if we aren't in convert mode): if (!convert) { @@ -111,7 +103,6 @@ struct type_caster> { function_type f; }; value = ((capture *) &rec->data)->f; - func_is_stateless_with_exact_type_ = true; return true; } rec = rec->next; diff --git a/tests/test_callbacks.cpp b/tests/test_callbacks.cpp index a718bd6ddb..20dbaa6b2c 100644 --- a/tests/test_callbacks.cpp +++ b/tests/test_callbacks.cpp @@ -14,30 +14,6 @@ #include -namespace test_callbacks { -namespace boost_histogram { // See PR #5580 - -double custom_transform_double(double value) { return value * 3; } -int custom_transform_int(int value) { return value; } - -// Originally derived from -// https://github.com/scikit-hep/boost-histogram/blob/460ef90905d6a8a9e6dd3beddfe7b4b49b364579/include/bh_python/transform.hpp#L68-L85 -double apply_custom_transform(const py::object &src, double value) { - using raw_t = double(double); - - py::detail::make_caster> func_caster; - if (!func_caster.load(src, /*convert*/ false)) { - return -100; - } - if (!func_caster.func_is_stateless_with_exact_type()) { - return -200; - } - return static_cast &>(func_caster)(value); -} - -} // namespace boost_histogram -} // namespace test_callbacks - int dummy_function(int i) { return i + 1; } TEST_SUBMODULE(callbacks, m) { @@ -296,11 +272,4 @@ TEST_SUBMODULE(callbacks, m) { // rec_capsule with nullptr name py::capsule rec_capsule2(std::malloc(1), [](void *data) { std::free(data); }); m.add_object("custom_function2", PyCFunction_New(custom_def, rec_capsule2.ptr())); - - m.def("boost_histogram_custom_transform_double", - test_callbacks::boost_histogram::custom_transform_double); - m.def("boost_histogram_custom_transform_int", - test_callbacks::boost_histogram::custom_transform_int); - m.def("boost_histogram_apply_custom_transform", - test_callbacks::boost_histogram::apply_custom_transform); } diff --git a/tests/test_callbacks.py b/tests/test_callbacks.py index c43a9290ac..fbe56b9257 100644 --- a/tests/test_callbacks.py +++ b/tests/test_callbacks.py @@ -234,15 +234,3 @@ def test_callback_docstring(): m.test_tuple_unpacking.__doc__.strip() == "test_tuple_unpacking(arg0: Callable) -> object" ) - - -def test_boost_histogram_apply_custom_transform(): - ctd = m.boost_histogram_custom_transform_double - cti = m.boost_histogram_custom_transform_int - apply = m.boost_histogram_apply_custom_transform - assert apply(ctd, 5) == 15 - assert apply(cti, 0) == -200 - assert apply(None, 0) == -100 - assert apply(lambda value: value, 9) == -200 - assert apply({}, 0) == -100 - assert apply("", 0) == -100 From d97186fa195f64a060d10c664cf044663f9733da Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Thu, 27 Mar 2025 14:13:05 -0700 Subject: [PATCH 06/18] git merge --squash add_test_for_boost_histogram_situation --- tests/test_callbacks.cpp | 33 +++++++++++++++++++++++++++++++++ tests/test_callbacks.py | 12 ++++++++++++ 2 files changed, 45 insertions(+) diff --git a/tests/test_callbacks.cpp b/tests/test_callbacks.cpp index 20dbaa6b2c..2f8485f027 100644 --- a/tests/test_callbacks.cpp +++ b/tests/test_callbacks.cpp @@ -14,6 +14,32 @@ #include +namespace test_callbacks { +namespace boost_histogram { // See PR #5580 + +double custom_transform_double(double value) { return value * 3; } +int custom_transform_int(int value) { return value; } + +// Originally derived from +// https://github.com/scikit-hep/boost-histogram/blob/460ef90905d6a8a9e6dd3beddfe7b4b49b364579/include/bh_python/transform.hpp#L68-L85 +double apply_custom_transform(const py::object &src, double value) { + using raw_t = double(double); + + py::detail::make_caster> func_caster; + if (!func_caster.load(src, /*convert*/ false)) { + return -100; + } + auto func = static_cast &>(func_caster); + auto *cfunc = func.target(); + if (cfunc == nullptr) { + return -200; + } + return (*cfunc)(value); +} + +} // namespace boost_histogram +} // namespace test_callbacks + int dummy_function(int i) { return i + 1; } TEST_SUBMODULE(callbacks, m) { @@ -272,4 +298,11 @@ TEST_SUBMODULE(callbacks, m) { // rec_capsule with nullptr name py::capsule rec_capsule2(std::malloc(1), [](void *data) { std::free(data); }); m.add_object("custom_function2", PyCFunction_New(custom_def, rec_capsule2.ptr())); + + m.def("boost_histogram_custom_transform_double", + test_callbacks::boost_histogram::custom_transform_double); + m.def("boost_histogram_custom_transform_int", + test_callbacks::boost_histogram::custom_transform_int); + m.def("boost_histogram_apply_custom_transform", + test_callbacks::boost_histogram::apply_custom_transform); } diff --git a/tests/test_callbacks.py b/tests/test_callbacks.py index fbe56b9257..c43a9290ac 100644 --- a/tests/test_callbacks.py +++ b/tests/test_callbacks.py @@ -234,3 +234,15 @@ def test_callback_docstring(): m.test_tuple_unpacking.__doc__.strip() == "test_tuple_unpacking(arg0: Callable) -> object" ) + + +def test_boost_histogram_apply_custom_transform(): + ctd = m.boost_histogram_custom_transform_double + cti = m.boost_histogram_custom_transform_int + apply = m.boost_histogram_apply_custom_transform + assert apply(ctd, 5) == 15 + assert apply(cti, 0) == -200 + assert apply(None, 0) == -100 + assert apply(lambda value: value, 9) == -200 + assert apply({}, 0) == -100 + assert apply("", 0) == -100 From 8b500da6a3b568d557f8cfb378477d48bad9e136 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Thu, 27 Mar 2025 16:33:57 -0700 Subject: [PATCH 07/18] Add `.get_capsule_for_scipy_LowLevelCallable()` method in function_record_pyobject.h --- .../detail/function_record_pyobject.h | 29 +++++++++++++++++++ tests/test_scipy_low_level_callable.cpp | 15 ++++++++++ tests/test_scipy_low_level_callable.py | 27 +++++++++++++++++ 3 files changed, 71 insertions(+) create mode 100644 tests/test_scipy_low_level_callable.cpp create mode 100644 tests/test_scipy_low_level_callable.py diff --git a/include/pybind11/detail/function_record_pyobject.h b/include/pybind11/detail/function_record_pyobject.h index fce08f3cf9..da9203479d 100644 --- a/include/pybind11/detail/function_record_pyobject.h +++ b/include/pybind11/detail/function_record_pyobject.h @@ -31,6 +31,8 @@ void tp_dealloc_impl(PyObject *self); void tp_free_impl(void *self); static PyObject *reduce_ex_impl(PyObject *self, PyObject *, PyObject *); +static PyObject * +get_capsule_for_scipy_LowLevelCallable_impl(PyObject *self, PyObject *, PyObject *); PYBIND11_WARNING_PUSH #if defined(__GNUC__) && __GNUC__ >= 8 @@ -41,6 +43,10 @@ PYBIND11_WARNING_DISABLE_CLANG("-Wcast-function-type-mismatch") #endif static PyMethodDef tp_methods_impl[] = {{"__reduce_ex__", (PyCFunction) reduce_ex_impl, METH_VARARGS | METH_KEYWORDS, nullptr}, + {"get_capsule_for_scipy_LowLevelCallable", + (PyCFunction) get_capsule_for_scipy_LowLevelCallable_impl, + METH_VARARGS | METH_KEYWORDS, + "for use with scipy.LowLevelCallable()"}, {nullptr, nullptr, 0, nullptr}}; PYBIND11_WARNING_POP @@ -202,6 +208,29 @@ inline PyObject *reduce_ex_impl(PyObject *self, PyObject *, PyObject *) { return nullptr; } +inline PyObject * +get_capsule_for_scipy_LowLevelCallable_impl(PyObject *self, PyObject *args, PyObject *kwargs) { + static const char *kwlist[] = {"signature", nullptr}; + const char *signature = nullptr; + if (!PyArg_ParseTupleAndKeywords(args, kwargs, "s", const_cast(kwlist), &signature)) { + return nullptr; + } + function_record *rec = function_record_ptr_from_PyObject(self); + if (rec == nullptr) { + pybind11_fail("FATAL: get_capsule_for_scipy_LowLevelCallable_impl(): cannot obtain C++ " + "function_record."); + } + if (!rec->is_stateless) { + set_error(PyExc_TypeError, repr(self) + str(" is not a stateless function.")); + return nullptr; + } + struct capture { + void *f; // DANGER: TYPE SAFETY IS LOST COMPLETELY. + }; + auto cap = reinterpret_cast(&rec->data); + return capsule(cap->f, signature).release().ptr(); +} + PYBIND11_NAMESPACE_END(function_record_PyTypeObject_methods) PYBIND11_NAMESPACE_END(detail) diff --git a/tests/test_scipy_low_level_callable.cpp b/tests/test_scipy_low_level_callable.cpp new file mode 100644 index 0000000000..edd86b13c7 --- /dev/null +++ b/tests/test_scipy_low_level_callable.cpp @@ -0,0 +1,15 @@ +#include "pybind11_tests.h" + +namespace pybind11_tests { +namespace scipy_low_level_callable { + +extern "C" double square(double x) { return x * x; } + +} // namespace scipy_low_level_callable +} // namespace pybind11_tests + +TEST_SUBMODULE(scipy_low_level_callable, m) { + using namespace pybind11_tests::scipy_low_level_callable; + + m.def("square", square); +} diff --git a/tests/test_scipy_low_level_callable.py b/tests/test_scipy_low_level_callable.py new file mode 100644 index 0000000000..466dced3c2 --- /dev/null +++ b/tests/test_scipy_low_level_callable.py @@ -0,0 +1,27 @@ +from __future__ import annotations + +import pytest + +from pybind11_tests import scipy_low_level_callable as m + + +def test_square(): + assert m.square(2.0) == 4.0 + + +def test_get_capsule_for_scipy_LowLevelCallable(): + cap = m.square.__self__.get_capsule_for_scipy_LowLevelCallable( + signature="double (double)" + ) + assert repr(cap).startswith(' Date: Thu, 27 Mar 2025 20:13:47 -0700 Subject: [PATCH 08/18] Resolve clang-tidy errors ``` /__w/pybind11/pybind11/include/pybind11/detail/function_record_pyobject.h:215:10: error: implicit conversion 'int' -> 'bool' [readability-implicit-bool-conversion,-warnings-as-errors] 215 | if (!PyArg_ParseTupleAndKeywords(args, kwargs, "s", const_cast(kwlist), &signature)) { | ~^ | == 0 /__w/pybind11/pybind11/include/pybind11/detail/function_record_pyobject.h:230:5: error: 'auto cap' can be declared as 'auto *cap' [readability-qualified-auto,-warnings-as-errors] 230 | auto cap = reinterpret_cast(&rec->data); | ^~~~ | auto * ``` --- include/pybind11/detail/function_record_pyobject.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/include/pybind11/detail/function_record_pyobject.h b/include/pybind11/detail/function_record_pyobject.h index da9203479d..a8e36691c1 100644 --- a/include/pybind11/detail/function_record_pyobject.h +++ b/include/pybind11/detail/function_record_pyobject.h @@ -212,7 +212,8 @@ inline PyObject * get_capsule_for_scipy_LowLevelCallable_impl(PyObject *self, PyObject *args, PyObject *kwargs) { static const char *kwlist[] = {"signature", nullptr}; const char *signature = nullptr; - if (!PyArg_ParseTupleAndKeywords(args, kwargs, "s", const_cast(kwlist), &signature)) { + if (PyArg_ParseTupleAndKeywords(args, kwargs, "s", const_cast(kwlist), &signature) + == 0) { return nullptr; } function_record *rec = function_record_ptr_from_PyObject(self); @@ -227,7 +228,7 @@ get_capsule_for_scipy_LowLevelCallable_impl(PyObject *self, PyObject *args, PyOb struct capture { void *f; // DANGER: TYPE SAFETY IS LOST COMPLETELY. }; - auto cap = reinterpret_cast(&rec->data); + auto *cap = reinterpret_cast(&rec->data); return capsule(cap->f, signature).release().ptr(); } From e4e527f6134f76308f6b576c7589487049389e71 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Thu, 27 Mar 2025 20:15:41 -0700 Subject: [PATCH 09/18] Add test_scipy_low_level_callable in tests/CMakeLists.txt --- tests/CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 1b65729dfd..a93e0479e4 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -163,6 +163,7 @@ set(PYBIND11_TEST_FILES test_pickling test_python_multiple_inheritance test_pytypes + test_scipy_low_level_callable test_sequences_and_iterators test_smart_ptr test_stl From 8757b56c5f04b604273c0c4aca280fd707787404 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Thu, 27 Mar 2025 22:22:38 -0700 Subject: [PATCH 10/18] Import scipy.integrate explicitly, in a way that doesn't trigger a ruff error. --- tests/test_scipy_low_level_callable.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/test_scipy_low_level_callable.py b/tests/test_scipy_low_level_callable.py index 466dced3c2..5af1efc306 100644 --- a/tests/test_scipy_low_level_callable.py +++ b/tests/test_scipy_low_level_callable.py @@ -18,10 +18,13 @@ def test_get_capsule_for_scipy_LowLevelCallable(): def test_with_scipy_LowLevelCallable(): scipy = pytest.importorskip("scipy") + # Explicit import needed with some (older) scipy versions: + from scipy import integrate + llc = scipy.LowLevelCallable( m.square.__self__.get_capsule_for_scipy_LowLevelCallable( signature="double (double)" ) ) - integral = scipy.integrate.quad(llc, 0, 1) + integral = integrate.quad(llc, 0, 1) assert integral[0] == pytest.approx(1 / 3, rel=1e-12) From 70d6d11a7a56f9e27d049ff9cdf6139f17507d39 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Thu, 27 Mar 2025 22:44:18 -0700 Subject: [PATCH 11/18] Resolve PyPy errors with a general workaround: 'builtin_function_or_method' object has no attribute '__self__' --- tests/test_scipy_low_level_callable.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/tests/test_scipy_low_level_callable.py b/tests/test_scipy_low_level_callable.py index 5af1efc306..86719b00d5 100644 --- a/tests/test_scipy_low_level_callable.py +++ b/tests/test_scipy_low_level_callable.py @@ -9,11 +9,18 @@ def test_square(): assert m.square(2.0) == 4.0 +def _m_square_self(): + try: + return m.square.__self__ + except AttributeError as e: + pytest.skip(f"{str(e)}") + + def test_get_capsule_for_scipy_LowLevelCallable(): - cap = m.square.__self__.get_capsule_for_scipy_LowLevelCallable( + cap = _m_square_self().get_capsule_for_scipy_LowLevelCallable( signature="double (double)" ) - assert repr(cap).startswith(' Date: Thu, 27 Mar 2025 23:56:10 -0700 Subject: [PATCH 12/18] =?UTF-8?q?Remove=20`extern=20"C"`:=20we=20want=20th?= =?UTF-8?q?e=20name=20mangling=20here,=20all=20we=20need=20is=20the=20func?= =?UTF-8?q?tion=20pointer.=20Also=20make=20the=20function=20a=20bit=20more?= =?UTF-8?q?=20unique=20(square=20=E2=86=92=20square21)=20for=20easier=20pi?= =?UTF-8?q?n-pointing.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Related ChatGPT conversation regarding `extern "C"`: https://chatgpt.com/share/67e6481e-0994-8008-a64a-a07ce2b781ea --- tests/test_scipy_low_level_callable.cpp | 4 ++-- tests/test_scipy_low_level_callable.py | 14 +++++++------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/test_scipy_low_level_callable.cpp b/tests/test_scipy_low_level_callable.cpp index edd86b13c7..d41979352b 100644 --- a/tests/test_scipy_low_level_callable.cpp +++ b/tests/test_scipy_low_level_callable.cpp @@ -3,7 +3,7 @@ namespace pybind11_tests { namespace scipy_low_level_callable { -extern "C" double square(double x) { return x * x; } +double square21(double x) { return x * x * 21; } } // namespace scipy_low_level_callable } // namespace pybind11_tests @@ -11,5 +11,5 @@ extern "C" double square(double x) { return x * x; } TEST_SUBMODULE(scipy_low_level_callable, m) { using namespace pybind11_tests::scipy_low_level_callable; - m.def("square", square); + m.def("square21", square21); } diff --git a/tests/test_scipy_low_level_callable.py b/tests/test_scipy_low_level_callable.py index 86719b00d5..f55b64e846 100644 --- a/tests/test_scipy_low_level_callable.py +++ b/tests/test_scipy_low_level_callable.py @@ -5,19 +5,19 @@ from pybind11_tests import scipy_low_level_callable as m -def test_square(): - assert m.square(2.0) == 4.0 +def test_square21(): + assert m.square21(2.0) == 2.0 * 2.0 * 21 -def _m_square_self(): +def _m_square21_self(): try: - return m.square.__self__ + return m.square21.__self__ except AttributeError as e: pytest.skip(f"{str(e)}") def test_get_capsule_for_scipy_LowLevelCallable(): - cap = _m_square_self().get_capsule_for_scipy_LowLevelCallable( + cap = _m_square21_self().get_capsule_for_scipy_LowLevelCallable( signature="double (double)" ) assert repr(cap).startswith(" Date: Fri, 28 Mar 2025 00:43:28 -0700 Subject: [PATCH 13/18] =?UTF-8?q?Rename=20get=5Fcapsule=5Ffor=5Fscipy=5FLo?= =?UTF-8?q?wLevelCallable=20=E2=86=92=20get=5Fcapsule=5Ffor=5Fscipy=5FLowL?= =?UTF-8?q?evelCallable=5FNO=5FTYPE=5FSAFETY.=20Make=20the=20reinterpret?= =?UTF-8?q?=5Fcast=20from=20function=20pointer=20to=20void=20pointer=20exp?= =?UTF-8?q?licit=20for=20clarity.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../detail/function_record_pyobject.h | 21 +++++++++++-------- tests/test_scipy_low_level_callable.py | 4 ++-- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/include/pybind11/detail/function_record_pyobject.h b/include/pybind11/detail/function_record_pyobject.h index a8e36691c1..472af7db40 100644 --- a/include/pybind11/detail/function_record_pyobject.h +++ b/include/pybind11/detail/function_record_pyobject.h @@ -32,7 +32,7 @@ void tp_free_impl(void *self); static PyObject *reduce_ex_impl(PyObject *self, PyObject *, PyObject *); static PyObject * -get_capsule_for_scipy_LowLevelCallable_impl(PyObject *self, PyObject *, PyObject *); +get_capsule_for_scipy_LowLevelCallable_NO_TYPE_SAFETY_impl(PyObject *self, PyObject *, PyObject *); PYBIND11_WARNING_PUSH #if defined(__GNUC__) && __GNUC__ >= 8 @@ -43,8 +43,8 @@ PYBIND11_WARNING_DISABLE_CLANG("-Wcast-function-type-mismatch") #endif static PyMethodDef tp_methods_impl[] = {{"__reduce_ex__", (PyCFunction) reduce_ex_impl, METH_VARARGS | METH_KEYWORDS, nullptr}, - {"get_capsule_for_scipy_LowLevelCallable", - (PyCFunction) get_capsule_for_scipy_LowLevelCallable_impl, + {"get_capsule_for_scipy_LowLevelCallable_NO_TYPE_SAFETY", + (PyCFunction) get_capsule_for_scipy_LowLevelCallable_NO_TYPE_SAFETY_impl, METH_VARARGS | METH_KEYWORDS, "for use with scipy.LowLevelCallable()"}, {nullptr, nullptr, 0, nullptr}}; @@ -208,8 +208,9 @@ inline PyObject *reduce_ex_impl(PyObject *self, PyObject *, PyObject *) { return nullptr; } -inline PyObject * -get_capsule_for_scipy_LowLevelCallable_impl(PyObject *self, PyObject *args, PyObject *kwargs) { +inline PyObject *get_capsule_for_scipy_LowLevelCallable_NO_TYPE_SAFETY_impl(PyObject *self, + PyObject *args, + PyObject *kwargs) { static const char *kwlist[] = {"signature", nullptr}; const char *signature = nullptr; if (PyArg_ParseTupleAndKeywords(args, kwargs, "s", const_cast(kwlist), &signature) @@ -225,11 +226,13 @@ get_capsule_for_scipy_LowLevelCallable_impl(PyObject *self, PyObject *args, PyOb set_error(PyExc_TypeError, repr(self) + str(" is not a stateless function.")); return nullptr; } - struct capture { - void *f; // DANGER: TYPE SAFETY IS LOST COMPLETELY. + // This is a type-erased match for `struct capture` in pybind11/functional.h + struct type_erased_capture { + // Return (*)(Args...) in pybind11/functional.h + void (*void_func_ptr)(); // TYPE SAFETY IS LOST COMPLETELY HERE. }; - auto *cap = reinterpret_cast(&rec->data); - return capsule(cap->f, signature).release().ptr(); + auto *tec = reinterpret_cast(&rec->data); + return capsule(reinterpret_cast(tec->void_func_ptr), signature).release().ptr(); } PYBIND11_NAMESPACE_END(function_record_PyTypeObject_methods) diff --git a/tests/test_scipy_low_level_callable.py b/tests/test_scipy_low_level_callable.py index f55b64e846..9a704a2606 100644 --- a/tests/test_scipy_low_level_callable.py +++ b/tests/test_scipy_low_level_callable.py @@ -17,7 +17,7 @@ def _m_square21_self(): def test_get_capsule_for_scipy_LowLevelCallable(): - cap = _m_square21_self().get_capsule_for_scipy_LowLevelCallable( + cap = _m_square21_self().get_capsule_for_scipy_LowLevelCallable_NO_TYPE_SAFETY( signature="double (double)" ) assert repr(cap).startswith(" Date: Fri, 28 Mar 2025 02:42:22 -0700 Subject: [PATCH 14/18] Use `PYBIND11_STD_LAUNDER` consistently in pybind11/pybind11.h, pybind11/functional.h, pybind11/detail/function_record_pyobject.h Also `static_assert(std::is_standard_layout::value, "")` in all three places. ChatGPT conversation guiding gh-rwgk through the subtle changes: https://chatgpt.com/share/67e66f74-cc80-8008-8b19-1d02e86acf71 --- include/pybind11/detail/common.h | 8 ++++++++ .../pybind11/detail/function_record_pyobject.h | 9 +++++++-- include/pybind11/functional.h | 7 ++++++- include/pybind11/pybind11.h | 18 ++++++++---------- 4 files changed, 29 insertions(+), 13 deletions(-) diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index bacc5a60db..23a9edf258 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -123,6 +123,14 @@ # endif #endif +#if defined(__cpp_lib_launder) && !(defined(_MSC_VER) && (_MSC_VER < 1914)) +# define PYBIND11_STD_LAUNDER std::launder +# define PYBIND11_HAS_STD_LAUNDER 1 +#else +# define PYBIND11_STD_LAUNDER +# define PYBIND11_HAS_STD_LAUNDER 0 +#endif + #if defined(PYBIND11_CPP20) # define PYBIND11_CONSTINIT constinit # define PYBIND11_DTOR_CONSTEXPR constexpr diff --git a/include/pybind11/detail/function_record_pyobject.h b/include/pybind11/detail/function_record_pyobject.h index 472af7db40..73770f6c69 100644 --- a/include/pybind11/detail/function_record_pyobject.h +++ b/include/pybind11/detail/function_record_pyobject.h @@ -226,12 +226,17 @@ inline PyObject *get_capsule_for_scipy_LowLevelCallable_NO_TYPE_SAFETY_impl(PyOb set_error(PyExc_TypeError, repr(self) + str(" is not a stateless function.")); return nullptr; } - // This is a type-erased match for `struct capture` in pybind11/functional.h + // This struct must match the layout of `struct capture` in pybind11/functional.h struct type_erased_capture { // Return (*)(Args...) in pybind11/functional.h void (*void_func_ptr)(); // TYPE SAFETY IS LOST COMPLETELY HERE. + + static type_erased_capture *from_data(void **data) { + return PYBIND11_STD_LAUNDER(reinterpret_cast(data)); + } }; - auto *tec = reinterpret_cast(&rec->data); + static_assert(std::is_standard_layout::value, ""); + auto *tec = type_erased_capture::from_data(rec->data); return capsule(reinterpret_cast(tec->void_func_ptr), signature).release().ptr(); } diff --git a/include/pybind11/functional.h b/include/pybind11/functional.h index 58e5b3645e..e9767e4c2a 100644 --- a/include/pybind11/functional.h +++ b/include/pybind11/functional.h @@ -101,8 +101,13 @@ struct type_caster> { *reinterpret_cast(rec->data[1]))) { struct capture { function_type f; + + static capture *from_data(void **data) { + return PYBIND11_STD_LAUNDER(reinterpret_cast(data)); + } }; - value = ((capture *) &rec->data)->f; + static_assert(std::is_standard_layout::value, ""); + value = capture::from_data(rec->data)->f; return true; } rec = rec->next; diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 453d19a508..f67dae689d 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -38,13 +38,6 @@ PYBIND11_WARNING_DISABLE_CLANG("-Wgnu-zero-variadic-macro-arguments") #endif -#if defined(__cpp_lib_launder) && !(defined(_MSC_VER) && (_MSC_VER < 1914)) -# define PYBIND11_STD_LAUNDER std::launder -# define PYBIND11_HAS_STD_LAUNDER 1 -#else -# define PYBIND11_STD_LAUNDER -# define PYBIND11_HAS_STD_LAUNDER 0 -#endif #if defined(__GNUG__) && !defined(__clang__) # include #endif @@ -345,6 +338,10 @@ class cpp_function : public function { using namespace detail; struct capture { remove_reference_t f; + + static capture *from_data(void **data) { + return PYBIND11_STD_LAUNDER(reinterpret_cast(data)); + } }; /* Store the function including any extra state it might have (e.g. a lambda capture @@ -364,7 +361,7 @@ class cpp_function : public function { PYBIND11_WARNING_DISABLE_GCC("-Wplacement-new") #endif - new ((capture *) &rec->data) capture{std::forward(f)}; + new (capture::from_data(rec->data)) capture{std::forward(f)}; #if !PYBIND11_HAS_STD_LAUNDER PYBIND11_WARNING_DISABLE_GCC("-Wstrict-aliasing") @@ -374,8 +371,8 @@ class cpp_function : public function { // a significant refactoring it's "impossible" to solve. if (!std::is_trivially_destructible::value) { rec->free_data = [](function_record *r) { - auto data = PYBIND11_STD_LAUNDER((capture *) &r->data); - (void) data; + auto data = capture::from_data(r->data); + (void) data; // suppress "unused variable" warnings data->~capture(); }; } @@ -492,6 +489,7 @@ class cpp_function : public function { using FunctionType = Return (*)(Args...); constexpr bool is_function_ptr = std::is_convertible::value && sizeof(capture) == sizeof(void *); + static_assert(!is_function_ptr || std::is_standard_layout::value, ""); if (is_function_ptr) { rec->is_stateless = true; rec->data[1] From 00bcb061cc9f7135d771a533fad30fdca0e19a38 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Fri, 28 Mar 2025 08:20:59 -0700 Subject: [PATCH 15/18] Rename again: insert `ABI_OR` for completeness (new full name is `get_capsule_for_scipy_LowLevelCallable_NO_ABI_OR_TYPE_SAFETY`) --- include/pybind11/detail/function_record_pyobject.h | 14 +++++++------- tests/test_scipy_low_level_callable.py | 8 +++++--- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/include/pybind11/detail/function_record_pyobject.h b/include/pybind11/detail/function_record_pyobject.h index 73770f6c69..6e5b5ef077 100644 --- a/include/pybind11/detail/function_record_pyobject.h +++ b/include/pybind11/detail/function_record_pyobject.h @@ -31,8 +31,9 @@ void tp_dealloc_impl(PyObject *self); void tp_free_impl(void *self); static PyObject *reduce_ex_impl(PyObject *self, PyObject *, PyObject *); -static PyObject * -get_capsule_for_scipy_LowLevelCallable_NO_TYPE_SAFETY_impl(PyObject *self, PyObject *, PyObject *); +static PyObject *get_capsule_for_scipy_LowLevelCallable_NO_ABI_OR_TYPE_SAFETY_impl(PyObject *self, + PyObject *, + PyObject *); PYBIND11_WARNING_PUSH #if defined(__GNUC__) && __GNUC__ >= 8 @@ -43,8 +44,8 @@ PYBIND11_WARNING_DISABLE_CLANG("-Wcast-function-type-mismatch") #endif static PyMethodDef tp_methods_impl[] = {{"__reduce_ex__", (PyCFunction) reduce_ex_impl, METH_VARARGS | METH_KEYWORDS, nullptr}, - {"get_capsule_for_scipy_LowLevelCallable_NO_TYPE_SAFETY", - (PyCFunction) get_capsule_for_scipy_LowLevelCallable_NO_TYPE_SAFETY_impl, + {"get_capsule_for_scipy_LowLevelCallable_NO_ABI_OR_TYPE_SAFETY", + (PyCFunction) get_capsule_for_scipy_LowLevelCallable_NO_ABI_OR_TYPE_SAFETY_impl, METH_VARARGS | METH_KEYWORDS, "for use with scipy.LowLevelCallable()"}, {nullptr, nullptr, 0, nullptr}}; @@ -208,9 +209,8 @@ inline PyObject *reduce_ex_impl(PyObject *self, PyObject *, PyObject *) { return nullptr; } -inline PyObject *get_capsule_for_scipy_LowLevelCallable_NO_TYPE_SAFETY_impl(PyObject *self, - PyObject *args, - PyObject *kwargs) { +inline PyObject *get_capsule_for_scipy_LowLevelCallable_NO_ABI_OR_TYPE_SAFETY_impl( + PyObject *self, PyObject *args, PyObject *kwargs) { static const char *kwlist[] = {"signature", nullptr}; const char *signature = nullptr; if (PyArg_ParseTupleAndKeywords(args, kwargs, "s", const_cast(kwlist), &signature) diff --git a/tests/test_scipy_low_level_callable.py b/tests/test_scipy_low_level_callable.py index 9a704a2606..76010d5e35 100644 --- a/tests/test_scipy_low_level_callable.py +++ b/tests/test_scipy_low_level_callable.py @@ -17,8 +17,10 @@ def _m_square21_self(): def test_get_capsule_for_scipy_LowLevelCallable(): - cap = _m_square21_self().get_capsule_for_scipy_LowLevelCallable_NO_TYPE_SAFETY( - signature="double (double)" + cap = ( + _m_square21_self().get_capsule_for_scipy_LowLevelCallable_NO_ABI_OR_TYPE_SAFETY( + signature="double (double)" + ) ) assert repr(cap).startswith(" Date: Fri, 28 Mar 2025 08:51:04 -0700 Subject: [PATCH 16/18] Introduce PYBIND11_ENSURE_PRECONDITION_FOR_FUNCTIONAL_H_PERFORMANCE_OPTIMIZATIONS macro, for readability/maintainability. --- include/pybind11/attr.h | 6 ++++++ include/pybind11/detail/function_record_pyobject.h | 3 ++- include/pybind11/functional.h | 3 ++- include/pybind11/pybind11.h | 3 ++- 4 files changed, 12 insertions(+), 3 deletions(-) diff --git a/include/pybind11/attr.h b/include/pybind11/attr.h index 651949bfc1..786133afe1 100644 --- a/include/pybind11/attr.h +++ b/include/pybind11/attr.h @@ -272,6 +272,12 @@ struct function_record { /// Pointer to next overload function_record *next = nullptr; }; +// The main purpose of this macro is to make it easy to pin-point the critically related code +// sections. +#define PYBIND11_ENSURE_PRECONDITION_FOR_FUNCTIONAL_H_PERFORMANCE_OPTIMIZATIONS(...) \ + static_assert( \ + __VA_ARGS__, \ + "Violation of precondition for pybind11/functional.h performance optimizations!") /// Special data structure which (temporarily) holds metadata about a bound class struct type_record { diff --git a/include/pybind11/detail/function_record_pyobject.h b/include/pybind11/detail/function_record_pyobject.h index 6e5b5ef077..9835eaab76 100644 --- a/include/pybind11/detail/function_record_pyobject.h +++ b/include/pybind11/detail/function_record_pyobject.h @@ -235,7 +235,8 @@ inline PyObject *get_capsule_for_scipy_LowLevelCallable_NO_ABI_OR_TYPE_SAFETY_im return PYBIND11_STD_LAUNDER(reinterpret_cast(data)); } }; - static_assert(std::is_standard_layout::value, ""); + PYBIND11_ENSURE_PRECONDITION_FOR_FUNCTIONAL_H_PERFORMANCE_OPTIMIZATIONS( + std::is_standard_layout::value); auto *tec = type_erased_capture::from_data(rec->data); return capsule(reinterpret_cast(tec->void_func_ptr), signature).release().ptr(); } diff --git a/include/pybind11/functional.h b/include/pybind11/functional.h index e9767e4c2a..77ddf290ef 100644 --- a/include/pybind11/functional.h +++ b/include/pybind11/functional.h @@ -106,7 +106,8 @@ struct type_caster> { return PYBIND11_STD_LAUNDER(reinterpret_cast(data)); } }; - static_assert(std::is_standard_layout::value, ""); + PYBIND11_ENSURE_PRECONDITION_FOR_FUNCTIONAL_H_PERFORMANCE_OPTIMIZATIONS( + std::is_standard_layout::value); value = capture::from_data(rec->data)->f; return true; } diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index f67dae689d..5bf79da4f1 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -489,7 +489,8 @@ class cpp_function : public function { using FunctionType = Return (*)(Args...); constexpr bool is_function_ptr = std::is_convertible::value && sizeof(capture) == sizeof(void *); - static_assert(!is_function_ptr || std::is_standard_layout::value, ""); + PYBIND11_ENSURE_PRECONDITION_FOR_FUNCTIONAL_H_PERFORMANCE_OPTIMIZATIONS( + !is_function_ptr || std::is_standard_layout::value); if (is_function_ptr) { rec->is_stateless = true; rec->data[1] From be4a234c903d0880515f4fc1001d4155873ccd9f Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Fri, 28 Mar 2025 09:52:48 -0700 Subject: [PATCH 17/18] Add static read-only properties to Python function record type: PYBIND11_DETAIL_FUNCTION_RECORD_ABI_ID, PYBIND11_INTERNALS_ID --- .../detail/function_record_pyobject.h | 19 ++++++++++++++++++- tests/test_scipy_low_level_callable.py | 7 +++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/include/pybind11/detail/function_record_pyobject.h b/include/pybind11/detail/function_record_pyobject.h index 9835eaab76..c7767eae62 100644 --- a/include/pybind11/detail/function_record_pyobject.h +++ b/include/pybind11/detail/function_record_pyobject.h @@ -56,6 +56,23 @@ constexpr char tp_name_impl[] = "pybind11_detail_function_record_" PYBIND11_DETAIL_FUNCTION_RECORD_ABI_ID "_" PYBIND11_PLATFORM_ABI_ID; +inline PyObject *get_property_pybind11_detail_function_record_abi_id(PyObject *, void *) { + return PyUnicode_FromString(PYBIND11_DETAIL_FUNCTION_RECORD_ABI_ID); +} + +inline PyObject *get_property_pybind11_platform_abi_id(PyObject *, void *) { + return PyUnicode_FromString(PYBIND11_PLATFORM_ABI_ID); +} + +static PyGetSetDef tp_getset_impl[] = { + {"PYBIND11_DETAIL_FUNCTION_RECORD_ABI_ID", + get_property_pybind11_detail_function_record_abi_id, + nullptr, + nullptr, + nullptr}, + {"PYBIND11_PLATFORM_ABI_ID", get_property_pybind11_platform_abi_id, nullptr, nullptr, nullptr}, + {nullptr, nullptr, nullptr, nullptr, nullptr}}; + PYBIND11_NAMESPACE_END(function_record_PyTypeObject_methods) // Designated initializers are a C++20 feature: @@ -96,7 +113,7 @@ static PyTypeObject function_record_PyTypeObject = { /* iternextfunc tp_iternext */ nullptr, /* struct PyMethodDef *tp_methods */ function_record_PyTypeObject_methods::tp_methods_impl, /* struct PyMemberDef *tp_members */ nullptr, - /* struct PyGetSetDef *tp_getset */ nullptr, + /* struct PyGetSetDef *tp_getset */ function_record_PyTypeObject_methods::tp_getset_impl, /* struct _typeobject *tp_base */ nullptr, /* PyObject *tp_dict */ nullptr, /* descrgetfunc tp_descr_get */ nullptr, diff --git a/tests/test_scipy_low_level_callable.py b/tests/test_scipy_low_level_callable.py index 76010d5e35..5a1173f382 100644 --- a/tests/test_scipy_low_level_callable.py +++ b/tests/test_scipy_low_level_callable.py @@ -2,6 +2,7 @@ import pytest +import pybind11_tests from pybind11_tests import scipy_low_level_callable as m @@ -16,6 +17,12 @@ def _m_square21_self(): pytest.skip(f"{str(e)}") +def test_python_function_record_static_properties(): + func_rec = _m_square21_self() + assert func_rec.PYBIND11_DETAIL_FUNCTION_RECORD_ABI_ID == "v1" + assert func_rec.PYBIND11_PLATFORM_ABI_ID in pybind11_tests.PYBIND11_INTERNALS_ID + + def test_get_capsule_for_scipy_LowLevelCallable(): cap = ( _m_square21_self().get_capsule_for_scipy_LowLevelCallable_NO_ABI_OR_TYPE_SAFETY( From 61c9c6d350db3be54f4a2da38ab278a4aa7f54ec Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Fri, 28 Mar 2025 11:43:45 -0700 Subject: [PATCH 18/18] [ci skip] Response to question by gh-henryiii (https://github.com/pybind/pybind11/pull/5580#discussion_r2019137216) --- tests/test_pickling.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/test_pickling.py b/tests/test_pickling.py index c3b5c63274..98381d9bdc 100644 --- a/tests/test_pickling.py +++ b/tests/test_pickling.py @@ -9,11 +9,12 @@ from pybind11_tests import pickling as m -def test_assumptions(): +def all_pickle_protocols(): assert pickle.HIGHEST_PROTOCOL >= 0 + return range(pickle.HIGHEST_PROTOCOL + 1) -@pytest.mark.parametrize("protocol", range(pickle.HIGHEST_PROTOCOL + 1)) +@pytest.mark.parametrize("protocol", all_pickle_protocols()) def test_pickle_simple_callable(protocol): assert m.simple_callable() == 20220426 serialized = pickle.dumps(m.simple_callable, protocol=protocol)