Skip to content

Add a scope guard call policy #740

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 2, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 38 additions & 10 deletions docs/advanced/functions.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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.

There is currently just
one policy named ``keep_alive<Nurse, Patient>``, 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
specified to indicate dependencies between parameters or ensure a certain state
for the function call.

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<Nurse, Patient>``
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``
Expand All @@ -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<T>`` 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<T>());

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<T1, T2, T3...>``. 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:

Expand Down
8 changes: 8 additions & 0 deletions docs/advanced/misc.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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)
=============================
Expand Down Expand Up @@ -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<py::gil_scoped_release>());


Binding sequence data types, iterators, the slicing protocol, etc.
==================================================================
Expand Down
48 changes: 48 additions & 0 deletions include/pybind11/attr.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<T>());

is equivalent to the following pseudocode:

.. code-block:: cpp

m.def("foo", [](args...) {
T scope_guard;
return foo(args...); // forwarded arguments
});
\endrst */
template <typename... Ts> struct call_guard;

template <> struct call_guard<> { using type = detail::void_type; };

template <typename T>
struct call_guard<T> {
static_assert(std::is_default_constructible<T>::value,
"The guard type must be default constructible");

using type = T;
};

template <typename T, typename... Ts>
struct call_guard<T, Ts...> {
struct type {
T guard{}; // Compose multiple guard types with left-to-right default-constructor order
typename call_guard<Ts...>::type next{};
};
};

/// @} annotations

