-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Make int_::operator T() (for integer types) behave the same as the caster for integers #2802
base: master
Are you sure you want to change the base?
Conversation
64d38da
to
f3aaad0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a quick glance. Not a full review.
m.def("implicitly_cast_to_int32", [](py::int_ value) { return int32_t{value}; }); | ||
m.def("implicitly_cast_to_uint32", [](py::int_ value) { return uint32_t{value}; }); | ||
m.def("implicitly_cast_to_int64", [](py::int_ value) { return int64_t{value}; }); | ||
m.def("implicitly_cast_to_uint64", [](py::int_ value) { return uint64_t{value}; }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are all explicit casts. If you want implicit, use trailing return types and just return value;
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think I per se want implicit casts. They invoke the same code anyway, right?
using py_type = detail::py_int_type_for<T>; | ||
auto value = detail::as_integer<py_type>(m_ptr); | ||
if (PyErr_Occurred()) | ||
throw error_already_set(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this now throwing OverflowError
instead of RuntimeError
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is what's currently making tests fail.
Description
Currently only demonstrates #2786, but soon hopefully fixes it.
Closes #2786.
Suggested changelog entry:
Conversion errors are now correctly handled by `py::int_`'s integer conversion operator.