Skip to content

Commit

Permalink
Add support for positional args with args/kwargs
Browse files Browse the repository at this point in the history
This commit rewrites the function dispatcher code to support mixing
regular arguments with py::args/py::kwargs arguments.  It also
simplifies the argument loader noticeably as it no longer has to worry
about args/kwargs: all of that is now sorted out in the dispatcher,
which now simply appends a tuple/dict if the function takes
py::args/py::kwargs, then passes all the arguments in a vector.

When the argument loader hit a py::args or py::kwargs, it doesn't do
anything special: it just calls the appropriate type_caster just like it
does for any other argument (thus removing the previous special cases
for args/kwargs).

Switching to passing arguments in a single std::vector instead of a pair
of tuples also makes things simpler, both in the dispatch and the
argument_loader: since this argument list is strictly pybind-internal
(i.e. it never goes to Python) we have no particular reason to use a
Python tuple here.

Some (intentional) restrictions:
- you may not bind a function that has args/kwargs somewhere other than
  the end (this somewhat matches Python, and keeps the dispatch code a
  little cleaner by being able to not worry about where to inject the
  args/kwargs in the argument list).
- If you specify an argument both positionally and via a keyword
  argument, you get a TypeError alerting you to this (as you do in
  Python).
  • Loading branch information
jagerman authored and wjakob committed Jan 31, 2017
1 parent 102c94f commit 2686da8
Show file tree
Hide file tree
Showing 7 changed files with 272 additions and 97 deletions.
21 changes: 13 additions & 8 deletions docs/advanced/functions.rst
Original file line number Diff line number Diff line change
Expand Up @@ -256,16 +256,21 @@ Such functions can also be created using pybind11:
m.def("generic", &generic);
The class ``py::args`` derives from ``py::tuple`` and ``py::kwargs`` derives
from ``py::dict``. Note that the ``kwargs`` argument is invalid if no keyword
arguments were actually provided. Please refer to the other examples for
details on how to iterate over these, and on how to cast their entries into
C++ objects. A demonstration is also available in
``tests/test_kwargs_and_defaults.cpp``.
from ``py::dict``.

.. warning::
You may also use just one or the other, and may combine these with other
arguments as long as the ``py::args`` and ``py::kwargs`` arguments are the last
arguments accepted by the function.

Please refer to the other examples for details on how to iterate over these,
and on how to cast their entries into C++ objects. A demonstration is also
available in ``tests/test_kwargs_and_defaults.cpp``.

.. note::

Unlike Python, pybind11 does not allow combining normal parameters with the
``args`` / ``kwargs`` special parameters.
When combining \*args or \*\*kwargs with :ref:`keyword_args` you should
*not* include ``py::arg`` tags for the ``py::args`` and ``py::kwargs``
arguments.

Default arguments revisited
===========================
Expand Down
26 changes: 13 additions & 13 deletions include/pybind11/attr.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ struct undefined_t;
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_alias;
inline void keep_alive_impl(int Nurse, int Patient, handle args, handle ret);
inline void keep_alive_impl(int Nurse, int Patient, function_arguments args, handle ret);

