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

Perform two passes in the variant caster #769

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
8 changes: 8 additions & 0 deletions docs/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,14 @@ case, both modules must use the same nanobind ABI version, or they will be
isolated from each other. Releases that don't explicitly mention an ABI version
below inherit that of the preceding release.

Version TBD (unreleased)
------------------------

- The ``std::variant`` type_caster now does two passes when converting from Python.
The first pass is done without implicit conversions. This fixes an issue where
``std::variant<U, T>`` might cast a Python object wrapping a ``T`` to a ``U`` if
there is an implicit conversion available from ``T`` to ``U``.

Version 2.2.0 (October 3, 2024)
-------------------------------

Expand Down
5 changes: 5 additions & 0 deletions include/nanobind/stl/variant.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ template <typename... Ts> struct type_caster<std::variant<Ts...>> {
}

bool from_python(handle src, uint8_t flags, cleanup_list *cleanup) noexcept {
if (flags & (uint8_t) cast_flags::convert) {
if ((try_variant<Ts>(src, flags & ~(uint8_t)cast_flags::convert, cleanup) || ...)){
return true;
}
}
return (try_variant<Ts>(src, flags, cleanup) || ...);
}

Expand Down
31 changes: 31 additions & 0 deletions tests/test_stl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -475,4 +475,35 @@ NB_MODULE(test_stl_ext, m) {
m.def("optional_cstr", [](std::optional<const char*> arg) {
return arg.value_or("none");
}, nb::arg().none());


// test73
struct BasicID1 {
uint64_t id;
BasicID1(uint64_t id) : id(id) {}
};

struct BasicID2 {
uint64_t id;
BasicID2(uint64_t id) : id(id) {}
};

nb::class_<BasicID1>(m, "BasicID1")
.def(nb::init<uint64_t>())
.def("__int__", [](const BasicID1& x) { return x.id; })
;

nb::class_<BasicID2>(m, "BasicID2")
.def(nb::init_implicit<uint64_t>());

using IDVariants = std::variant<std::monostate, BasicID2, BasicID1>;

struct IDHavingEvent {
IDVariants id;
IDHavingEvent() = default;
};

nb::class_<IDHavingEvent>(m, "IDHavingEvent")
.def(nb::init<>())
.def_rw("id", &IDHavingEvent::id);
}
6 changes: 6 additions & 0 deletions tests/test_stl.py
Original file line number Diff line number Diff line change
Expand Up @@ -794,3 +794,9 @@ def test71_null_input():
@skip_on_pypy # PyPy fails this test on Windows :-(
def test72_wstr():
assert t.pass_wstr('🎈') == '🎈'

def test73_variant_implicit_conversions():
event = t.IDHavingEvent()
assert event.id is None
event.id = t.BasicID1(78)
assert type(event.id) is t.BasicID1
17 changes: 17 additions & 0 deletions tests/test_stl_ext.pyi.ref
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,14 @@ import pathlib
from typing import overload


class BasicID1:
def __init__(self, arg: int, /) -> None: ...

def __int__(self) -> int: ...

class BasicID2:
def __init__(self, arg: int, /) -> None: ...

class ClassWithMovableField:
def __init__(self) -> None: ...

Expand Down Expand Up @@ -38,6 +46,15 @@ class FuncWrapper:
alive: int = ...
"""static read-only property"""

class IDHavingEvent:
def __init__(self) -> None: ...

@property
def id(self) -> None | BasicID2 | BasicID1: ...

@id.setter
def id(self, arg: BasicID2 | BasicID1, /) -> None: ...

class Movable:
@overload
def __init__(self) -> None: ...
Expand Down