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

Make int_::operator T() (for integer types) behave the same as the caster for integers #2802

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft
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
12 changes: 3 additions & 9 deletions include/pybind11/cast.h
Original file line number Diff line number Diff line change
Expand Up @@ -1014,9 +1014,7 @@ template <typename CharT> using is_std_char_type = any_of<

template <typename T>
struct type_caster<T, enable_if_t<std::is_arithmetic<T>::value && !is_std_char_type<T>::value>> {
using _py_type_0 = conditional_t<sizeof(T) <= sizeof(long), long, long long>;
using _py_type_1 = conditional_t<std::is_signed<T>::value, _py_type_0, typename std::make_unsigned<_py_type_0>::type>;
using py_type = conditional_t<std::is_floating_point<T>::value, double, _py_type_1>;
using py_type = conditional_t<std::is_floating_point<T>::value, double, py_int_type_for<T>>;
public:

bool load(handle src, bool convert) {
Expand All @@ -1042,12 +1040,8 @@ struct type_caster<T, enable_if_t<std::is_arithmetic<T>::value && !is_std_char_t
return false;
} else if (!convert && !index_check(src.ptr()) && !PYBIND11_LONG_CHECK(src.ptr())) {
return false;
} else if (std::is_unsigned<py_type>::value) {
py_value = as_unsigned<py_type>(src.ptr());
} else { // signed integer:
py_value = sizeof(T) <= sizeof(long)
? (py_type) PyLong_AsLong(src.ptr())
: (py_type) PYBIND11_LONG_AS_LONGLONG(src.ptr());
} else {
py_value = as_integer<py_type>(src.ptr());
}

// Python API reported an error
Expand Down
2 changes: 2 additions & 0 deletions include/pybind11/detail/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@
#define PYBIND11_BYTES_SIZE PyBytes_Size
#define PYBIND11_LONG_CHECK(o) PyLong_Check(o)
#define PYBIND11_LONG_AS_LONGLONG(o) PyLong_AsLongLong(o)
#define PYBIND11_LONG_AS_UNSIGNED_LONGLONG(o) PyLong_AsUnsignedLongLong(o)
#define PYBIND11_LONG_FROM_SIGNED(o) PyLong_FromSsize_t((ssize_t) o)
#define PYBIND11_LONG_FROM_UNSIGNED(o) PyLong_FromSize_t((size_t) o)
#define PYBIND11_BYTES_NAME "bytes"
Expand Down Expand Up @@ -203,6 +204,7 @@
#define PYBIND11_BYTES_SIZE PyString_Size
#define PYBIND11_LONG_CHECK(o) (PyInt_Check(o) || PyLong_Check(o))
#define PYBIND11_LONG_AS_LONGLONG(o) (PyInt_Check(o) ? (long long) PyLong_AsLong(o) : PyLong_AsLongLong(o))
#define PYBIND11_LONG_AS_UNSIGNED_LONGLONG(o) (PyInt_Check(o) ? (unsigned long long) PyLong_AsUnsignedLong(o) : PyLong_AsUnsignedLongLong(o))
#define PYBIND11_LONG_FROM_SIGNED(o) PyInt_FromSsize_t((ssize_t) o) // Returns long if needed.
#define PYBIND11_LONG_FROM_UNSIGNED(o) PyInt_FromSize_t((size_t) o) // Returns long if needed.
#define PYBIND11_BYTES_NAME "str"
Expand Down
50 changes: 36 additions & 14 deletions include/pybind11/pytypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#pragma once

#include "detail/common.h"
#include "detail/typeid.h"
#include "buffer_info.h"
#include <utility>
#include <type_traits>
Expand Down Expand Up @@ -1091,23 +1092,37 @@ class bool_ : public object {
};

PYBIND11_NAMESPACE_BEGIN(detail)
template <typename T>
using py_signed_int_type_for = conditional_t<sizeof(T) <= sizeof(long), long, long long>;

template <typename T>
using py_unsigned_int_type_for = typename std::make_unsigned<py_signed_int_type_for<T>>::type;

template <typename T>
using py_int_type_for = conditional_t<std::is_signed<T>::value, py_signed_int_type_for<T>, py_unsigned_int_type_for<T>>;

// Converts a value to the given unsigned type. If an error occurs, you get back (Unsigned) -1;
// otherwise you get back the unsigned long or unsigned long long value cast to (Unsigned).
// (The distinction is critically important when casting a returned -1 error value to some other
// unsigned type: (A)-1 != (B)-1 when A and B are unsigned types of different sizes).
template <typename Unsigned>
Unsigned as_unsigned(PyObject *o) {
if (sizeof(Unsigned) <= sizeof(unsigned long)
#if PY_VERSION_HEX < 0x03000000
|| PyInt_Check(o)
#endif
) {
unsigned long v = PyLong_AsUnsignedLong(o);
return v == (unsigned long) -1 && PyErr_Occurred() ? (Unsigned) -1 : (Unsigned) v;
using py_type = py_unsigned_int_type_for<Unsigned>;
py_type v = (sizeof(Unsigned) <= sizeof(unsigned long))
? (py_type) PyLong_AsUnsignedLong(o)
: (py_type) PYBIND11_LONG_AS_UNSIGNED_LONGLONG(o);
return v == (py_unsigned_int_type_for<Unsigned>) -1 && PyErr_Occurred() ? (Unsigned) -1 : (Unsigned) v;
}

template <typename Int>
Int as_integer(PyObject *o) {
if (std::is_unsigned<Int>::value) {
return as_unsigned<Int>(o);
}
else {
unsigned long long v = PyLong_AsUnsignedLongLong(o);
return v == (unsigned long long) -1 && PyErr_Occurred() ? (Unsigned) -1 : (Unsigned) v;
return sizeof(Int) <= sizeof(long)
? (Int) PyLong_AsLong(o)
: (Int) PYBIND11_LONG_AS_LONGLONG(o);
}
}
PYBIND11_NAMESPACE_END(detail)
Expand Down Expand Up @@ -1137,11 +1152,18 @@ class int_ : public object {
template <typename T,
detail::enable_if_t<std::is_integral<T>::value, int> = 0>
operator T() const {
return std::is_unsigned<T>::value
? detail::as_unsigned<T>(m_ptr)
: sizeof(T) <= sizeof(long)
? (T) PyLong_AsLong(m_ptr)
: (T) PYBIND11_LONG_AS_LONGLONG(m_ptr);
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();
Copy link
Collaborator

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?

Copy link
Collaborator Author

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.

if (sizeof(py_type) != sizeof(T) && value != (py_type) (T) value)
#if defined(NDEBUG)
throw cast_error("Unable to cast Python instance to C++ type (compile in debug mode for details)");
#else
throw cast_error("Unable to cast Python instance of type " +
(std::string) pybind11::str(type::handle_of(*this)) + " to C++ type '" + type_id<T>() + "'");
#endif
return (T) value;
}
};

Expand Down
7 changes: 7 additions & 0 deletions tests/test_pytypes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,7 @@ TEST_SUBMODULE(pytypes, m) {
throw std::runtime_error("Invalid type");
});

// test_implicit_casting
m.def("get_implicit_casting", []() {
py::dict d;
d["char*_i1"] = "abc";
Expand Down Expand Up @@ -307,6 +308,12 @@ TEST_SUBMODULE(pytypes, m) {
);
});

// (see also test_builtin_casters)
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}; });
Comment on lines +312 to +315
Copy link
Collaborator

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;.

