Skip to content

Commit

Permalink
Avoid a crash in nb::try_cast<T>(nb::none()) (#386)
Browse files Browse the repository at this point in the history
  • Loading branch information
oremanj committed Dec 15, 2023
1 parent a307eac commit 94fa30e
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 6 deletions.
7 changes: 6 additions & 1 deletion docs/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,12 @@ below inherit that of the preceding release.
Version 1.9.0 (TBA)
-------------------

* Nothing yet
* :cpp:func:`nb::try_cast() <try_cast>` no longer crashes the interpreter
when attempting to cast a Python ``None`` to a C++ type that was bound
using ``nb::class_<>``. Previously this would raise an exception from the
cast operator, which would result in a call to ``std::terminate()``
because ``try_cast()`` is declared ``noexcept``. (PR `#386
<https://github.com/wjakob/nanobind/pull/386>`__.)

Version 1.8.0 (Nov 2, 2023)
---------------------------
Expand Down
8 changes: 6 additions & 2 deletions include/nanobind/nb_cast.h
Original file line number Diff line number Diff line change
Expand Up @@ -393,8 +393,12 @@ bool try_cast(const detail::api<Derived> &value, T &out, bool convert = true) no
if (caster.from_python(value.derived().ptr(),
convert ? (uint8_t) detail::cast_flags::convert
: (uint8_t) 0, nullptr)) {
out = caster.operator detail::cast_t<T>();
return true;
try {
out = caster.operator detail::cast_t<T>();
return true;
} catch (const builtin_exception&) {
return false;
}
}

return false;
Expand Down
6 changes: 3 additions & 3 deletions tests/test_classes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -500,20 +500,20 @@ NB_MODULE(test_classes_ext, m) {
Struct s;
bool rv = nb::try_cast<Struct>(h, s);
return std::make_pair(rv, std::move(s));
});
}, nb::arg().none());

m.def("try_cast_2", [](nb::handle h) {
Struct s;
Struct &s2 = s;
bool rv = nb::try_cast<Struct &>(h, s2);
return std::make_pair(rv, std::move(s2));
});
}, nb::arg().none());

m.def("try_cast_3", [](nb::handle h) {
Struct *sp = nullptr;
bool rv = nb::try_cast<Struct *>(h, sp);
return std::make_pair(rv, sp);
}, nb::rv_policy::none);
}, nb::arg().none(), nb::rv_policy::none);

m.def("try_cast_4", [](nb::handle h) {
int i = 0;
Expand Down
18 changes: 18 additions & 0 deletions tests/test_classes.py
Original file line number Diff line number Diff line change
Expand Up @@ -684,18 +684,36 @@ def test39_try_cast(clean):
assert_stats(default_constructed=1, move_constructed=2, copy_assigned=1, destructed=3)
t.reset()

rv, s2 = t.try_cast_1(None)
assert rv is False and s2 is not s and s2.value() == 5
del s2
assert_stats(default_constructed=1, move_constructed=2, copy_assigned=0, destructed=3)
t.reset()

rv, s2 = t.try_cast_2(s)
assert rv is True and s2 is not s and s.value() == 123 and s2.value() == 123
del s2
assert_stats(default_constructed=1, move_constructed=2, copy_assigned=1, destructed=3)
t.reset()

rv, s2 = t.try_cast_2(None)
assert rv is False and s2 is not s and s2.value() == 5
del s2
assert_stats(default_constructed=1, move_constructed=2, copy_assigned=0, destructed=3)
t.reset()

rv, s2 = t.try_cast_3(s)
assert rv is True and s2 is s and s.value() == 123
del s2
assert_stats()
t.reset()

rv, s2 = t.try_cast_3(None)
assert rv is True and s2 is None
del s2
assert_stats(default_constructed=0, move_constructed=0, copy_assigned=0, destructed=0)
t.reset()

rv, s2 = t.try_cast_2(1)
assert rv is False
del s2
Expand Down

0 comments on commit 94fa30e

Please sign in to comment.