Skip to content
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

Add support for keyword-only arguments #448

Merged
merged 1 commit into from
Mar 6, 2024
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
24 changes: 24 additions & 0 deletions docs/api_core.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1739,6 +1739,30 @@ parameter of :cpp:func:`module_::def`, :cpp:func:`class_::def`,
This policy matches `automatic` but falls back to `reference` when the
return value is a pointer.

.. cpp:struct:: kw_only

Indicate that all following function parameters are keyword-only. This
may only be used if you supply an :cpp:struct:`arg` annotation for each
parameters, because keyword-only parameters are useless if they don't have
names. For example, if you write

.. code-block:: cpp

int some_func(int one, const char* two);

m.def("some_func", &some_func,
nb::arg("one"), nb::kw_only(), nb::arg("two"));

then in Python you can write ``some_func(42, two="hi")``, or
``some_func(one=42, two="hi")``, but not ``some_func(42, "hi")``.

Just like in Python, any parameters appearing after variadic
:cpp:class:`*args <args>` are implicitly keyword-only. You don't
need to include the :cpp:struct:`kw_only` annotation in this case,
but if you do include it, it must be in the correct position:
immediately after the :cpp:struct:`arg` annotation for the variadic
:cpp:class:`*args <args>` parameter.

.. cpp:struct:: template <typename T> for_getter

When defining a property with a getter and a setter, you can use this to
Expand Down
4 changes: 4 additions & 0 deletions docs/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,10 @@ noteworthy:
nanobind versions but was awkward to use, as it required the user to provide
a custom type formatter. This release makes the interface more convenient.

* :ref:`Keyword-only arguments <kw_only>` are now supported, and can be
indicated using the new :cpp:struct:`nb::kw_only() <kw_only>` function
annotation. (PR `#448 <https://github.com/wjakob/nanobind/pull/448>`__).

* ABI version 14.

.. rubric:: Footnote
Expand Down
69 changes: 64 additions & 5 deletions docs/functions.rst
Original file line number Diff line number Diff line change
Expand Up @@ -255,11 +255,10 @@ The class :cpp:class:`nb::args <args>` derives from :cpp:class:`nb::tuple
<dict>`.

You may also use them individually or even combine them with ordinary
arguments. Note, however, that :cpp:class:`nb::args <args>` and
:cpp:class:`nb::kwargs <kwargs>` must always be the last arguments of the
function, and in that order if both are specified. This is a restriction
compared to pybind11, which allowed more general arrangements. nanobind also
lacks the ``kw_only`` and ``pos_only`` annotations available in pybind11.
parameters. Note that :cpp:class:`nb::kwargs <kwargs>` must be the last
parameter if it is specified, and any parameters after
:cpp:class:`nb::args <args>` are implicitly :ref:`keyword-only <kw_only>`,
just like in regular Python.

.. _args_kwargs_2:

Expand Down Expand Up @@ -301,6 +300,66 @@ Here is an example use of the above extension in Python:
(1, 'positional')
{'keyword': 'value'}


.. _kw_only:

Keyword-only parameters
-----------------------

Python supports keyword-only parameters; these can't be filled positionally,
thus requiring the caller to specify their name. They can be used
to enforce more clarity at call sites if a function has
multiple paramaters that could be confused with each other, or to accept
named options alongside variadic ``*args``.

.. code-block:: python

def example(val: int, *, check: bool) -> None:
# val can be passed either way; check must be given as a keyword arg
pass

example(val=42, check=True) # good
example(check=False, val=5) # good
example(100, check=True) # good
example(200, False) # TypeError:
# example() takes 1 positional argument but 2 were given

def munge(*args: int, invert: bool = False) -> int:
return sum(args) * (-1 if invert else 1)

munge(1, 2, 3) # 6
munge(4, 5, 6, invert=True) # -15

nanobind provides a :cpp:struct:`nb::kw_only() <kw_only>` annotation
that allows you to produce bindings that behave like these
examples. It must be placed before the :cpp:struct:`nb::arg() <arg>`
annotation for the first keyword-only parameter; you can think of it
as equivalent to the bare ``*,`` in a Python function signature. For
example, the above examples could be written in C++ as:

