diff --git a/docs/advanced/functions.rst b/docs/advanced/functions.rst index abd1084ab..ea9f35223 100644 --- a/docs/advanced/functions.rst +++ b/docs/advanced/functions.rst @@ -306,8 +306,9 @@ The class ``py::args`` derives from ``py::tuple`` and ``py::kwargs`` derives from ``py::dict``. 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. +arguments. Note, however, that ``py::kwargs`` must always be the last argument +of the function, and ``py::args`` implies that any further arguments are +keyword-only (see :ref:`keyword_only_arguments`). 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 @@ -366,6 +367,8 @@ like so: py::class_("MyClass") .def("myFunction", py::arg("arg") = static_cast(nullptr)); +.. _keyword_only_arguments: + Keyword-only arguments ====================== @@ -397,6 +400,15 @@ feature does *not* require Python 3 to work. .. versionadded:: 2.6 +As of pybind11 2.9, a ``py::args`` argument implies that any following arguments +are keyword-only, as if ``py::kw_only()`` had been specified in the same +relative location of the argument list as the ``py::args`` argument. The +``py::kw_only()`` may be included to be explicit about this, but is not +required. (Prior to 2.9 ``py::args`` may only occur at the end of the argument +list, or immediately before a ``py::kwargs`` argument at the end). + +.. versionadded:: 2.9 + Positional-only arguments ========================= diff --git a/include/pybind11/attr.h b/include/pybind11/attr.h index 0dedbc08d..b95c84904 100644 --- a/include/pybind11/attr.h +++ b/include/pybind11/attr.h @@ -174,7 +174,7 @@ struct function_record { function_record() : is_constructor(false), is_new_style_constructor(false), is_stateless(false), is_operator(false), is_method(false), has_args(false), - has_kwargs(false), has_kw_only_args(false), prepend(false) { } + has_kwargs(false), prepend(false) { } /// Function name char *name = nullptr; /* why no C++ strings? They generate heavier code.. */ @@ -221,17 +221,15 @@ struct function_record { /// True if the function has a '**kwargs' argument bool has_kwargs : 1; - /// True once a 'py::kw_only' is encountered (any following args are keyword-only) - bool has_kw_only_args : 1; - /// True if this function is to be inserted at the beginning of the overload resolution chain bool prepend : 1; /// Number of arguments (including py::args and/or py::kwargs, if present) std::uint16_t nargs; - /// Number of trailing arguments (counted in `nargs`) that are keyword-only - std::uint16_t nargs_kw_only = 0; + /// Number of leading positional arguments, which are terminated by a py::args or py::kwargs + /// argument or by a py::kw_only annotation. + std::uint16_t nargs_pos = 0; /// Number of leading arguments (counted in `nargs`) that are positional-only std::uint16_t nargs_pos_only = 0; @@ -411,10 +409,9 @@ template <> struct process_attribute : process_attribu static void init(const is_new_style_constructor &, function_record *r) { r->is_new_style_constructor = true; } }; -inline void process_kw_only_arg(const arg &a, function_record *r) { - if (!a.name || a.name[0] == '\0') - pybind11_fail("arg(): cannot specify an unnamed argument after an kw_only() annotation"); - ++r->nargs_kw_only; +inline void check_kw_only_arg(const arg &a, function_record *r) { + if (r->args.size() > r->nargs_pos && (!a.name || a.name[0] == '\0')) + pybind11_fail("arg(): cannot specify an unnamed argument after a kw_only() annotation or args() argument"); } /// Process a keyword argument attribute (*without* a default value) @@ -424,7 +421,7 @@ template <> struct process_attribute : process_attribute_default { r->args.emplace_back("self", nullptr, handle(), true /*convert*/, false /*none not allowed*/); r->args.emplace_back(a.name, nullptr, handle(), !a.flag_noconvert, a.flag_none); - if (r->has_kw_only_args) process_kw_only_arg(a, r); + check_kw_only_arg(a, r); } }; @@ -457,14 +454,16 @@ template <> struct process_attribute : process_attribute_default { } r->args.emplace_back(a.name, a.descr, a.value.inc_ref(), !a.flag_noconvert, a.flag_none); - if (r->has_kw_only_args) process_kw_only_arg(a, r); + check_kw_only_arg(a, r); } }; /// Process a keyword-only-arguments-follow pseudo argument template <> struct process_attribute : process_attribute_default { static void init(const kw_only &, function_record *r) { - r->has_kw_only_args = true; + if (r->has_args && r->nargs_pos != static_cast(r->args.size())) + pybind11_fail("Mismatched args() and kw_only(): they must occur at the same relative argument location (or omit kw_only() entirely)"); + r->nargs_pos = static_cast(r->args.size()); } }; @@ -472,6 +471,9 @@ template <> struct process_attribute : process_attribute_default struct process_attribute : process_attribute_default { static void init(const pos_only &, function_record *r) { r->nargs_pos_only = static_cast(r->args.size()); + if (r->nargs_pos_only > r->nargs_pos) + pybind11_fail("pos_only(): cannot follow a py::args() argument"); + // It also can't follow a kw_only, but a static_assert in pybind11.h checks that } }; diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 20fbb3258..4b39827c2 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1130,6 +1130,9 @@ constexpr arg operator"" _a(const char *name, size_t) { return arg(name); } PYBIND11_NAMESPACE_BEGIN(detail) +template using is_kw_only = std::is_same, kw_only>; +template using is_pos_only = std::is_same, pos_only>; + // forward declaration (definition in attr.h) struct function_record; @@ -1165,17 +1168,18 @@ class argument_loader { template using argument_is_args = std::is_same, args>; template using argument_is_kwargs = std::is_same, kwargs>; - // Get args/kwargs argument positions relative to the end of the argument list: - static constexpr auto args_pos = constexpr_first() - (int) sizeof...(Args), - kwargs_pos = constexpr_first() - (int) sizeof...(Args); + // Get kwargs argument position, or -1 if not present: + static constexpr auto kwargs_pos = constexpr_last(); - static constexpr bool args_kwargs_are_last = kwargs_pos >= - 1 && args_pos >= kwargs_pos - 1; - - static_assert(args_kwargs_are_last, "py::args/py::kwargs are only permitted as the last argument(s) of a function"); + static_assert(kwargs_pos == -1 || kwargs_pos == (int) sizeof...(Args) - 1, "py::kwargs is only permitted as the last argument of a function"); public: - static constexpr bool has_kwargs = kwargs_pos < 0; - static constexpr bool has_args = args_pos < 0; + static constexpr bool has_kwargs = kwargs_pos != -1; + + // py::args argument position; -1 if not present. + static constexpr int args_pos = constexpr_last(); + + static_assert(args_pos == -1 || args_pos == constexpr_first(), "py::args cannot be specified more than once"); static constexpr auto arg_names = concat(type_descr(make_caster::name)...); diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index bfc1c368c..124ea37e5 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -203,7 +203,7 @@ protected: conditional_t::value, void_type, Return> >; - static_assert(expected_num_args(sizeof...(Args), cast_in::has_args, cast_in::has_kwargs), + static_assert(expected_num_args(sizeof...(Args), cast_in::args_pos >= 0, cast_in::has_kwargs), "The number of argument annotations does not match the number of function arguments"); /* Dispatch code which converts function arguments and performs the actual function call */ @@ -238,17 +238,27 @@ protected: return result; }; + rec->nargs_pos = cast_in::args_pos >= 0 + ? static_cast(cast_in::args_pos) + : sizeof...(Args) - cast_in::has_kwargs; // Will get reduced more if we have a kw_only + rec->has_args = cast_in::args_pos >= 0; + rec->has_kwargs = cast_in::has_kwargs; + /* Process any user-provided function attributes */ process_attributes::init(extra..., rec); { constexpr bool has_kw_only_args = any_of...>::value, has_pos_only_args = any_of...>::value, - has_args = any_of...>::value, has_arg_annotations = any_of...>::value; static_assert(has_arg_annotations || !has_kw_only_args, "py::kw_only requires the use of argument annotations"); static_assert(has_arg_annotations || !has_pos_only_args, "py::pos_only requires the use of argument annotations (for docstrings and aligning the annotations to the argument)"); - static_assert(!(has_args && has_kw_only_args), "py::kw_only cannot be combined with a py::args argument"); + + static_assert(constexpr_sum(is_kw_only::value...) <= 1, "py::kw_only may be specified only once"); + static_assert(constexpr_sum(is_pos_only::value...) <= 1, "py::pos_only may be specified only once"); + constexpr auto kw_only_pos = constexpr_first(); + constexpr auto pos_only_pos = constexpr_first(); + static_assert(!(has_kw_only_args && has_pos_only_args) || pos_only_pos < kw_only_pos, "py::pos_only must come before py::kw_only"); } /* Generate a readable signature describing the function's arguments and return value types */ @@ -259,9 +269,6 @@ protected: // Pass on the ownership over the `unique_rec` to `initialize_generic`. `rec` stays valid. initialize_generic(std::move(unique_rec), signature.text, types.data(), sizeof...(Args)); - if (cast_in::has_args) rec->has_args = true; - if (cast_in::has_kwargs) rec->has_kwargs = true; - /* Stash some additional information used by an important optimization in 'functional.h' */ using FunctionType = Return (*)(Args...); constexpr bool is_function_ptr = @@ -340,16 +347,18 @@ protected: /* Generate a proper function signature */ std::string signature; size_t type_index = 0, arg_index = 0; + bool is_starred = false; for (auto *pc = text; *pc != '\0'; ++pc) { const auto c = *pc; if (c == '{') { // Write arg name for everything except *args and **kwargs. - if (*(pc + 1) == '*') + is_starred = *(pc + 1) == '*'; + if (is_starred) continue; // Separator for keyword-only arguments, placed before the kw - // arguments start - if (rec->nargs_kw_only > 0 && arg_index + rec->nargs_kw_only == args) + // arguments start (unless we are already putting an *args) + if (!rec->has_args && arg_index == rec->nargs_pos) signature += "*, "; if (arg_index < rec->args.size() && rec->args[arg_index].name) { signature += rec->args[arg_index].name; @@ -361,7 +370,7 @@ protected: signature += ": "; } else if (c == '}') { // Write default value if available. - if (arg_index < rec->args.size() && rec->args[arg_index].descr) { + if (!is_starred && arg_index < rec->args.size() && rec->args[arg_index].descr) { signature += " = "; signature += rec->args[arg_index].descr; } @@ -369,7 +378,8 @@ protected: // argument, rather than before like * if (rec->nargs_pos_only > 0 && (arg_index + 1) == rec->nargs_pos_only) signature += ", /"; - arg_index++; + if (!is_starred) + arg_index++; } else if (c == '%') { const std::type_info *t = types[type_index++]; if (!t) @@ -395,7 +405,7 @@ protected: } } - if (arg_index != args || types[type_index] != nullptr) + if (arg_index != args - rec->has_args - rec->has_kwargs || types[type_index] != nullptr) pybind11_fail("Internal error while parsing type signature (2)"); #if PY_MAJOR_VERSION < 3 @@ -631,7 +641,7 @@ protected: named positional arguments weren't *also* specified via kwarg. 2. If we weren't given enough, try to make up the omitted ones by checking whether they were provided by a kwarg matching the `py::arg("name")` name. If - so, use it (and remove it from kwargs; if not, see if the function binding + so, use it (and remove it from kwargs); if not, see if the function binding provided a default that we can use. 3. Ensure that either all keyword arguments were "consumed", or that the function takes a kwargs argument to accept unconsumed kwargs. @@ -649,7 +659,7 @@ protected: size_t num_args = func.nargs; // Number of positional arguments that we need if (func.has_args) --num_args; // (but don't count py::args if (func.has_kwargs) --num_args; // or py::kwargs) - size_t pos_args = num_args - func.nargs_kw_only; + size_t pos_args = func.nargs_pos; if (!func.has_args && n_args_in > pos_args) continue; // Too many positional arguments for this overload @@ -695,6 +705,10 @@ protected: if (bad_arg) continue; // Maybe it was meant for another overload (issue #688) + // Keep track of how many position args we copied out in case we need to come back + // to copy the rest into a py::args argument. + size_t positional_args_copied = args_copied; + // We'll need to copy this if we steal some kwargs for defaults dict kwargs = reinterpret_borrow(kwargs_in); @@ -747,6 +761,10 @@ protected: } if (value) { + // If we're at the py::args index then first insert a stub for it to be replaced later + if (func.has_args && call.args.size() == func.nargs_pos) + call.args.push_back(none()); + call.args.push_back(value); call.args_convert.push_back(arg_rec.convert); } @@ -769,16 +787,19 @@ protected: // We didn't copy out any position arguments from the args_in tuple, so we // can reuse it directly without copying: extra_args = reinterpret_borrow(args_in); - } else if (args_copied >= n_args_in) { + } else if (positional_args_copied >= n_args_in) { extra_args = tuple(0); } else { - size_t args_size = n_args_in - args_copied; + size_t args_size = n_args_in - positional_args_copied; extra_args = tuple(args_size); for (size_t i = 0; i < args_size; ++i) { - extra_args[i] = PyTuple_GET_ITEM(args_in, args_copied + i); + extra_args[i] = PyTuple_GET_ITEM(args_in, positional_args_copied + i); } } - call.args.push_back(extra_args); + if (call.args.size() <= func.nargs_pos) + call.args.push_back(extra_args); + else + call.args[func.nargs_pos] = extra_args; call.args_convert.push_back(false); call.args_ref = std::move(extra_args); } diff --git a/tests/test_kwargs_and_defaults.cpp b/tests/test_kwargs_and_defaults.cpp index 63332d32e..312596dab 100644 --- a/tests/test_kwargs_and_defaults.cpp +++ b/tests/test_kwargs_and_defaults.cpp @@ -56,6 +56,23 @@ TEST_SUBMODULE(kwargs_and_defaults, m) { m.def("mixed_plus_args_kwargs_defaults", mixed_plus_both, py::arg("i") = 1, py::arg("j") = 3.14159); + m.def("args_kwonly", + [](int i, double j, const py::args &args, int z) { return py::make_tuple(i, j, args, z); }, + "i"_a, "j"_a, "z"_a); + m.def("args_kwonly_kwargs", + [](int i, double j, const py::args &args, int z, const py::kwargs &kwargs) { + return py::make_tuple(i, j, args, z, kwargs); }, + "i"_a, "j"_a, py::kw_only{}, "z"_a); + m.def("args_kwonly_kwargs_defaults", + [](int i, double j, const py::args &args, int z, const py::kwargs &kwargs) { + return py::make_tuple(i, j, args, z, kwargs); }, + "i"_a = 1, "j"_a = 3.14159, "z"_a = 42); + m.def("args_kwonly_full_monty", + [](int h, int i, double j, const py::args &args, int z, const py::kwargs &kwargs) { + return py::make_tuple(h, i, j, args, z, kwargs); }, + py::arg() = 1, py::arg() = 2, py::pos_only{}, "j"_a = 3.14159, "z"_a = 42); + + // test_args_refcount // PyPy needs a garbage collection to get the reference count values to match CPython's behaviour #ifdef PYPY_VERSION diff --git a/tests/test_kwargs_and_defaults.py b/tests/test_kwargs_and_defaults.py index ddc387eeb..68903bde1 100644 --- a/tests/test_kwargs_and_defaults.py +++ b/tests/test_kwargs_and_defaults.py @@ -141,6 +141,53 @@ def test_mixed_args_and_kwargs(msg): """ # noqa: E501 line too long ) + # Arguments after a py::args are automatically keyword-only (pybind 2.9+) + assert m.args_kwonly(2, 2.5, z=22) == (2, 2.5, (), 22) + assert m.args_kwonly(2, 2.5, "a", "b", "c", z=22) == (2, 2.5, ("a", "b", "c"), 22) + assert m.args_kwonly(z=22, i=4, j=16) == (4, 16, (), 22) + + with pytest.raises(TypeError) as excinfo: + assert m.args_kwonly(2, 2.5, 22) # missing z= keyword + assert ( + msg(excinfo.value) + == """ + args_kwonly(): incompatible function arguments. The following argument types are supported: + 1. (i: int, j: float, *args, z: int) -> tuple + + Invoked with: 2, 2.5, 22 + """ + ) + + assert m.args_kwonly_kwargs(i=1, k=4, j=10, z=-1, y=9) == ( + 1, + 10, + (), + -1, + {"k": 4, "y": 9}, + ) + assert m.args_kwonly_kwargs(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, z=11, y=12) == ( + 1, + 2, + (3, 4, 5, 6, 7, 8, 9, 10), + 11, + {"y": 12}, + ) + assert ( + m.args_kwonly_kwargs.__doc__ + == "args_kwonly_kwargs(i: int, j: float, *args, z: int, **kwargs) -> tuple\n" + ) + + assert ( + m.args_kwonly_kwargs_defaults.__doc__ + == "args_kwonly_kwargs_defaults(i: int = 1, j: float = 3.14159, *args, z: int = 42, **kwargs) -> tuple\n" # noqa: E501 line too long + ) + assert m.args_kwonly_kwargs_defaults() == (1, 3.14159, (), 42, {}) + assert m.args_kwonly_kwargs_defaults(2) == (2, 3.14159, (), 42, {}) + assert m.args_kwonly_kwargs_defaults(z=-99) == (1, 3.14159, (), -99, {}) + assert m.args_kwonly_kwargs_defaults(5, 6, 7, 8) == (5, 6, (7, 8), 42, {}) + assert m.args_kwonly_kwargs_defaults(5, 6, 7, m=8) == (5, 6, (7,), 42, {"m": 8}) + assert m.args_kwonly_kwargs_defaults(5, 6, 7, m=8, z=9) == (5, 6, (7,), 9, {"m": 8}) + def test_keyword_only_args(msg): assert m.kw_only_all(i=1, j=2) == (1, 2) @@ -178,7 +225,7 @@ def test_keyword_only_args(msg): assert ( msg(excinfo.value) == """ - arg(): cannot specify an unnamed argument after an kw_only() annotation + arg(): cannot specify an unnamed argument after a kw_only() annotation or args() argument """ ) @@ -222,6 +269,47 @@ def test_positional_only_args(msg): m.pos_only_def_mix(1, j=4) assert "incompatible function arguments" in str(excinfo.value) + # Mix it with args and kwargs: + assert ( + m.args_kwonly_full_monty.__doc__ + == "args_kwonly_full_monty(arg0: int = 1, arg1: int = 2, /, j: float = 3.14159, *args, z: int = 42, **kwargs) -> tuple\n" # noqa: E501 line too long + ) + assert m.args_kwonly_full_monty() == (1, 2, 3.14159, (), 42, {}) + assert m.args_kwonly_full_monty(8) == (8, 2, 3.14159, (), 42, {}) + assert m.args_kwonly_full_monty(8, 9) == (8, 9, 3.14159, (), 42, {}) + assert m.args_kwonly_full_monty(8, 9, 10) == (8, 9, 10.0, (), 42, {}) + assert m.args_kwonly_full_monty(3, 4, 5, 6, 7, m=8, z=9) == ( + 3, + 4, + 5.0, + ( + 6, + 7, + ), + 9, + {"m": 8}, + ) + assert m.args_kwonly_full_monty(3, 4, 5, 6, 7, m=8, z=9) == ( + 3, + 4, + 5.0, + ( + 6, + 7, + ), + 9, + {"m": 8}, + ) + assert m.args_kwonly_full_monty(5, j=7, m=8, z=9) == (5, 2, 7.0, (), 9, {"m": 8}) + assert m.args_kwonly_full_monty(i=5, j=7, m=8, z=9) == ( + 1, + 2, + 7.0, + (), + 9, + {"i": 5, "m": 8}, + ) + def test_signatures(): assert "kw_only_all(*, i: int, j: int) -> tuple\n" == m.kw_only_all.__doc__