From dff76b85735d2ad62aa2f47cde17d3bacfdfb865 Mon Sep 17 00:00:00 2001 From: Xiaofei Wang <6218006+wangxf123456@users.noreply.github.com> Date: Wed, 29 Mar 2023 16:13:09 -0700 Subject: [PATCH 1/7] Allow specializations based on callback function return values. --- include/pybind11/functional.h | 77 +++++++++++++++++++---------------- 1 file changed, 43 insertions(+), 34 deletions(-) diff --git a/include/pybind11/functional.h b/include/pybind11/functional.h index 87ec4d10cb..b4528cf7ca 100644 --- a/include/pybind11/functional.h +++ b/include/pybind11/functional.h @@ -15,6 +15,47 @@ PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) PYBIND11_NAMESPACE_BEGIN(detail) +PYBIND11_NAMESPACE_BEGIN(type_caster_std_function_specializations) + +// ensure GIL is held during functor destruction +struct func_handle { + function f; +#if !(defined(_MSC_VER) && _MSC_VER == 1916 && defined(PYBIND11_CPP17)) + // This triggers a syntax error under very special conditions (very weird indeed). + explicit +#endif + func_handle(function &&f_) noexcept + : f(std::move(f_)) { + } + func_handle(const func_handle &f_) { operator=(f_); } + func_handle &operator=(const func_handle &f_) { + gil_scoped_acquire acq; + f = f_.f; + return *this; + } + ~func_handle() { + gil_scoped_acquire acq; + function kill_f(std::move(f)); + } +}; + +// to emulate 'move initialization capture' in C++11 +struct func_wrapper_base { + func_handle hfunc; + explicit func_wrapper_base(func_handle &&hf) noexcept : hfunc(std::move(hf)) {} +}; + +template <typename Return, typename... Args> +struct func_wrapper : func_wrapper_base { + using func_wrapper_base::func_wrapper_base; + Return operator()(Args... args) const { + gil_scoped_acquire acq; + // casts the returned object as a rvalue to the return type + return hfunc.f(std::forward<Args>(args)...).template cast<Return>(); + } +}; + +PYBIND11_NAMESPACE_END(type_caster_std_function_specializations) template <typename Return, typename... Args> struct type_caster<std::function<Return(Args...)>> { @@ -77,40 +118,8 @@ struct type_caster<std::function<Return(Args...)>> { // See PR #1413 for full details } - // ensure GIL is held during functor destruction - struct func_handle { - function f; -#if !(defined(_MSC_VER) && _MSC_VER == 1916 && defined(PYBIND11_CPP17)) - // This triggers a syntax error under very special conditions (very weird indeed). - explicit -#endif - func_handle(function &&f_) noexcept - : f(std::move(f_)) { - } - func_handle(const func_handle &f_) { operator=(f_); } - func_handle &operator=(const func_handle &f_) { - gil_scoped_acquire acq; - f = f_.f; - return *this; - } - ~func_handle() { - gil_scoped_acquire acq; - function kill_f(std::move(f)); - } - }; - - // to emulate 'move initialization capture' in C++11 - struct func_wrapper { - func_handle hfunc; - explicit func_wrapper(func_handle &&hf) noexcept : hfunc(std::move(hf)) {} - Return operator()(Args... args) const { - gil_scoped_acquire acq; - // casts the returned object as a rvalue to the return type - return hfunc.f(std::forward<Args>(args)...).template cast<Return>(); - } - }; - - value = func_wrapper(func_handle(std::move(func))); + value = type_caster_std_function_specializations::func_wrapper<Return, Args...>( + type_caster_std_function_specializations::func_handle(std::move(func))); return true; } From a403cd65f17a2137edb30719a54760b2b3226644 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" <rwgk@google.com> Date: Wed, 29 Mar 2023 17:49:07 -0700 Subject: [PATCH 2/7] clang-tidy auto fix --- include/pybind11/functional.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/pybind11/functional.h b/include/pybind11/functional.h index b4528cf7ca..53c9e41f2d 100644 --- a/include/pybind11/functional.h +++ b/include/pybind11/functional.h @@ -42,7 +42,7 @@ struct func_handle { // to emulate 'move initialization capture' in C++11 struct func_wrapper_base { func_handle hfunc; - explicit func_wrapper_base(func_handle &&hf) noexcept : hfunc(std::move(hf)) {} + explicit func_wrapper_base(func_handle &&hf) noexcept : hfunc(hf) {} }; template <typename Return, typename... Args> From 2062f4dc85552a9eca1e66836a0580af013af18c Mon Sep 17 00:00:00 2001 From: Xiaofei Wang <6218006+wangxf123456@users.noreply.github.com> Date: Thu, 30 Mar 2023 14:20:46 -0700 Subject: [PATCH 3/7] Add a test case for function specialization. --- tests/CMakeLists.txt | 1 + ...pe_caster_std_function_specializations.cpp | 41 +++++++++++++++++++ ...ype_caster_std_function_specializations.py | 9 ++++ 3 files changed, 51 insertions(+) create mode 100644 tests/test_type_caster_std_function_specializations.cpp create mode 100644 tests/test_type_caster_std_function_specializations.py diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index b1cb222b4a..7d0a72df7f 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -154,6 +154,7 @@ set(PYBIND11_TEST_FILES test_stl_binders test_tagbased_polymorphic test_thread + test_type_caster_std_function_specializations test_union test_virtual_functions) diff --git a/tests/test_type_caster_std_function_specializations.cpp b/tests/test_type_caster_std_function_specializations.cpp new file mode 100644 index 0000000000..961e45cade --- /dev/null +++ b/tests/test_type_caster_std_function_specializations.cpp @@ -0,0 +1,41 @@ +#include <pybind11/functional.h> +#include <pybind11/pybind11.h> + +#include "pybind11_tests.h" + +namespace py = pybind11; + +namespace { + +struct SpecialReturn { + int value = 99; +}; + +} // namespace + +namespace pybind11 { +namespace detail { +namespace type_caster_std_function_specializations { + +template <typename... Args> +struct func_wrapper<SpecialReturn, Args...> : func_wrapper_base { + using func_wrapper_base::func_wrapper_base; + SpecialReturn operator()(Args... args) const { + gil_scoped_acquire acq; + SpecialReturn result = hfunc.f(std::forward<Args>(args)...).template cast<SpecialReturn>(); + result.value += 100; + return result; + } +}; + +} // namespace type_caster_std_function_specializations +} // namespace detail +} // namespace pybind11 + +TEST_SUBMODULE(type_caster_std_function_specializations, m) { + py::class_<SpecialReturn>(m, "SpecialReturn") + .def(py::init<>()) + .def_readwrite("value", &SpecialReturn::value); + m.def("call_callback_with_special_return", + [](const std::function<SpecialReturn()> &func) { return func(); }); +} diff --git a/tests/test_type_caster_std_function_specializations.py b/tests/test_type_caster_std_function_specializations.py new file mode 100644 index 0000000000..c722e1a3af --- /dev/null +++ b/tests/test_type_caster_std_function_specializations.py @@ -0,0 +1,9 @@ +from pybind11_tests import type_caster_std_function_specializations as m + + +def test_callback(): + def func(): + return m.SpecialReturn() + + assert func().value == 99 + assert m.call_callback_with_special_return(func).value == 199 From aade073190a4518eeb0d54492b9c17ccec992bd2 Mon Sep 17 00:00:00 2001 From: Xiaofei Wang <6218006+wangxf123456@users.noreply.github.com> Date: Thu, 30 Mar 2023 15:11:52 -0700 Subject: [PATCH 4/7] Add test for callback function that raises Python exception. --- ...test_type_caster_std_function_specializations.cpp | 7 ++++++- .../test_type_caster_std_function_specializations.py | 12 ++++++++---- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/tests/test_type_caster_std_function_specializations.cpp b/tests/test_type_caster_std_function_specializations.cpp index 961e45cade..f28f68203a 100644 --- a/tests/test_type_caster_std_function_specializations.cpp +++ b/tests/test_type_caster_std_function_specializations.cpp @@ -22,7 +22,12 @@ struct func_wrapper<SpecialReturn, Args...> : func_wrapper_base { using func_wrapper_base::func_wrapper_base; SpecialReturn operator()(Args... args) const { gil_scoped_acquire acq; - SpecialReturn result = hfunc.f(std::forward<Args>(args)...).template cast<SpecialReturn>(); + SpecialReturn result; + try { + result = hfunc.f(std::forward<Args>(args)...).template cast<SpecialReturn>(); + } catch (error_already_set &e) { + result.value += 1; + } result.value += 100; return result; } diff --git a/tests/test_type_caster_std_function_specializations.py b/tests/test_type_caster_std_function_specializations.py index c722e1a3af..7549670b44 100644 --- a/tests/test_type_caster_std_function_specializations.py +++ b/tests/test_type_caster_std_function_specializations.py @@ -1,9 +1,13 @@ from pybind11_tests import type_caster_std_function_specializations as m -def test_callback(): - def func(): +def test_callback_with_special_return(): + def return_special(): return m.SpecialReturn() - assert func().value == 99 - assert m.call_callback_with_special_return(func).value == 199 + def raise_exception(): + raise ValueError("called raise_exception.") + + assert return_special().value == 99 + assert m.call_callback_with_special_return(return_special).value == 199 + assert m.call_callback_with_special_return(raise_exception).value == 200 From 6e29a3669ef2bcebc5443daa6e753712fca960d7 Mon Sep 17 00:00:00 2001 From: Xiaofei Wang <6218006+wangxf123456@users.noreply.github.com> Date: Thu, 30 Mar 2023 15:28:23 -0700 Subject: [PATCH 5/7] Fix test failures. --- tests/test_type_caster_std_function_specializations.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_type_caster_std_function_specializations.cpp b/tests/test_type_caster_std_function_specializations.cpp index f28f68203a..d0d0e942aa 100644 --- a/tests/test_type_caster_std_function_specializations.cpp +++ b/tests/test_type_caster_std_function_specializations.cpp @@ -25,8 +25,8 @@ struct func_wrapper<SpecialReturn, Args...> : func_wrapper_base { SpecialReturn result; try { result = hfunc.f(std::forward<Args>(args)...).template cast<SpecialReturn>(); - } catch (error_already_set &e) { - result.value += 1; + } catch (error_already_set &) { + result.value += 1; } result.value += 100; return result; From f532f62521079241a130df1043ee2945b23f6956 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 30 Mar 2023 22:29:08 +0000 Subject: [PATCH 6/7] style: pre-commit fixes --- tests/test_type_caster_std_function_specializations.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_type_caster_std_function_specializations.cpp b/tests/test_type_caster_std_function_specializations.cpp index d0d0e942aa..89213ddb19 100644 --- a/tests/test_type_caster_std_function_specializations.cpp +++ b/tests/test_type_caster_std_function_specializations.cpp @@ -26,7 +26,7 @@ struct func_wrapper<SpecialReturn, Args...> : func_wrapper_base { try { result = hfunc.f(std::forward<Args>(args)...).template cast<SpecialReturn>(); } catch (error_already_set &) { - result.value += 1; + result.value += 1; } result.value += 100; return result; From ced63ef3b830816c692c4bf24046c981ecec0d8f Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" <rwgk@google.com> Date: Fri, 9 Aug 2024 12:35:27 -0700 Subject: [PATCH 7/7] Add `#define PYBIND11_HAS_TYPE_CASTER_STD_FUNCTION_SPECIALIZATIONS` --- include/pybind11/functional.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/pybind11/functional.h b/include/pybind11/functional.h index 5846b76713..4b3610117c 100644 --- a/include/pybind11/functional.h +++ b/include/pybind11/functional.h @@ -9,6 +9,8 @@ #pragma once +#define PYBIND11_HAS_TYPE_CASTER_STD_FUNCTION_SPECIALIZATIONS + #include "pybind11.h" #include <functional>