NAMESPACE_BEGIN(detail)
Expand Down Expand Up @@ -374,6 +412,9 @@ struct process_attribute<metaclass> : process_attribute_default<metaclass> {
template <>
struct process_attribute<arithmetic> : process_attribute_default<arithmetic> {};

template <typename... Ts>
struct process_attribute<call_guard<Ts...>> : process_attribute_default<call_guard<Ts...>> { };

/***
* 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
Expand Down Expand Up @@ -410,6 +451,13 @@ template <typename... Args> struct process_attributes {
}
};

template <typename T>
using is_call_guard = is_instantiation<call_guard, T>;

/// Extract the ``type`` from the first `call_guard` in `Extras...` (or `void_type` if none found)
template <typename... Extra>
using extract_guard_t = typename exactly_one_t<is_call_guard, call_guard<>, Extra...>::type;

/// Check the number of named arguments at compile time
template <typename... Extra,
size_t named = constexpr_sum(std::is_base_of<arg, Extra>::value...),
Expand Down
12 changes: 6 additions & 6 deletions include/pybind11/cast.h
Original file line number Diff line number Diff line change
Expand Up @@ -1383,14 +1383,14 @@ class argument_loader {
return load_impl_sequence(call, indices{});
}

template <typename Return, typename Func>
template <typename Return, typename Guard, typename Func>
enable_if_t<!std::is_void<Return>::value, Return> call(Func &&f) {
return call_impl<Return>(std::forward<Func>(f), indices{});
return call_impl<Return>(std::forward<Func>(f), indices{}, Guard{});
}

template <typename Return, typename Func>
template <typename Return, typename Guard, typename Func>
enable_if_t<std::is_void<Return>::value, void_type> call(Func &&f) {
call_impl<Return>(std::forward<Func>(f), indices{});
call_impl<Return>(std::forward<Func>(f), indices{}, Guard{});
return void_type();
}

Expand All @@ -1406,8 +1406,8 @@ class argument_loader {
return true;
}

template <typename Return, typename Func, size_t... Is>
Return call_impl(Func &&f, index_sequence<Is...>) {
template <typename Return, typename Func, size_t... Is, typename Guard>
Return call_impl(Func &&f, index_sequence<Is...>, Guard &&) {
return std::forward<Func>(f)(cast_op<Args>(std::get<Is>(value))...);
}

Expand Down
43 changes: 28 additions & 15 deletions include/pybind11/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -502,20 +502,27 @@ constexpr int constexpr_first() { return constexpr_impl::first(0, Predicate<Ts>:
template <template<typename> class Predicate, typename... Ts>
constexpr int constexpr_last() { return constexpr_impl::last(0, -1, Predicate<Ts>::value...); }

// Extracts the first type from the template parameter pack matching the predicate, or Default if none match.
template <template<class> class Predicate, class Default, class... Ts> struct first_of;
template <template<class> class Predicate, class Default> struct first_of<Predicate, Default> {
using type = Default;
};
template <template<class> class Predicate, class Default, class T, class... Ts>
struct first_of<Predicate, Default, T, Ts...> {
using type = typename std::conditional<
Predicate<T>::value,
T,
typename first_of<Predicate, Default, Ts...>::type
>::type;
/// Return the Nth element from the parameter pack
template <size_t N, typename T, typename... Ts>
struct pack_element { using type = typename pack_element<N - 1, Ts...>::type; };
template <typename T, typename... Ts>
struct pack_element<0, T, Ts...> { using type = T; };

/// Return the one and only type which matches the predicate, or Default if none match.
/// If more than one type matches the predicate, fail at compile-time.
template <template<typename> class Predicate, typename Default, typename... Ts>
struct exactly_one {
static constexpr auto found = constexpr_sum(Predicate<Ts>::value...);
static_assert(found <= 1, "Found more than one type matching the predicate");

static constexpr auto index = found ? constexpr_first<Predicate, Ts...>() : 0;
using type = conditional_t<found, typename pack_element<index, Ts...>::type, Default>;
};
template <template<class> class Predicate, class Default, class... T> using first_of_t = typename first_of<Predicate, Default, T...>::type;
template <template<typename> class P, typename Default>
struct exactly_one<P, Default> { using type = Default; };

template <template<typename> class Predicate, typename Default, typename... Ts>
using exactly_one_t = typename exactly_one<Predicate, Default, Ts...>::type;

/// Defer the evaluation of type T until types Us are instantiated
template <typename T, typename... /*Us*/> struct deferred_type { using type = T; };
Expand All @@ -536,9 +543,15 @@ using is_template_base_of = decltype(is_template_base_of_impl<Base>::check((remo
struct is_template_base_of : decltype(is_template_base_of_impl<Base>::check((remove_cv_t<T>*)nullptr)) { };
#endif

/// Check if T is an instantiation of the template `Class`. For example:
/// `is_instantiation<shared_ptr, T>` is true if `T == shared_ptr<U>` where U can be anything.
template <template<typename...> class Class, typename T>
struct is_instantiation : std::false_type { };
template <template<typename...> class Class, typename... Us>
struct is_instantiation<Class, Class<Us...>> : std::true_type { };

/// Check if T is std::shared_ptr<U> where U can be anything
template <typename T> struct is_shared_ptr : std::false_type { };
template <typename U> struct is_shared_ptr<std::shared_ptr<U>> : std::true_type { };
template <typename T> using is_shared_ptr = is_instantiation<std::shared_ptr, T>;

/// Ignore that a variable is unused in compiler warnings
inline void ignore_unused(const int *) { }
Expand Down
14 changes: 7 additions & 7 deletions include/pybind11/pybind11.h
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,11 @@ class cpp_function : public function {
/* Override policy for rvalues -- usually to enforce rvp::move on an rvalue */
const auto policy = detail::return_value_policy_override<Return>::policy(call.func.policy);

/* Function scope guard -- defaults to the compile-to-nothing `void_type` */
Copy link
Member

@jagerman jagerman Apr 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think `void_type` should be `call_guard<>`

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scratch that, I see now: it's going to be call_guard<>::type, which is the void_type.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the 0 and 1 argument versions just return the type directly since those are the most common cases. The additional wrappers start with 2 arguments and above.

using Guard = detail::extract_guard_t<Extra...>;

/* Perform the function call */
handle result = cast_out::cast(args_converter.template call<Return>(cap->f),
handle result = cast_out::cast(args_converter.template call<Return, Guard>(cap->f),
policy, call.parent);

/* Invoke call policy post-call hook */
Expand Down Expand Up @@ -886,9 +889,9 @@ class class_ : public detail::generic_type {

public:
using type = type_;
using type_alias = detail::first_of_t<is_subtype, void, options...>;
using type_alias = detail::exactly_one_t<is_subtype, void, options...>;
constexpr static bool has_alias = !std::is_void<type_alias>::value;
using holder_type = detail::first_of_t<is_holder, std::unique_ptr<type>, options...>;
using holder_type = detail::exactly_one_t<is_holder, std::unique_ptr<type>, options...>;
using instance_type = detail::instance<type, holder_type>;

static_assert(detail::all_of<is_valid_class_option<options>...>::value,
Expand Down Expand Up @@ -1156,15 +1159,12 @@ template <typename Type> class enum_ : public class_<Type> {
using class_<Type>::def;
using class_<Type>::def_property_readonly_static;
using Scalar = typename std::underlying_type<Type>::type;
template <typename T> using arithmetic_tag = std::is_same<T, arithmetic>;

template <typename... Extra>
enum_(const handle &scope, const char *name, const Extra&... extra)
: class_<Type>(scope, name, extra...), m_entries(), m_parent(scope) {

constexpr bool is_arithmetic =
!std::is_same<detail::first_of_t<arithmetic_tag, void, Extra...>,
void>::value;
constexpr bool is_arithmetic = detail::any_of<std::is_same<arithmetic, Extra>...>::value;

auto m_entries_ptr = m_entries.inc_ref().ptr();
def("__repr__", [name, m_entries_ptr](Type value) -> pybind11::str {
Expand Down
2 changes: 1 addition & 1 deletion tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
91 changes: 91 additions & 0 deletions tests/test_call_policies.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
/*
tests/test_call_policies.cpp -- keep_alive and call_guard

Copyright (c) 2016 Wenzel Jakob <[email protected]>

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_<Parent>(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_<Child>(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<CustomGuard>());

m.def("multiple_guards_correct_order", []() {
return CustomGuard::report_status() + std::string(" & ") + DependentGuard::report_status();
}, py::call_guard<CustomGuard, DependentGuard>());

m.def("multiple_guards_wrong_order", []() {
return DependentGuard::report_status() + std::string(" & ") + CustomGuard::report_status();
}, py::call_guard<DependentGuard, CustomGuard>());

#if defined(WITH_THREAD) && !defined(PYPY_VERSION)
// `py::call_guard<py::gil_scoped_release>()` 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<py::gil_scoped_release>());
#endif
});
Loading