Pack all function call data into a single struct

This cleans up the previous commit slightly by further reducing the
function call arguments to a single struct (containing the
function_record, arguments vector, and parent).

Although this doesn't currently change anything, it does allow for
future functionality to have a place for precalls to store temporary
objects that need to be destroyed after a function call (whether or not
the call succeeds).

As a concrete example, with this change #625 could be easily implemented
(I think) by adding a std::unique_ptr<gil_scoped_release> member to the
`function_call` struct with a precall that actually constructs it.
Without this, the precall can't do that: the postcall won't be invoked
if the call throws an exception.

This doesn't seems to affect the .so size noticeably (either way).
This commit is contained in:
Jason Rhinelander 2017-01-30 13:34:38 -05:00 committed by Wenzel Jakob
parent 70ed2a4897
commit bfcf952e01
3 changed files with 66 additions and 44 deletions

View File

@ -69,7 +69,8 @@ struct undefined_t;
template <op_id id, op_type ot, typename L = undefined_t, typename R = undefined_t> struct op_; template <op_id id, op_type ot, typename L = undefined_t, typename R = undefined_t> struct op_;
template <typename... Args> struct init; template <typename... Args> struct init;
template <typename... Args> struct init_alias; template <typename... Args> struct init_alias;
inline void keep_alive_impl(size_t Nurse, size_t Patient, function_arguments args, handle ret); struct function_call;
inline void keep_alive_impl(size_t Nurse, size_t Patient, function_call &call, handle ret);
/// Internal data structure which holds metadata about a keyword argument /// Internal data structure which holds metadata about a keyword argument
struct argument_record { struct argument_record {
@ -100,7 +101,7 @@ struct function_record {
std::vector<argument_record> args; std::vector<argument_record> args;
/// Pointer to lambda function which converts arguments and performs the actual call /// Pointer to lambda function which converts arguments and performs the actual call
handle (*impl) (function_record *, function_arguments, handle) = nullptr; handle (*impl) (function_call &) = nullptr;
/// Storage for the wrapped function pointer and captured data, if any /// Storage for the wrapped function pointer and captured data, if any
void *data[3] = { }; void *data[3] = { };
@ -221,6 +222,22 @@ struct type_record {
} }
}; };
/// Internal data associated with a single function call
struct function_call {
function_call(function_record &f, handle p) : func(f), parent(p) {
args.reserve(f.nargs);
}
/// The function data:
const function_record &func;
/// Arguments passed to the function:
std::vector<handle> args;
/// The parent, if any
handle parent;
};
/** /**
* Partial template specializations to process custom attributes provided to * Partial template specializations to process custom attributes provided to
* cpp_function_ and class_. These are either used to initialize the respective * cpp_function_ and class_. These are either used to initialize the respective
@ -233,8 +250,8 @@ template <typename T> struct process_attribute_default {
/// Default implementation: do nothing /// Default implementation: do nothing
static void init(const T &, function_record *) { } static void init(const T &, function_record *) { }
static void init(const T &, type_record *) { } static void init(const T &, type_record *) { }
static void precall(function_arguments) { } static void precall(function_call &) { }
static void postcall(function_arguments, handle) { } static void postcall(function_call &, handle) { }
}; };
/// Process an attribute specifying the function's name /// Process an attribute specifying the function's name
@ -362,13 +379,13 @@ struct process_attribute<arithmetic> : process_attribute_default<arithmetic> {};
*/ */
template <size_t Nurse, size_t Patient> struct process_attribute<keep_alive<Nurse, Patient>> : public process_attribute_default<keep_alive<Nurse, Patient>> { template <size_t Nurse, size_t Patient> struct process_attribute<keep_alive<Nurse, Patient>> : public process_attribute_default<keep_alive<Nurse, Patient>> {
template <size_t N = Nurse, size_t P = Patient, enable_if_t<N != 0 && P != 0, int> = 0> template <size_t N = Nurse, size_t P = Patient, enable_if_t<N != 0 && P != 0, int> = 0>
static void precall(function_arguments args) { keep_alive_impl(Nurse, Patient, args, handle()); } static void precall(function_call &call) { keep_alive_impl(Nurse, Patient, call, handle()); }
template <size_t N = Nurse, size_t P = Patient, enable_if_t<N != 0 && P != 0, int> = 0> template <size_t N = Nurse, size_t P = Patient, enable_if_t<N != 0 && P != 0, int> = 0>
static void postcall(function_arguments, handle) { } static void postcall(function_call &, handle) { }
template <size_t N = Nurse, size_t P = Patient, enable_if_t<N == 0 || P == 0, int> = 0> template <size_t N = Nurse, size_t P = Patient, enable_if_t<N == 0 || P == 0, int> = 0>
static void precall(function_arguments) { } static void precall(function_call &) { }
template <size_t N = Nurse, size_t P = Patient, enable_if_t<N == 0 || P == 0, int> = 0> template <size_t N = Nurse, size_t P = Patient, enable_if_t<N == 0 || P == 0, int> = 0>
static void postcall(function_arguments args, handle ret) { keep_alive_impl(Nurse, Patient, args, ret); } static void postcall(function_call &call, handle ret) { keep_alive_impl(Nurse, Patient, call, ret); }
}; };
/// Recursively iterate over variadic template arguments /// Recursively iterate over variadic template arguments
@ -381,12 +398,12 @@ template <typename... Args> struct process_attributes {
int unused[] = { 0, (process_attribute<typename std::decay<Args>::type>::init(args, r), 0) ... }; int unused[] = { 0, (process_attribute<typename std::decay<Args>::type>::init(args, r), 0) ... };
ignore_unused(unused); ignore_unused(unused);
} }
static void precall(function_arguments fn_args) { static void precall(function_call &call) {
int unused[] = { 0, (process_attribute<typename std::decay<Args>::type>::precall(fn_args), 0) ... }; int unused[] = { 0, (process_attribute<typename std::decay<Args>::type>::precall(call), 0) ... };
ignore_unused(unused); ignore_unused(unused);
} }
static void postcall(function_arguments fn_args, handle fn_ret) { static void postcall(function_call &call, handle fn_ret) {
int unused[] = { 0, (process_attribute<typename std::decay<Args>::type>::postcall(fn_args, fn_ret), 0) ... }; int unused[] = { 0, (process_attribute<typename std::decay<Args>::type>::postcall(call, fn_ret), 0) ... };
ignore_unused(unused); ignore_unused(unused);
} }
}; };