.. code-block:: cpp

void example(int val, bool check);
int munge(nb::args args, bool invert);

m.def("example", &example,
nb::arg("val"), nb::kw_only(), nb::arg("check"));

// Parameters after *args are implicitly keyword-only:
m.def("munge", &munge,
nb::arg("args"), nb::arg("invert"));

// But you can be explicit about it too, as long as you put the
// kw_only annotation in the correct position:
m.def("munge", &munge,
nb::arg("args"), nb::kw_only(), nb::arg("invert"));

.. note:: nanobind does *not* support the ``pos_only()`` argument annotation
provided by pybind11, which marks the parameters before it as positional-only.
However, a parameter can be made effectively positional-only by giving it
no name (using an empty :cpp:struct:`nb::arg() <arg>` specifier).


.. _function_templates:

Function templates
Expand Down
6 changes: 4 additions & 2 deletions docs/porting.rst
Original file line number Diff line number Diff line change
Expand Up @@ -334,8 +334,10 @@ Removed features include:
executable or run several independent Python interpreters in the same process
is unsupported. Nanobind caters to bindings only. Multi-interpreter support
would require TLS lookups for nanobind data structures, which is undesirable.
- ○ **Function binding annotations**: the ``kw_only`` / ``pos_only`` argument
annotations were removed.
- ○ **Function binding annotations**: The ``pos_only`` argument
annotation was removed. However, the same behavior can be achieved by
creating unnamed arguments; see the discussion in the section on
:ref:`keyword-only arguments <kw_only>`.
- ○ **Metaclasses**: creating types with custom metaclasses is unsupported.
- ○ **Module-local bindings**: support was removed (both for types and exceptions).
- ○ **Custom allocation**: C++ classes with an overloaded or deleted ``operator
Expand Down
20 changes: 18 additions & 2 deletions include/nanobind/nb_attr.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ struct is_operator {};
struct is_arithmetic {};
struct is_final {};
struct is_generic {};
struct kw_only {};

