Skip to content

Add default and converting constructors for all concrete Python types #464

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

Merged
merged 4 commits into from
Nov 17, 2016
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
2 changes: 1 addition & 1 deletion docs/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ Breaking changes queued for v2.0.0 (Not yet released)
(now uses prefix increment operator); it now also accepts iterators with
different begin/end types as long as they are equality comparable.
* ``arg()`` now accepts a wider range of argument types for default values
* Added ``repr()`` method to the ``handle`` class.
* Added ``py::repr()`` function which is equivalent to Python's builtin ``repr()``.
* Added support for registering structured dtypes via ``PYBIND11_NUMPY_DTYPE()`` macro.
* Added ``PYBIND11_STR_TYPE`` macro which maps to the ``builtins.str`` type.
* Added a simplified ``buffer_info`` constructor for 1-dimensional buffers.
Expand Down
4 changes: 2 additions & 2 deletions include/pybind11/attr.h
Original file line number Diff line number Diff line change
Expand Up @@ -265,9 +265,9 @@ template <> struct process_attribute<arg_v> : process_attribute_default<arg_v> {
auto descr = "'" + std::string(a.name) + ": " + a.type + "'";
if (r->class_) {
if (r->name)
descr += " in method '" + (std::string) r->class_.str() + "." + (std::string) r->name + "'";
descr += " in method '" + (std::string) str(r->class_) + "." + (std::string) r->name + "'";
else
descr += " in method of '" + (std::string) r->class_.str() + "'";
descr += " in method of '" + (std::string) str(r->class_) + "'";
} else if (r->name) {
descr += " in function named '" + (std::string) r->name + "'";
}
Expand Down
118 changes: 70 additions & 48 deletions include/pybind11/cast.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,8 @@ PYBIND11_NOINLINE inline internals &get_internals() {
return *internals_ptr;
handle builtins(PyEval_GetBuiltins());
const char *id = PYBIND11_INTERNALS_ID;
capsule caps;
if (builtins.contains(id)) {
caps = builtins[id];
}
if (caps.check()) {
internals_ptr = caps;
if (builtins.contains(id) && isinstance<capsule>(builtins[id])) {
internals_ptr = capsule(builtins[id]);
} else {
internals_ptr = new internals();
#if defined(WITH_THREAD)
Expand Down Expand Up @@ -111,6 +107,17 @@ PYBIND11_NOINLINE inline handle get_type_handle(const std::type_info &tp, bool t
return handle(type_info ? ((PyObject *) type_info->type) : nullptr);
}

PYBIND11_NOINLINE inline bool isinstance_generic(handle obj, const std::type_info &tp) {
const auto type = detail::get_type_handle(tp, false);
if (!type)
return false;

const auto result = PyObject_IsInstance(obj.ptr(), type.ptr());
if (result == -1)
throw error_already_set();
return result != 0;
}

PYBIND11_NOINLINE inline std::string error_string() {
if (!PyErr_Occurred()) {
PyErr_SetString(PyExc_RuntimeError, "Unknown internal error occurred");
Expand All @@ -125,7 +132,7 @@ PYBIND11_NOINLINE inline std::string error_string() {
errorString += ": ";
}
if (scope.value)
errorString += (std::string) handle(scope.value).str();
errorString += (std::string) str(scope.value);

PyErr_NormalizeException(&scope.type, &scope.value, &scope.trace);

Expand Down Expand Up @@ -209,7 +216,7 @@ class type_caster_generic {
auto const &type_dict = get_internals().registered_types_py;
bool new_style_class = PyType_Check(tobj);
if (type_dict.find(tobj) == type_dict.end() && new_style_class && tobj->tp_bases) {
tuple parents(tobj->tp_bases, true);
auto parents = reinterpret_borrow<tuple>(tobj->tp_bases);
for (handle parent : parents) {
bool result = load(src, convert, (PyTypeObject *) parent.ptr());
if (result)
Expand All @@ -230,7 +237,7 @@ class type_caster_generic {
/* Perform an implicit conversion */
if (convert) {
for (auto &converter : typeinfo->implicit_conversions) {
temp = object(converter(src.ptr(), typeinfo->type), false);
temp = reinterpret_steal<object>(converter(src.ptr(), typeinfo->type));
if (load(temp, false))
return true;
}
Expand Down Expand Up @@ -277,7 +284,7 @@ class type_caster_generic {
return handle((PyObject *) it_i->second).inc_ref();
}

object inst(PyType_GenericAlloc(tinfo->type, 0), false);
auto inst = reinterpret_steal<object>(PyType_GenericAlloc(tinfo->type, 0));

auto wrapper = (instance<void> *) inst.ptr();

Expand Down Expand Up @@ -480,9 +487,9 @@ struct type_caster<T, enable_if_t<std::is_arithmetic<T>::value>> {
#endif
PyErr_Clear();
if (type_error && PyNumber_Check(src.ptr())) {
object tmp(std::is_floating_point<T>::value
? PyNumber_Float(src.ptr())
: PyNumber_Long(src.ptr()), true);
auto tmp = reinterpret_borrow<object>(std::is_floating_point<T>::value
? PyNumber_Float(src.ptr())
: PyNumber_Long(src.ptr()));
PyErr_Clear();
return load(tmp, false);
}
Expand Down Expand Up @@ -536,9 +543,8 @@ template <> class type_caster<void> : public type_caster<void_type> {
}

/* Check if this is a capsule */
capsule c(h, true);
if (c.check()) {
value = (void *) c;
if (isinstance<capsule>(h)) {
value = reinterpret_borrow<capsule>(h);
return true;
}

Expand Down Expand Up @@ -590,7 +596,7 @@ template <> class type_caster<std::string> {
if (!src) {
return false;
} else if (PyUnicode_Check(load_src.ptr())) {
temp = object(PyUnicode_AsUTF8String(load_src.ptr()), false);
temp = reinterpret_steal<object>(PyUnicode_AsUTF8String(load_src.ptr()));
if (!temp) { PyErr_Clear(); return false; } // UnicodeEncodeError
load_src = temp;
}
Expand Down Expand Up @@ -631,7 +637,7 @@ template <> class type_caster<std::wstring> {
if (!src) {
return false;
} else if (!PyUnicode_Check(load_src.ptr())) {
temp = object(PyUnicode_FromObject(load_src.ptr()), false);
temp = reinterpret_steal<object>(PyUnicode_FromObject(load_src.ptr()));
if (!temp) { PyErr_Clear(); return false; }
load_src = temp;
}
Expand All @@ -640,10 +646,10 @@ template <> class type_caster<std::wstring> {
#if PY_MAJOR_VERSION >= 3
buffer = PyUnicode_AsWideCharString(load_src.ptr(), &length);
#else
temp = object(
temp = reinterpret_steal<object>(
sizeof(wchar_t) == sizeof(short)
? PyUnicode_AsUTF16String(load_src.ptr())
: PyUnicode_AsUTF32String(load_src.ptr()), false);
: PyUnicode_AsUTF32String(load_src.ptr()));
if (temp) {
int err = PYBIND11_BYTES_AS_STRING_AND_SIZE(temp.ptr(), (char **) &buffer, &length);
if (err == -1) { buffer = nullptr; } // TypeError
Expand Down Expand Up @@ -724,8 +730,8 @@ template <typename T1, typename T2> class type_caster<std::pair<T1, T2>> {
}

static handle cast(const type &src, return_value_policy policy, handle parent) {
object o1 = object(make_caster<T1>::cast(src.first, policy, parent), false);
object o2 = object(make_caster<T2>::cast(src.second, policy, parent), false);
auto o1 = reinterpret_steal<object>(make_caster<T1>::cast(src.first, policy, parent));
auto o2 = reinterpret_steal<object>(make_caster<T2>::cast(src.second, policy, parent));
if (!o1 || !o2)
return handle();
tuple result(2);
Expand Down Expand Up @@ -840,7 +846,7 @@ template <typename... Tuple> class type_caster<std::tuple<Tuple...>> {
/* Implementation: Convert a C++ tuple into a Python tuple */
template <size_t ... Indices> static handle cast(const type &src, return_value_policy policy, handle parent, index_sequence<Indices...>) {
std::array<object, size> entries {{
object(make_caster<Tuple>::cast(std::get<Indices>(src), policy, parent), false)...
reinterpret_steal<object>(make_caster<Tuple>::cast(std::get<Indices>(src), policy, parent))...
}};
for (const auto &entry: entries)
if (!entry)
Expand Down Expand Up @@ -899,7 +905,7 @@ template <typename type, typename holder_type> class type_caster_holder : public
auto const &type_dict = get_internals().registered_types_py;
bool new_style_class = PyType_Check(tobj);
if (type_dict.find(tobj) == type_dict.end() && new_style_class && tobj->tp_bases) {
tuple parents(tobj->tp_bases, true);
auto parents = reinterpret_borrow<tuple>(tobj->tp_bases);
for (handle parent : parents) {
bool result = load(src, convert, (PyTypeObject *) parent.ptr());
if (result)
Expand All @@ -913,7 +919,7 @@ template <typename type, typename holder_type> class type_caster_holder : public

if (convert) {
for (auto &converter : typeinfo->implicit_conversions) {
temp = object(converter(src.ptr(), typeinfo->type), false);
temp = reinterpret_steal<object>(converter(src.ptr(), typeinfo->type));
if (load(temp, false))
return true;
}
Expand Down Expand Up @@ -986,20 +992,27 @@ template <> struct handle_type_name<args> { static PYBIND11_DESCR name() { retur
template <> struct handle_type_name<kwargs> { static PYBIND11_DESCR name() { return _("**kwargs"); } };

template <typename type>
struct type_caster<type, enable_if_t<is_pyobject<type>::value>> {
public:
template <typename T = type, enable_if_t<!std::is_base_of<object, T>::value, int> = 0>
bool load(handle src, bool /* convert */) { value = type(src); return value.check(); }
struct pyobject_caster {
template <typename T = type, enable_if_t<std::is_same<T, handle>::value, int> = 0>
bool load(handle src, bool /* convert */) { value = src; return static_cast<bool>(value); }

template <typename T = type, enable_if_t<std::is_base_of<object, T>::value, int> = 0>
bool load(handle src, bool /* convert */) { value = type(src, true); return value.check(); }
bool load(handle src, bool /* convert */) {
if (!isinstance<type>(src))
return false;
value = reinterpret_borrow<type>(src);
return true;
}

static handle cast(const handle &src, return_value_policy /* policy */, handle /* parent */) {
return src.inc_ref();
}
PYBIND11_TYPE_CASTER(type, handle_type_name<type>::name());
};

template <typename T>
class type_caster<T, enable_if_t<is_pyobject<T>::value>> : public pyobject_caster<T> { };

// Our conditions for enabling moving are quite restrictive:
// At compile time:
// - T needs to be a non-const, non-pointer, non-reference type
Expand Down Expand Up @@ -1043,7 +1056,7 @@ template <typename T, typename SFINAE> type_caster<T, SFINAE> &load_type(type_ca
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) handle.get_type().str() + " to C++ type '" + type_id<T>() + "''");
(std::string) str(handle.get_type()) + " to C++ type '" + type_id<T>() + "''");
#endif
}
return conv;
Expand All @@ -1057,21 +1070,28 @@ template <typename T> make_caster<T> load_type(const handle &handle) {

NAMESPACE_END(detail)

template <typename T> T cast(const handle &handle) {
// pytype -> C++ type
template <typename T, detail::enable_if_t<!detail::is_pyobject<T>::value, int> = 0>
T cast(const handle &handle) {
static_assert(!detail::cast_is_temporary_value_reference<T>::value,
"Unable to cast type to reference: value is local to type caster");
using type_caster = detail::make_caster<T>;
return detail::load_type<T>(handle).operator typename type_caster::template cast_op_type<T>();
}

template <typename T> object cast(const T &value,
return_value_policy policy = return_value_policy::automatic_reference,
handle parent = handle()) {
// pytype -> pytype (calls converting constructor)
template <typename T, detail::enable_if_t<detail::is_pyobject<T>::value, int> = 0>
T cast(const handle &handle) { return T(reinterpret_borrow<object>(handle)); }

// C++ type -> py::object
template <typename T, detail::enable_if_t<!detail::is_pyobject<T>::value, int> = 0>
object cast(const T &value, return_value_policy policy = return_value_policy::automatic_reference,
handle parent = handle()) {
if (policy == return_value_policy::automatic)
policy = std::is_pointer<T>::value ? return_value_policy::take_ownership : return_value_policy::copy;
else if (policy == return_value_policy::automatic_reference)
policy = std::is_pointer<T>::value ? return_value_policy::reference : return_value_policy::copy;
return object(detail::make_caster<T>::cast(value, policy, parent), false);
return reinterpret_steal<object>(detail::make_caster<T>::cast(value, policy, parent));
}

template <typename T> T handle::cast() const { return pybind11::cast<T>(*this); }
Expand All @@ -1084,7 +1104,7 @@ detail::enable_if_t<detail::move_always<T>::value || detail::move_if_unreference
throw cast_error("Unable to cast Python instance to C++ rvalue: instance has multiple references"
" (compile in debug mode for details)");
#else
throw cast_error("Unable to move from Python " + (std::string) obj.get_type().str() +
throw cast_error("Unable to move from Python " + (std::string) str(obj.get_type()) +
" instance to C++ " + type_id<T>() + " instance: instance has multiple references");
#endif

Expand Down Expand Up @@ -1145,8 +1165,8 @@ template <return_value_policy policy = return_value_policy::automatic_reference,
typename... Args> tuple make_tuple(Args&&... args_) {
const size_t size = sizeof...(Args);
std::array<object, size> args {
{ object(detail::make_caster<Args>::cast(
std::forward<Args>(args_), policy, nullptr), false)... }
{ reinterpret_steal<object>(detail::make_caster<Args>::cast(
std::forward<Args>(args_), policy, nullptr))... }
};
for (auto &arg_value : args) {
if (!arg_value) {
Expand Down Expand Up @@ -1178,7 +1198,9 @@ struct arg_v : arg {
template <typename T>
arg_v(const char *name, T &&x, const char *descr = nullptr)
: arg(name),
value(detail::make_caster<T>::cast(x, return_value_policy::automatic, handle()), false),
value(reinterpret_steal<object>(
detail::make_caster<T>::cast(x, return_value_policy::automatic, {})
)),
descr(descr)
#if !defined(NDEBUG)
, type(type_id<T>())
Expand Down Expand Up @@ -1239,10 +1261,10 @@ class simple_collector {

/// Call a Python function and pass the collected arguments
object call(PyObject *ptr) const {
auto result = object(PyObject_CallObject(ptr, m_args.ptr()), false);
PyObject *result = PyObject_CallObject(ptr, m_args.ptr());
if (!result)
throw error_already_set();
return result;
return reinterpret_steal<object>(result);
}

private:
Expand All @@ -1261,7 +1283,7 @@ class unpacking_collector {
int _[] = { 0, (process(args_list, std::forward<Ts>(values)), 0)... };
ignore_unused(_);

m_args = object(PyList_AsTuple(args_list.ptr()), false);
m_args = std::move(args_list);
Copy link
Member

Choose a reason for hiding this comment

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

This is an optimization, I assume?

Copy link
Member Author

Choose a reason for hiding this comment

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

The way the conversion was done in the original PYBIND11_OBJECT_CVT macro, the move constructor was more efficient, so the std::move here resulted in smaller binary size. With the reworked CVT macro, the lvalue ref constructor is equally good, so the move isn't strictly needed any more, but it doesn't hurt either (args_list's lifetime ends here anyway).

}

const tuple &args() const & { return m_args; }
Expand All @@ -1272,16 +1294,16 @@ class unpacking_collector {

/// Call a Python function and pass the collected arguments
object call(PyObject *ptr) const {
auto result = object(PyObject_Call(ptr, m_args.ptr(), m_kwargs.ptr()), false);
PyObject *result = PyObject_Call(ptr, m_args.ptr(), m_kwargs.ptr());
if (!result)
throw error_already_set();
return result;
return reinterpret_steal<object>(result);
}

private:
template <typename T>
void process(list &args_list, T &&x) {
auto o = object(detail::make_caster<T>::cast(std::forward<T>(x), policy, nullptr), false);
auto o = reinterpret_steal<object>(detail::make_caster<T>::cast(std::forward<T>(x), policy, {}));
if (!o) {
#if defined(NDEBUG)
argument_cast_error();
Expand Down Expand Up @@ -1318,12 +1340,12 @@ class unpacking_collector {
void process(list &/*args_list*/, detail::kwargs_proxy kp) {
if (!kp)
return;
for (const auto &k : dict(kp, true)) {
for (const auto &k : reinterpret_borrow<dict>(kp)) {
if (m_kwargs.contains(k.first)) {
#if defined(NDEBUG)
multiple_values_error();
#else
multiple_values_error(k.first.str());
multiple_values_error(str(k.first));
#endif
}
m_kwargs[k.first] = k.second;
Expand Down
2 changes: 0 additions & 2 deletions include/pybind11/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,6 @@
#define PYBIND11_BYTES_FROM_STRING_AND_SIZE PyBytes_FromStringAndSize
#define PYBIND11_BYTES_AS_STRING_AND_SIZE PyBytes_AsStringAndSize
#define PYBIND11_BYTES_AS_STRING PyBytes_AsString
#define PYBIND11_BYTES_CHECK PyBytes_Check
#define PYBIND11_LONG_CHECK(o) PyLong_Check(o)
Copy link
Member

Choose a reason for hiding this comment

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

I'm surprised these duplicate definitions didn't cause warnings before.

#define PYBIND11_LONG_AS_LONGLONG(o) PyLong_AsLongLong(o)
#define PYBIND11_LONG_AS_UNSIGNED_LONGLONG(o) PyLong_AsUnsignedLongLong(o)
Expand All @@ -130,7 +129,6 @@
#define PYBIND11_BYTES_FROM_STRING_AND_SIZE PyString_FromStringAndSize
#define PYBIND11_BYTES_AS_STRING_AND_SIZE PyString_AsStringAndSize
#define PYBIND11_BYTES_AS_STRING PyString_AsString
#define PYBIND11_BYTES_CHECK PyString_Check
#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))
Expand Down
8 changes: 4 additions & 4 deletions include/pybind11/eigen.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ struct type_caster<Type, enable_if_t<is_eigen_dense<Type>::value && !is_eigen_re
static constexpr bool isVector = Type::IsVectorAtCompileTime;

bool load(handle src, bool) {
array_t<Scalar> buf(src, true);
if (!buf.check())
auto buf = array_t<Scalar>::ensure(src);
if (!buf)
return false;

if (buf.ndim() == 1) {
Expand Down Expand Up @@ -182,7 +182,7 @@ struct type_caster<Type, enable_if_t<is_eigen_sparse<Type>::value>> {
if (!src)
return false;

object obj(src, true);
auto obj = reinterpret_borrow<object>(src);
object sparse_module = module::import("scipy.sparse");
object matrix_type = sparse_module.attr(
rowMajor ? "csr_matrix" : "csc_matrix");
Expand All @@ -201,7 +201,7 @@ struct type_caster<Type, enable_if_t<is_eigen_sparse<Type>::value>> {
auto shape = pybind11::tuple((pybind11::object) obj.attr("shape"));
auto nnz = obj.attr("nnz").cast<Index>();

if (!values.check() || !innerIndices.check() || !outerIndices.check())
if (!values || !innerIndices || !outerIndices)
return false;

value = Eigen::MappedSparseMatrix<Scalar, Type::Flags, StorageIndex>(
Expand Down
Loading