/// Internal data structure which holds metadata about a keyword argument
struct argument_record {
Expand Down Expand Up @@ -100,7 +100,7 @@ struct function_record {
std::vector<argument_record> args;

/// Pointer to lambda function which converts arguments and performs the actual call
handle (*impl) (function_record *, handle, handle, handle) = nullptr;
handle (*impl) (function_record *, function_arguments, handle) = nullptr;

/// Storage for the wrapped function pointer and captured data, if any
void *data[3] = { };
Expand Down Expand Up @@ -129,7 +129,7 @@ struct function_record {
/// True if this is a method
bool is_method : 1;

/// Number of arguments
/// Number of arguments (including py::args and/or py::kwargs, if present)
uint16_t nargs;

/// Python method object
Expand Down Expand Up @@ -233,8 +233,8 @@ template <typename T> struct process_attribute_default {
/// Default implementation: do nothing
static void init(const T &, function_record *) { }
static void init(const T &, type_record *) { }
static void precall(handle) { }
static void postcall(handle, handle) { }
static void precall(function_arguments) { }
static void postcall(function_arguments, handle) { }
};

/// Process an attribute specifying the function's name
Expand Down Expand Up @@ -362,13 +362,13 @@ struct process_attribute<arithmetic> : process_attribute_default<arithmetic> {};
*/
template <int Nurse, int Patient> struct process_attribute<keep_alive<Nurse, Patient>> : public process_attribute_default<keep_alive<Nurse, Patient>> {
template <int N = Nurse, int P = Patient, enable_if_t<N != 0 && P != 0, int> = 0>
static void precall(handle args) { keep_alive_impl(Nurse, Patient, args, handle()); }
static void precall(function_arguments args) { keep_alive_impl(Nurse, Patient, args, handle()); }
template <int N = Nurse, int P = Patient, enable_if_t<N != 0 && P != 0, int> = 0>
static void postcall(handle, handle) { }
static void postcall(function_arguments, handle) { }
template <int N = Nurse, int P = Patient, enable_if_t<N == 0 || P == 0, int> = 0>
static void precall(handle) { }
static void precall(function_arguments) { }
template <int N = Nurse, int P = Patient, enable_if_t<N == 0 || P == 0, int> = 0>
static void postcall(handle args, handle ret) { keep_alive_impl(Nurse, Patient, args, ret); }
static void postcall(function_arguments args, handle ret) { keep_alive_impl(Nurse, Patient, args, ret); }
};

/// Recursively iterate over variadic template arguments
Expand All @@ -381,11 +381,11 @@ template <typename... Args> struct process_attributes {
int unused[] = { 0, (process_attribute<typename std::decay<Args>::type>::init(args, r), 0) ... };
ignore_unused(unused);
}
static void precall(handle fn_args) {
static void precall(function_arguments fn_args) {
int unused[] = { 0, (process_attribute<typename std::decay<Args>::type>::precall(fn_args), 0) ... };
ignore_unused(unused);
}
static void postcall(handle fn_args, handle fn_ret) {
static void postcall(function_arguments fn_args, handle fn_ret) {
int unused[] = { 0, (process_attribute<typename std::decay<Args>::type>::postcall(fn_args, fn_ret), 0) ... };
ignore_unused(unused);
}
Expand All @@ -395,8 +395,8 @@ template <typename... Args> struct process_attributes {
template <typename... Extra,
size_t named = constexpr_sum(std::is_base_of<arg, Extra>::value...),
size_t self = constexpr_sum(std::is_same<is_method, Extra>::value...)>
constexpr bool expected_num_args(size_t nargs) {
return named == 0 || (self + named) == nargs;
constexpr bool expected_num_args(size_t nargs, bool has_args, bool has_kwargs) {
return named == 0 || (self + named + has_args + has_kwargs) == nargs;
}

NAMESPACE_END(detail)
Expand Down
54 changes: 30 additions & 24 deletions include/pybind11/cast.h
Original file line number Diff line number Diff line change
Expand Up @@ -1245,22 +1245,42 @@ constexpr arg operator"" _a(const char *name, size_t) { return arg(name); }

NAMESPACE_BEGIN(detail)

// forward declaration
struct function_record;

// Helper struct to only allow py::args and/or py::kwargs at the end of the function arguments
template <bool args, bool kwargs, bool args_kwargs_are_last> struct assert_args_kwargs_must_be_last {
static constexpr bool has_args = args, has_kwargs = kwargs;
static_assert(args_kwargs_are_last, "py::args/py::kwargs are only permitted as the last argument(s) of a function");
};
template <typename... T> struct args_kwargs_must_be_last;
template <typename T1, typename... Tmore> struct args_kwargs_must_be_last<T1, Tmore...>
: args_kwargs_must_be_last<Tmore...> {};
template <typename... T> struct args_kwargs_must_be_last<args, T...>
: assert_args_kwargs_must_be_last<true, false, sizeof...(T) == 0> {};
template <typename... T> struct args_kwargs_must_be_last<kwargs, T...>
: assert_args_kwargs_must_be_last<false, true, sizeof...(T) == 0> {};
template <typename... T> struct args_kwargs_must_be_last<args, kwargs, T...>
: assert_args_kwargs_must_be_last<true, true, sizeof...(T) == 0> {};
template <> struct args_kwargs_must_be_last<> : assert_args_kwargs_must_be_last<false, false, true> {};

using function_arguments = const std::vector<handle> &;

/// Helper class which loads arguments for C++ functions called from Python
template <typename... Args>
class argument_loader {
using itypes = type_list<intrinsic_t<Args>...>;
using indices = make_index_sequence<sizeof...(Args)>;

public:
argument_loader() : value() {} // Helps gcc-7 properly initialize value
using check_args_kwargs = args_kwargs_must_be_last<intrinsic_t<Args>...>;

static constexpr auto has_kwargs = std::is_same<itypes, type_list<args, kwargs>>::value;
static constexpr auto has_args = has_kwargs || std::is_same<itypes, type_list<args>>::value;
public:
static constexpr bool has_kwargs = check_args_kwargs::has_kwargs;
static constexpr bool has_args = check_args_kwargs::has_args;

static PYBIND11_DESCR arg_names() { return detail::concat(make_caster<Args>::name()...); }

bool load_args(handle args, handle kwargs) {
return load_impl(args, kwargs, itypes{});
bool load_args(function_arguments args) {
return load_impl_sequence(args, indices{});
}

template <typename Return, typename Func>
Expand All @@ -1275,26 +1295,12 @@ class argument_loader {
}

private:
bool load_impl(handle args_, handle, type_list<args>) {
std::get<0>(value).load(args_, true);
return true;
}

bool load_impl(handle args_, handle kwargs_, type_list<args, kwargs>) {
std::get<0>(value).load(args_, true);
std::get<1>(value).load(kwargs_, true);
return true;
}

bool load_impl(handle args, handle, ... /* anything else */) {
return load_impl_sequence(args, indices{});
}

static bool load_impl_sequence(handle, index_sequence<>) { return true; }
static bool load_impl_sequence(function_arguments, index_sequence<>) { return true; }

template <size_t... Is>
bool load_impl_sequence(handle src, index_sequence<Is...>) {
for (bool r : {std::get<Is>(value).load(PyTuple_GET_ITEM(src.ptr(), Is), true)...})
bool load_impl_sequence(function_arguments args, index_sequence<Is...>) {
for (bool r : {std::get<Is>(value).load(args[Is], true)...})
if (!r)
return false;
return true;
Expand Down
Loading

0 comments on commit 2686da8

Please sign in to comment.