From 1ac19036d6146024fb5881ac888f7775e16b203d Mon Sep 17 00:00:00 2001 From: Dean Moldovan Date: Thu, 16 Mar 2017 11:22:26 +0100 Subject: [PATCH] Add a scope guard call policy ```c++ m.def("foo", foo, py::call_guard()); ``` is equivalent to: ```c++ m.def("foo", [](args...) { T scope_guard; return foo(args...); // forwarded arguments }); ``` --- docs/advanced/functions.rst | 46 ++++++++-- docs/advanced/misc.rst | 8 ++ include/pybind11/attr.h | 48 ++++++++++ include/pybind11/cast.h | 12 +-- include/pybind11/common.h | 10 +- include/pybind11/pybind11.h | 5 +- tests/CMakeLists.txt | 2 +- tests/test_call_policies.cpp | 91 +++++++++++++++++++ ...st_keep_alive.py => test_call_policies.py} | 14 +++ tests/test_keep_alive.cpp | 40 -------- 10 files changed, 217 insertions(+), 59 deletions(-) create mode 100644 tests/test_call_policies.cpp rename tests/{test_keep_alive.py => test_call_policies.py} (81%) delete mode 100644 tests/test_keep_alive.cpp diff --git a/docs/advanced/functions.rst b/docs/advanced/functions.rst index e0b0fe095..b11a2ab61 100644 --- a/docs/advanced/functions.rst +++ b/docs/advanced/functions.rst @@ -162,14 +162,16 @@ Additional call policies ======================== In addition to the above return value policies, further *call policies* can be -specified to indicate dependencies between parameters. In general, call policies -are required when the C++ object is any kind of container and another object is being -added to the container. +specified to indicate dependencies between parameters or ensure a certain state +for the function call. -There is currently just -one policy named ``keep_alive``, which indicates that the -argument with index ``Patient`` should be kept alive at least until the -argument with index ``Nurse`` is freed by the garbage collector. Argument +Keep alive +---------- + +In general, this policy is required when the C++ object is any kind of container +and another object is being added to the container. ``keep_alive`` +indicates that the argument with index ``Patient`` should be kept alive at least +until the argument with index ``Nurse`` is freed by the garbage collector. Argument indices start at one, while zero refers to the return value. For methods, index ``1`` refers to the implicit ``this`` pointer, while regular arguments begin at index ``2``. Arbitrarily many call policies can be specified. When a ``Nurse`` @@ -194,10 +196,36 @@ container: Patient != 0) and ``with_custodian_and_ward_postcall`` (if Nurse/Patient == 0) policies from Boost.Python. +Call guard +---------- + +The ``call_guard`` policy allows any scope guard type ``T`` to be placed +around the function call. For example, this definition: + +.. code-block:: cpp + + m.def("foo", foo, py::call_guard()); + +is equivalent to the following pseudocode: + +.. code-block:: cpp + + m.def("foo", [](args...) { + T scope_guard; + return foo(args...); // forwarded arguments + }); + +The only requirement is that ``T`` is default-constructible, but otherwise any +scope guard will work. This is very useful in combination with `gil_scoped_release`. +See :ref:`gil`. + +Multiple guards can also be specified as ``py::call_guard``. The +constructor order is left to right and destruction happens in reverse. + .. seealso:: - The file :file:`tests/test_keep_alive.cpp` contains a complete example - that demonstrates using :class:`keep_alive` in more detail. + The file :file:`tests/test_call_policies.cpp` contains a complete example + that demonstrates using `keep_alive` and `call_guard` in more detail. .. _python_objects_as_args: diff --git a/docs/advanced/misc.rst b/docs/advanced/misc.rst index d98466512..6ebc0c343 100644 --- a/docs/advanced/misc.rst +++ b/docs/advanced/misc.rst @@ -15,6 +15,7 @@ T2>, myFunc)``. In this case, the preprocessor assumes that the comma indicates the beginning of the next parameter. Use a ``typedef`` to bind the template to another name and use it in the macro to avoid this problem. +.. _gil: Global Interpreter Lock (GIL) ============================= @@ -68,6 +69,13 @@ could be realized as follows (important changes highlighted): return m.ptr(); } +The ``call_go`` wrapper can also be simplified using the `call_guard` policy +(see :ref:`call_policies`) which yields the same result: + +.. code-block:: cpp + + m.def("call_go", &call_go, py::call_guard()); + Binding sequence data types, iterators, the slicing protocol, etc. ================================================================== diff --git a/include/pybind11/attr.h b/include/pybind11/attr.h index e38a1a32d..7fbdfcbc3 100644 --- a/include/pybind11/attr.h +++ b/include/pybind11/attr.h @@ -67,6 +67,44 @@ struct metaclass { /// Annotation to mark enums as an arithmetic type struct arithmetic { }; +/** \rst + A call policy which places one or more guard variables (``Ts...``) around the function call. + + For example, this definition: + + .. code-block:: cpp + + m.def("foo", foo, py::call_guard()); + + is equivalent to the following pseudocode: + + .. code-block:: cpp + + m.def("foo", [](args...) { + T scope_guard; + return foo(args...); // forwarded arguments + }); + \endrst */ +template struct call_guard; + +template <> struct call_guard<> { using type = detail::void_type; }; + +template +struct call_guard { + static_assert(std::is_default_constructible::value, + "The guard type must be default constructible"); + + using type = T; +}; + +template +struct call_guard { + struct type { + T guard{}; // Compose multiple guard types with left-to-right default-constructor order + typename call_guard::type next{}; + }; +}; + /// @} annotations NAMESPACE_BEGIN(detail) @@ -374,6 +412,9 @@ struct process_attribute : process_attribute_default { template <> struct process_attribute : process_attribute_default {}; +template +struct process_attribute> : process_attribute_default> { }; + /*** * Process a keep_alive call policy -- invokes keep_alive_impl during the * pre-call handler if both Nurse, Patient != 0 and use the post-call handler @@ -410,6 +451,13 @@ template struct process_attributes { } }; +template +using is_call_guard = is_instantiation; + +/// Extract the ``type`` from the first `call_guard` in `Extras...` (or `void_type` if none found) +template +using extract_guard_t = typename first_of_t, Extra...>::type; + /// Check the number of named arguments at compile time template ::value...), diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index fe19075e4..0beaf6e48 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1383,14 +1383,14 @@ public: return load_impl_sequence(call, indices{}); } - template + template enable_if_t::value, Return> call(Func &&f) { - return call_impl(std::forward(f), indices{}); + return call_impl(std::forward(f), indices{}, Guard{}); } - template + template enable_if_t::value, void_type> call(Func &&f) { - call_impl(std::forward(f), indices{}); + call_impl(std::forward(f), indices{}, Guard{}); return void_type(); } @@ -1406,8 +1406,8 @@ private: return true; } - template - Return call_impl(Func &&f, index_sequence) { + template + Return call_impl(Func &&f, index_sequence, Guard &&) { return std::forward(f)(cast_op(std::get(value))...); } diff --git a/include/pybind11/common.h b/include/pybind11/common.h index 3d78ae1b4..9e6b0e1a6 100644 --- a/include/pybind11/common.h +++ b/include/pybind11/common.h @@ -536,9 +536,15 @@ using is_template_base_of = decltype(is_template_base_of_impl::check((remo struct is_template_base_of : decltype(is_template_base_of_impl::check((remove_cv_t*)nullptr)) { }; #endif +/// Check if T is an instantiation of the template `Class`. For example: +/// `is_instantiation` is true if `T == shared_ptr` where U can be anything. +template class Class, typename T> +struct is_instantiation : std::false_type { }; +template class Class, typename... Us> +struct is_instantiation> : std::true_type { }; + /// Check if T is std::shared_ptr where U can be anything -template struct is_shared_ptr : std::false_type { }; -template struct is_shared_ptr> : std::true_type { }; +template using is_shared_ptr = is_instantiation; /// Ignore that a variable is unused in compiler warnings inline void ignore_unused(const int *) { } diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 36a12e95a..9a3af8e0e 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -143,8 +143,11 @@ protected: /* Override policy for rvalues -- usually to enforce rvp::move on an rvalue */ const auto policy = detail::return_value_policy_override::policy(call.func.policy); + /* Function scope guard -- defaults to the compile-to-nothing `void_type` */ + using Guard = detail::extract_guard_t; + /* Perform the function call */ - handle result = cast_out::cast(args_converter.template call(cap->f), + handle result = cast_out::cast(args_converter.template call(cap->f), policy, call.parent); /* Invoke call policy post-call hook */ diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 11be49e53..132edbfb8 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -28,6 +28,7 @@ endif() set(PYBIND11_TEST_FILES test_alias_initialization.cpp test_buffers.cpp + test_call_policies.cpp test_callbacks.cpp test_chrono.cpp test_class_args.cpp @@ -40,7 +41,6 @@ set(PYBIND11_TEST_FILES test_exceptions.cpp test_inheritance.cpp test_issues.cpp - test_keep_alive.cpp test_kwargs_and_defaults.cpp test_methods_and_attributes.cpp test_modules.cpp diff --git a/tests/test_call_policies.cpp b/tests/test_call_policies.cpp new file mode 100644 index 000000000..33a3f15ee --- /dev/null +++ b/tests/test_call_policies.cpp @@ -0,0 +1,91 @@ +/* + tests/test_call_policies.cpp -- keep_alive and call_guard + + Copyright (c) 2016 Wenzel Jakob + + All rights reserved. Use of this source code is governed by a + BSD-style license that can be found in the LICENSE file. +*/ + +#include "pybind11_tests.h" + +class Child { +public: + Child() { py::print("Allocating child."); } + ~Child() { py::print("Releasing child."); } +}; + +class Parent { +public: + Parent() { py::print("Allocating parent."); } + ~Parent() { py::print("Releasing parent."); } + void addChild(Child *) { } + Child *returnChild() { return new Child(); } + Child *returnNullChild() { return nullptr; } +}; + +test_initializer keep_alive([](py::module &m) { + py::class_(m, "Parent") + .def(py::init<>()) + .def("addChild", &Parent::addChild) + .def("addChildKeepAlive", &Parent::addChild, py::keep_alive<1, 2>()) + .def("returnChild", &Parent::returnChild) + .def("returnChildKeepAlive", &Parent::returnChild, py::keep_alive<1, 0>()) + .def("returnNullChildKeepAliveChild", &Parent::returnNullChild, py::keep_alive<1, 0>()) + .def("returnNullChildKeepAliveParent", &Parent::returnNullChild, py::keep_alive<0, 1>()); + + py::class_(m, "Child") + .def(py::init<>()); +}); + +struct CustomGuard { + static bool enabled; + + CustomGuard() { enabled = true; } + ~CustomGuard() { enabled = false; } + + static const char *report_status() { return enabled ? "guarded" : "unguarded"; } +}; + +bool CustomGuard::enabled = false; + +struct DependentGuard { + static bool enabled; + + DependentGuard() { enabled = CustomGuard::enabled; } + ~DependentGuard() { enabled = false; } + + static const char *report_status() { return enabled ? "guarded" : "unguarded"; } +}; + +bool DependentGuard::enabled = false; + +test_initializer call_guard([](py::module &pm) { + auto m = pm.def_submodule("call_policies"); + + m.def("unguarded_call", &CustomGuard::report_status); + m.def("guarded_call", &CustomGuard::report_status, py::call_guard()); + + m.def("multiple_guards_correct_order", []() { + return CustomGuard::report_status() + std::string(" & ") + DependentGuard::report_status(); + }, py::call_guard()); + + m.def("multiple_guards_wrong_order", []() { + return DependentGuard::report_status() + std::string(" & ") + CustomGuard::report_status(); + }, py::call_guard()); + +#if defined(WITH_THREAD) && !defined(PYPY_VERSION) + // `py::call_guard()` should work in PyPy as well, + // but it's unclear how to test it without `PyGILState_GetThisThreadState`. + auto report_gil_status = []() { + auto is_gil_held = false; + if (auto tstate = py::detail::get_thread_state_unchecked()) + is_gil_held = (tstate == PyGILState_GetThisThreadState()); + + return is_gil_held ? "GIL held" : "GIL released"; + }; + + m.def("with_gil", report_gil_status); + m.def("without_gil", report_gil_status, py::call_guard()); +#endif +}); diff --git a/tests/test_keep_alive.py b/tests/test_call_policies.py similarity index 81% rename from tests/test_keep_alive.py rename to tests/test_call_policies.py index bfd7d40c3..1cb11c222 100644 --- a/tests/test_keep_alive.py +++ b/tests/test_call_policies.py @@ -95,3 +95,17 @@ def test_return_none(capture): del p pytest.gc_collect() assert capture == "Releasing parent." + + +def test_call_guard(): + from pybind11_tests import call_policies + + assert call_policies.unguarded_call() == "unguarded" + assert call_policies.guarded_call() == "guarded" + + assert call_policies.multiple_guards_correct_order() == "guarded & guarded" + assert call_policies.multiple_guards_wrong_order() == "unguarded & guarded" + + if hasattr(call_policies, "with_gil"): + assert call_policies.with_gil() == "GIL held" + assert call_policies.without_gil() == "GIL released" diff --git a/tests/test_keep_alive.cpp b/tests/test_keep_alive.cpp deleted file mode 100644 index cd62a02e8..000000000 --- a/tests/test_keep_alive.cpp +++ /dev/null @@ -1,40 +0,0 @@ -/* - tests/test_keep_alive.cpp -- keep_alive modifier (pybind11's version - of Boost.Python's with_custodian_and_ward / with_custodian_and_ward_postcall) - - Copyright (c) 2016 Wenzel Jakob - - All rights reserved. Use of this source code is governed by a - BSD-style license that can be found in the LICENSE file. -*/ - -#include "pybind11_tests.h" - -class Child { -public: - Child() { py::print("Allocating child."); } - ~Child() { py::print("Releasing child."); } -}; - -class Parent { -public: - Parent() { py::print("Allocating parent."); } - ~Parent() { py::print("Releasing parent."); } - void addChild(Child *) { } - Child *returnChild() { return new Child(); } - Child *returnNullChild() { return nullptr; } -}; - -test_initializer keep_alive([](py::module &m) { - py::class_(m, "Parent") - .def(py::init<>()) - .def("addChild", &Parent::addChild) - .def("addChildKeepAlive", &Parent::addChild, py::keep_alive<1, 2>()) - .def("returnChild", &Parent::returnChild) - .def("returnChildKeepAlive", &Parent::returnChild, py::keep_alive<1, 0>()) - .def("returnNullChildKeepAliveChild", &Parent::returnNullChild, py::keep_alive<1, 0>()) - .def("returnNullChildKeepAliveParent", &Parent::returnNullChild, py::keep_alive<0, 1>()); - - py::class_(m, "Child") - .def(py::init<>()); -});