template <size_t /* Nurse */, size_t /* Patient */> struct keep_alive {};
template <typename T> struct supplement {};
Expand Down Expand Up @@ -156,8 +157,20 @@ template <size_t Size> struct func_data_prelim {
/// Supplementary flags
uint32_t flags;

/// Total number of function call arguments
uint32_t nargs;
/// Total number of parameters accepted by the C++ function; nb::args
/// and nb::kwargs parameters are counted as one each. If the
/// 'has_args' flag is set, then there is one arg_data structure
/// for each of these.
uint16_t nargs;

/// Number of paramters to the C++ function that may be filled from
/// Python positional arguments without additional ceremony. nb::args and
/// nb::kwargs parameters are not counted in this total, nor are any
/// parameters after nb::args or after a nb::kw_only annotation.
/// The parameters counted here may be either named (nb::arg("name"))
/// or unnamed (nb::arg()). If unnamed, they are effectively positional-only.
oremanj marked this conversation as resolved.
Show resolved Hide resolved
/// nargs_pos is always <= nargs.
uint16_t nargs_pos;

// ------- Extra fields -------

Expand Down Expand Up @@ -270,6 +283,9 @@ NB_INLINE void func_extra_apply(F &f, const arg_v &a, size_t &index) {
arg.none = a.none_;
}

template <typename F>
NB_INLINE void func_extra_apply(F &, kw_only, size_t &) {}

template <typename F, typename... Ts>
NB_INLINE void func_extra_apply(F &, call_guard<Ts...>, size_t &) {}

Expand Down
80 changes: 74 additions & 6 deletions include/nanobind/nb_func.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,15 @@ bool from_python_keep_alive(Caster &c, PyObject **args, uint8_t *args_flags,
return true;
}

// Return the number of nb::arg and nb::arg_v types in the first I types Ts.
// Invoke with std::make_index_sequence<sizeof...(Ts)>() to provide
// an index pack 'Is' that parallels the types pack Ts.
template <size_t I, typename... Ts, size_t... Is>
oremanj marked this conversation as resolved.
Show resolved Hide resolved
constexpr size_t count_args_before_index(std::index_sequence<Is...>) {
static_assert(sizeof...(Is) == sizeof...(Ts));
return ((Is < I && (std::is_same_v<arg, Ts> || std::is_same_v<arg_v, Ts>)) + ... + 0);
}

template <bool ReturnRef, bool CheckGuard, typename Func, typename Return,
typename... Args, size_t... Is, typename... Extra>
NB_INLINE PyObject *func_create(Func &&func, Return (*)(Args...),
Expand All @@ -45,7 +54,9 @@ NB_INLINE PyObject *func_create(Func &&func, Return (*)(Args...),

(void) is;

// Detect locations of nb::args / nb::kwargs (if exists)
// Detect locations of nb::args / nb::kwargs (if they exist).
// Find the first and last occurrence of each; we'll later make sure these
// match, in order to guarantee there's only one instance.
static constexpr size_t
args_pos_1 = index_1_v<std::is_same_v<intrinsic_t<Args>, args>...>,
args_pos_n = index_n_v<std::is_same_v<intrinsic_t<Args>, args>...>,
Expand All @@ -60,16 +71,59 @@ NB_INLINE PyObject *func_create(Func &&func, Return (*)(Args...),
(std::is_same_v<is_method, Extra> + ... + 0) != 0;
constexpr bool is_getter_det =
(std::is_same_v<is_getter, Extra> + ... + 0) != 0;
constexpr bool has_arg_annotations = nargs_provided > 0 && !is_getter_det;

// Detect location of nb::kw_only annotation, if supplied. As with args/kwargs
// we find the first and last location and later verify they match each other.
// Note this is an index in Extra... while args/kwargs_pos_* are indices in
// Args... .
constexpr size_t
kwonly_pos_1 = index_1_v<std::is_same_v<kw_only, Extra>...>,
wjakob marked this conversation as resolved.
Show resolved Hide resolved
kwonly_pos_n = index_n_v<std::is_same_v<kw_only, Extra>...>;
// Arguments after nb::args are implicitly keyword-only even if there is no
// nb::kw_only annotation
constexpr bool explicit_kw_only = kwonly_pos_1 != sizeof...(Extra);
constexpr bool implicit_kw_only = args_pos_1 + 1 < kwargs_pos_1;

// A few compile-time consistency checks
static_assert(args_pos_1 == args_pos_n && kwargs_pos_1 == kwargs_pos_n,
"Repeated use of nb::kwargs or nb::args in the function signature!");
static_assert(nargs_provided == 0 || nargs_provided + is_method_det == nargs || is_getter_det,
static_assert(!has_arg_annotations || nargs_provided + is_method_det == nargs,
"The number of nb::arg annotations must match the argument count!");
static_assert(kwargs_pos_1 == nargs || kwargs_pos_1 + 1 == nargs,
"nb::kwargs must be the last element of the function signature!");
static_assert(args_pos_1 == nargs || args_pos_1 + 1 == kwargs_pos_1,
"nb::args must follow positional arguments and precede nb::kwargs!");
static_assert(args_pos_1 == nargs || args_pos_1 < kwargs_pos_1,
"nb::args must precede nb::kwargs if both are present!");
static_assert(has_arg_annotations || (!implicit_kw_only && !explicit_kw_only),
"Keyword-only arguments must have names!");

// Find the index in Args... of the first keyword-only parameter. Since
// the 'self' parameter doesn't get a nb::arg annotation, we must adjust
// by 1 for methods. Note that nargs_before_kw_only is only used if
// a kw_only annotation exists (i.e., if explicit_kw_only is true);
// the conditional is just to save the compiler some effort otherwise.
constexpr size_t nargs_before_kw_only =
Copy link
Owner

Choose a reason for hiding this comment

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

stupid question: what's the difference between keeping this code here as constexpr size_t or just making a size_t variable and then potentially overwriting it in the constexpr block? If efficiency (avoiding the template expansion) is the reason, then it would seem to me that the compiler must in any case be smart enough to not expand templates in a if constexpr (false) block to avoid potential exponential instantiation of templates in recursive constexpr programs. Anyways, this way of writing things down seems somehow more natural (closer to the behavior of a regular non-constexpr program).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need the nargs_before_kw_only value available as a constexpr in order to static_assert about it in the explicit_kw_only block, and we also need it down at the bottom of the function to initialize nargs_pos. The latter doesn't need to be constexpr, so I guess we could do something like

size_t nargs_before_kw_only = nargs;
if constexpr (explicit_kw_only) {
    constexpr size_t cx_nargs_before_kw_only = is_method_det + count_args_before_index<...>(...);
    // static_assert about cx_nargs_before_kw_only
    nargs_before_kw_only = cx_nargs_before_kw_only;
}

but that seemed like way too much trouble to go to when there's not any reason to believe the instantiation of count_args_before_index is particularly expensive. I was only trying to avoid it on principle, but the cure seemed worse than the disease in the end.

Copy link
Owner

Choose a reason for hiding this comment

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

Got it. Let's leave it like this then.

explicit_kw_only
? is_method_det + count_args_before_index<kwonly_pos_1, Extra...>(
std::make_index_sequence<sizeof...(Extra)>())
: nargs;

if constexpr (explicit_kw_only) {
static_assert(kwonly_pos_1 == kwonly_pos_n,
"Repeated use of nb::kw_only annotation!");

// If both kw_only and *args are specified, kw_only must be
// immediately after the nb::arg for *args.
static_assert(args_pos_1 == nargs || nargs_before_kw_only == args_pos_1 + 1,
"Arguments after nb::args are implicitly keyword-only; any "
"nb::kw_only() annotation must be positioned to reflect that!");

// If both kw_only and **kwargs are specified, kw_only must be
// before the nb::arg for **kwargs.
static_assert(nargs_before_kw_only < kwargs_pos_1,
"Variadic nb::kwargs are implicitly keyword-only; any "
"nb::kw_only() annotation must be positioned to reflect that!");
}

// Collect function signature information for the docstring
using cast_out = make_caster<
Expand All @@ -96,8 +150,7 @@ NB_INLINE PyObject *func_create(Func &&func, Return (*)(Args...),
f.flags = (args_pos_1 < nargs ? (uint32_t) func_flags::has_var_args : 0) |
(kwargs_pos_1 < nargs ? (uint32_t) func_flags::has_var_kwargs : 0) |
(ReturnRef ? (uint32_t) func_flags::return_ref : 0) |
(nargs_provided &&
!is_getter_det ? (uint32_t) func_flags::has_args : 0);
(has_arg_annotations ? (uint32_t) func_flags::has_args : 0);

/* Store captured function inside 'func_data_prelim' if there is space. Issues
with aliasing are resolved via separate compilation of libnanobind. */
Expand Down Expand Up @@ -166,6 +219,21 @@ NB_INLINE PyObject *func_create(Func &&func, Return (*)(Args...),
f.descr_types = descr_types;
f.nargs = nargs;

// Set nargs_pos to the number of C++ function parameters (Args...) that
// can be filled from Python positional arguments in a one-to-one fashion.
// This ends at:
// - the location of the variadic *args parameter, if present; otherwise
// - the location of the first keyword-only parameter, if any; otherwise
// - the location of the variadic **kwargs parameter, if present; otherwise
// - the end of the parameter list
// It's correct to give *args priority over kw_only because we verified
// above that kw_only comes afterward if both are present. It's correct
// to give kw_only priority over **kwargs because we verified above that
// kw_only comes before if both are present.
f.nargs_pos = args_pos_1 < nargs ? args_pos_1 :
oremanj marked this conversation as resolved.
Show resolved Hide resolved
explicit_kw_only ? nargs_before_kw_only :
kwargs_pos_1 < nargs ? kwargs_pos_1 : nargs;

// Fill remaining fields of 'f'
size_t arg_index = 0;
(void) arg_index;
Expand Down
Loading