View File

@ -1248,12 +1248,11 @@ NAMESPACE_BEGIN(detail)
// forward declaration // forward declaration
struct function_record; struct function_record;
using function_arguments = const std::vector<handle> &;
/// Helper class which loads arguments for C++ functions called from Python /// Helper class which loads arguments for C++ functions called from Python
template <typename... Args> template <typename... Args>
class argument_loader { class argument_loader {
using indices = make_index_sequence<sizeof...(Args)>; using indices = make_index_sequence<sizeof...(Args)>;
using function_arguments = const std::vector<handle> &;
template <typename Arg> using argument_is_args = std::is_same<intrinsic_t<Arg>, args>; template <typename Arg> using argument_is_args = std::is_same<intrinsic_t<Arg>, args>;
template <typename Arg> using argument_is_kwargs = std::is_same<intrinsic_t<Arg>, kwargs>; template <typename Arg> using argument_is_kwargs = std::is_same<intrinsic_t<Arg>, kwargs>;

View File

@ -118,31 +118,31 @@ protected:
"The number of named arguments does not match the function signature"); "The number of named arguments does not match the function signature");
/* Dispatch code which converts function arguments and performs the actual function call */ /* Dispatch code which converts function arguments and performs the actual function call */
rec->impl = [](detail::function_record *rec, detail::function_arguments args, handle parent) -> handle { rec->impl = [](detail::function_call &call) -> handle {
cast_in args_converter; cast_in args_converter;
/* Try to cast the function arguments into the C++ domain */ /* Try to cast the function arguments into the C++ domain */
if (!args_converter.load_args(args)) if (!args_converter.load_args(call.args))
return PYBIND11_TRY_NEXT_OVERLOAD; return PYBIND11_TRY_NEXT_OVERLOAD;
/* Invoke call policy pre-call hook */ /* Invoke call policy pre-call hook */
detail::process_attributes<Extra...>::precall(args); detail::process_attributes<Extra...>::precall(call);
/* Get a pointer to the capture object */ /* Get a pointer to the capture object */
capture *cap = (capture *) (sizeof(capture) <= sizeof(rec->data) capture *cap = (capture *) (sizeof(capture) <= sizeof(call.func.data)
? &rec->data : rec->data[0]); ? &call.func.data : call.func.data[0]);
/* Override policy for rvalues -- always move */ /* Override policy for rvalues -- always move */
constexpr auto is_rvalue = !std::is_pointer<Return>::value constexpr auto is_rvalue = !std::is_pointer<Return>::value
&& !std::is_lvalue_reference<Return>::value; && !std::is_lvalue_reference<Return>::value;
const auto policy = is_rvalue ? return_value_policy::move : rec->policy; const auto policy = is_rvalue ? return_value_policy::move : call.func.policy;
/* Perform the function call */ /* 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>(cap->f),
policy, parent); policy, call.parent);
/* Invoke call policy post-call hook */ /* Invoke call policy post-call hook */
detail::process_attributes<Extra...>::postcall(args, result); detail::process_attributes<Extra...>::postcall(call, result);
return result; return result;
}; };
@ -381,9 +381,11 @@ protected:
/// Main dispatch logic for calls to functions bound using pybind11 /// Main dispatch logic for calls to functions bound using pybind11
static PyObject *dispatcher(PyObject *self, PyObject *args_in, PyObject *kwargs_in) { static PyObject *dispatcher(PyObject *self, PyObject *args_in, PyObject *kwargs_in) {
using namespace detail;
/* Iterator over the list of potentially admissible overloads */ /* Iterator over the list of potentially admissible overloads */
detail::function_record *overloads = (detail::function_record *) PyCapsule_GetPointer(self, nullptr), function_record *overloads = (function_record *) PyCapsule_GetPointer(self, nullptr),
*it = overloads; *it = overloads;
/* Need to know how many arguments + keyword arguments there are to pick the right overload */ /* Need to know how many arguments + keyword arguments there are to pick the right overload */
const size_t n_args_in = (size_t) PyTuple_GET_SIZE(args_in); const size_t n_args_in = (size_t) PyTuple_GET_SIZE(args_in);
@ -411,18 +413,18 @@ protected:
result other than PYBIND11_TRY_NEXT_OVERLOAD. result other than PYBIND11_TRY_NEXT_OVERLOAD.
*/ */
size_t pos_args = it->nargs; // Number of positional arguments that we need function_record &func = *it;
if (it->has_args) --pos_args; // (but don't count py::args size_t pos_args = func.nargs; // Number of positional arguments that we need
if (it->has_kwargs) --pos_args; // or py::kwargs) if (func.has_args) --pos_args; // (but don't count py::args
if (func.has_kwargs) --pos_args; // or py::kwargs)
if (!it->has_args && n_args_in > pos_args) if (!func.has_args && n_args_in > pos_args)
continue; // Too many arguments for this overload continue; // Too many arguments for this overload
if (n_args_in < pos_args && it->args.size() < pos_args) if (n_args_in < pos_args && func.args.size() < pos_args)
continue; // Not enough arguments given, and not enough defaults to fill in the blanks continue; // Not enough arguments given, and not enough defaults to fill in the blanks
std::vector<handle> pass_args; function_call call(func, parent);
pass_args.reserve(it->nargs);
size_t args_to_copy = std::min(pos_args, n_args_in); size_t args_to_copy = std::min(pos_args, n_args_in);
size_t args_copied = 0; size_t args_copied = 0;
@ -440,7 +442,7 @@ protected:
std::string(it->args[args_copied].name) + "'"); std::string(it->args[args_copied].name) + "'");
} }
pass_args.push_back(PyTuple_GET_ITEM(args_in, args_copied)); call.args.push_back(PyTuple_GET_ITEM(args_in, args_copied));
} }
// We'll need to copy this if we steal some kwargs for defaults // We'll need to copy this if we steal some kwargs for defaults
@ -470,7 +472,7 @@ protected:
} }
if (value) if (value)
pass_args.push_back(value); call.args.push_back(value);
else else
break; break;
} }
@ -502,22 +504,26 @@ protected:
extra_args[i] = item.inc_ref().ptr(); extra_args[i] = item.inc_ref().ptr();
} }
} }
pass_args.push_back(extra_args); call.args.push_back(extra_args);
} }
// 4b. If we have a py::kwargs, pass on any remaining kwargs // 4b. If we have a py::kwargs, pass on any remaining kwargs
if (it->has_kwargs) { if (it->has_kwargs) {
if (!kwargs.ptr()) if (!kwargs.ptr())
kwargs = dict(); // If we didn't get one, send an empty one kwargs = dict(); // If we didn't get one, send an empty one
pass_args.push_back(kwargs); call.args.push_back(kwargs);
} }
// 5. Put everything in a big tuple. Not technically step 5, we've been building it // 5. Put everything in a vector. Not technically step 5, we've been building it
// in `pass_args` all along. // in `call.args` all along.
#if !defined(NDEBUG)
if (call.args.size() != call.func.nargs)
pybind11_fail("Internal error: function call dispatcher inserted wrong number of arguments!");
#endif
// 6. Call the function. // 6. Call the function.
try { try {
result = it->impl(it, pass_args, parent); result = it->impl(call);
} catch (reference_cast_error &) { } catch (reference_cast_error &) {
result = PYBIND11_TRY_NEXT_OVERLOAD; result = PYBIND11_TRY_NEXT_OVERLOAD;
} }
@ -541,7 +547,7 @@ protected:
- delegate translation to the next translator by throwing a new type of exception. */ - delegate translation to the next translator by throwing a new type of exception. */
auto last_exception = std::current_exception(); auto last_exception = std::current_exception();
auto &registered_exception_translators = pybind11::detail::get_internals().registered_exception_translators; auto &registered_exception_translators = get_internals().registered_exception_translators;
for (auto& translator : registered_exception_translators) { for (auto& translator : registered_exception_translators) {
try { try {
translator(last_exception); translator(last_exception);
@ -564,7 +570,7 @@ protected:
" arguments. The following argument types are supported:\n"; " arguments. The following argument types are supported:\n";
int ctr = 0; int ctr = 0;
for (detail::function_record *it2 = overloads; it2 != nullptr; it2 = it2->next) { for (function_record *it2 = overloads; it2 != nullptr; it2 = it2->next) {
msg += " "+ std::to_string(++ctr) + ". "; msg += " "+ std::to_string(++ctr) + ". ";
bool wrote_sig = false; bool wrote_sig = false;
@ -609,7 +615,7 @@ protected:
if (overloads->is_constructor) { if (overloads->is_constructor) {
/* When a constructor ran successfully, the corresponding /* When a constructor ran successfully, the corresponding
holder type (e.g. std::unique_ptr) must still be initialized. */ holder type (e.g. std::unique_ptr) must still be initialized. */
auto tinfo = detail::get_type_info(Py_TYPE(parent.ptr())); auto tinfo = get_type_info(Py_TYPE(parent.ptr()));
tinfo->init_holder(parent.ptr(), nullptr); tinfo->init_holder(parent.ptr(), nullptr);
} }
return result.ptr(); return result.ptr();
@ -1494,10 +1500,10 @@ inline void keep_alive_impl(handle nurse, handle patient) {
(void) wr.release(); (void) wr.release();
} }
PYBIND11_NOINLINE inline void keep_alive_impl(size_t Nurse, size_t Patient, function_arguments args, handle ret) { PYBIND11_NOINLINE inline void keep_alive_impl(size_t Nurse, size_t Patient, function_call &call, handle ret) {
keep_alive_impl( keep_alive_impl(
Nurse == 0 ? ret : Nurse <= args.size() ? args[Nurse - 1] : handle(), Nurse == 0 ? ret : Nurse <= call.args.size() ? call.args[Nurse - 1] : handle(),
Patient == 0 ? ret : Patient <= args.size() ? args[Patient - 1] : handle() Patient == 0 ? ret : Patient <= call.args.size() ? call.args[Patient - 1] : handle()
); );
} }