Copy link
Collaborator Author

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?


// test_print
m.def("print_function", []() {
py::print("Hello, World!");
Expand Down
20 changes: 20 additions & 0 deletions tests/test_pytypes.py
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,26 @@ def test_implicit_casting():
}
assert z["l"] == [3, 6, 9, 12, 15]

assert m.implicitly_cast_to_int32(42) == 42
assert m.implicitly_cast_to_int32(2 ** 31 - 1) == 2 ** 31 - 1
with pytest.raises(RuntimeError, match="Unable to cast Python instance"):
m.implicitly_cast_to_int32(2 ** 31)

assert m.implicitly_cast_to_uint32(42) == 42
assert m.implicitly_cast_to_uint32(2 ** 32 - 1) == 2 ** 32 - 1
with pytest.raises(RuntimeError, match="Unable to cast Python instance"):
m.implicitly_cast_to_uint32(2 ** 32)

assert m.implicitly_cast_to_int64(42) == 42
assert m.implicitly_cast_to_int64(2 ** 63 - 1) == 2 ** 63 - 1
with pytest.raises(RuntimeError, match="Unable to cast Python instance"):
m.implicitly_cast_to_int64(2 ** 63)

assert m.implicitly_cast_to_uint64(42) == 42
assert m.implicitly_cast_to_uint64(2 ** 64 - 1) == 2 ** 64 - 1
with pytest.raises(RuntimeError, match="Unable to cast Python instance"):
m.implicitly_cast_to_uint64(2 ** 64)


def test_print(capture):
with capture:
Expand Down