From 63dcf441a779f462bbe3065d1a7b03bd3d3530cf Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Sat, 2 Apr 2022 11:50:48 -0400 Subject: [PATCH 01/11] Make slice constructor consistent --- include/pybind11/pytypes.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 18cd715805..d26f6a4727 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -1519,8 +1519,8 @@ class weakref : public object { class slice : public object { public: PYBIND11_OBJECT_DEFAULT(slice, object, PySlice_Check) - slice(handle start, handle stop, handle step) { - m_ptr = PySlice_New(start.ptr(), stop.ptr(), step.ptr()); + slice(handle start, handle stop, handle step) + : object(PySlice_New(start.ptr(), stop.ptr(), step.ptr()), stolen_t{}) { if (!m_ptr) { pybind11_fail("Could not allocate slice object!"); } From e9eb99544ea2a2343579a6e6afc2a6571c478e65 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Sat, 2 Apr 2022 12:16:21 -0400 Subject: [PATCH 02/11] Add more missing std::move for ref steals --- include/pybind11/cast.h | 2 +- include/pybind11/numpy.h | 6 +++--- include/pybind11/stl.h | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index d45b49c52e..4cc94aa488 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1491,7 +1491,7 @@ class unpacking_collector { type_id()); #endif } - args_list.append(o); + args_list.append(std::move(o)); } void process(list &args_list, detail::args_proxy ap) { diff --git a/include/pybind11/numpy.h b/include/pybind11/numpy.h index 7624c9fbf4..d45fe42803 100644 --- a/include/pybind11/numpy.h +++ b/include/pybind11/numpy.h @@ -640,9 +640,9 @@ class dtype : public object { list names, formats, offsets; for (auto &descr : field_descriptors) { - names.append(descr.name); - formats.append(descr.format); - offsets.append(descr.offset); + names.append(std::move(descr.name)); + formats.append(std::move(descr.format)); + offsets.append(std::move(descr.offset)); } return dtype(std::move(names), std::move(formats), std::move(offsets), itemsize); } diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index 3d1ca7ac23..51b57a92ba 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -79,7 +79,7 @@ struct set_caster { for (auto &&value : src) { auto value_ = reinterpret_steal( key_conv::cast(forward_like(value), policy, parent)); - if (!value_ || !s.add(value_)) { + if (!value_ || !s.add(std::move(value_))) { return handle(); } } From 784e96c5ad8e48175367045d55da3227d6051bd5 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Sat, 2 Apr 2022 14:08:03 -0400 Subject: [PATCH 03/11] Add missing perfect forwarding for arg_v ctor --- include/pybind11/cast.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 4cc94aa488..07a56e71f7 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1243,8 +1243,8 @@ struct arg_v : arg { private: template arg_v(arg &&base, T &&x, const char *descr = nullptr) - : arg(base), value(reinterpret_steal( - detail::make_caster::cast(x, return_value_policy::automatic, {}))), + : arg(base), value(reinterpret_steal(detail::make_caster::cast( + std::forward(x), return_value_policy::automatic, {}))), descr(descr) #if !defined(NDEBUG) , From 126fc7c524ea7a51b54720defd75de3470d69557 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Sat, 2 Apr 2022 14:14:53 -0400 Subject: [PATCH 04/11] Add missing move in arg_v constructor --- include/pybind11/cast.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 07a56e71f7..92e9be640a 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1243,8 +1243,8 @@ struct arg_v : arg { private: template arg_v(arg &&base, T &&x, const char *descr = nullptr) - : arg(base), value(reinterpret_steal(detail::make_caster::cast( - std::forward(x), return_value_policy::automatic, {}))), + : arg(std::move(base)), value(reinterpret_steal(detail::make_caster::cast( + std::forward(x), return_value_policy::automatic, {}))), descr(descr) #if !defined(NDEBUG) , From 55d7fe59ddd9ac9e6299b5352adb845082e2092e Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Sat, 2 Apr 2022 14:27:20 -0400 Subject: [PATCH 05/11] Revert "Add missing move in arg_v constructor" This reverts commit 126fc7c524ea7a51b54720defd75de3470d69557. --- include/pybind11/cast.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 92e9be640a..07a56e71f7 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1243,8 +1243,8 @@ struct arg_v : arg { private: template arg_v(arg &&base, T &&x, const char *descr = nullptr) - : arg(std::move(base)), value(reinterpret_steal(detail::make_caster::cast( - std::forward(x), return_value_policy::automatic, {}))), + : arg(base), value(reinterpret_steal(detail::make_caster::cast( + std::forward(x), return_value_policy::automatic, {}))), descr(descr) #if !defined(NDEBUG) , From dd1244f953550a6deb348f0d2ce62c406ae9ec12 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Sat, 2 Apr 2022 14:31:36 -0400 Subject: [PATCH 06/11] Add another missing move in cast.h --- include/pybind11/cast.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 07a56e71f7..da55698023 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1143,7 +1143,7 @@ using override_caster_t = conditional_t enable_if_t::value, T> cast_ref(object &&o, make_caster &caster) { - return cast_op(load_type(caster, o)); + return cast_op(load_type(caster, std::move(o))); } template enable_if_t::value, T> cast_ref(object &&, From af05a8da5a5efb420630b4ca5a938ace1dcf4827 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Sun, 3 Apr 2022 13:49:20 -0400 Subject: [PATCH 07/11] Optimize object move ctor --- include/pybind11/pytypes.h | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index d26f6a4727..b726eec643 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -268,10 +268,7 @@ class object : public handle { /// Copy constructor; always increases the reference count object(const object &o) : handle(o) { inc_ref(); } /// Move constructor; steals the object from ``other`` and preserves its reference count - object(object &&other) noexcept { - m_ptr = other.m_ptr; - other.m_ptr = nullptr; - } + object(object &&other) noexcept : handle(other.m_ptr) { other.m_ptr = nullptr; } /// Destructor; automatically calls `handle::dec_ref()` ~object() { dec_ref(); } From 74845d5cbfaa849e8e97f52f34cd937419fc7998 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Mon, 4 Apr 2022 12:55:43 -0400 Subject: [PATCH 08/11] Don't do useless move --- include/pybind11/cast.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index da55698023..07a56e71f7 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1143,7 +1143,7 @@ using override_caster_t = conditional_t enable_if_t::value, T> cast_ref(object &&o, make_caster &caster) { - return cast_op(load_type(caster, std::move(o))); + return cast_op(load_type(caster, o)); } template enable_if_t::value, T> cast_ref(object &&, From d12c01f9185e003cf15c96eb8e4eaaddfab9f635 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Tue, 5 Apr 2022 12:52:43 -0400 Subject: [PATCH 09/11] Make move ctor same as nb --- include/pybind11/pytypes.h | 56 ++++++++++++++++++++++---------------- 1 file changed, 32 insertions(+), 24 deletions(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index b726eec643..e4f2ea4bc6 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -268,7 +268,7 @@ class object : public handle { /// Copy constructor; always increases the reference count object(const object &o) : handle(o) { inc_ref(); } /// Move constructor; steals the object from ``other`` and preserves its reference count - object(object &&other) noexcept : handle(other.m_ptr) { other.m_ptr = nullptr; } + object(object &&other) noexcept : handle(other) { other.m_ptr = nullptr; } /// Destructor; automatically calls `handle::dec_ref()` ~object() { dec_ref(); } @@ -1579,35 +1579,43 @@ class capsule : public object { } } - capsule(const void *value, void (*destructor)(void *)) { - m_ptr = PyCapsule_New(const_cast(value), nullptr, [](PyObject *o) { - auto destructor = reinterpret_cast(PyCapsule_GetContext(o)); - if (destructor == nullptr) { - if (PyErr_Occurred()) { - throw error_already_set(); - } - pybind11_fail("Unable to get capsule context"); - } - void *ptr = PyCapsule_GetPointer(o, nullptr); - if (ptr == nullptr) { - throw error_already_set(); - } - destructor(ptr); - }); + capsule(const void *value, void (*destructor)(void *)) + : object(PyCapsule_New(const_cast(value), + nullptr, + [](PyObject *o) { + auto destructor = reinterpret_cast( + PyCapsule_GetContext(o)); + if (destructor == nullptr) { + if (PyErr_Occurred()) { + throw error_already_set(); + } + pybind11_fail("Unable to get capsule context"); + } + void *ptr = PyCapsule_GetPointer(o, nullptr); + if (ptr == nullptr) { + throw error_already_set(); + } + destructor(ptr); + }), + stolen_t{}) { if (!m_ptr || PyCapsule_SetContext(m_ptr, (void *) destructor) != 0) { throw error_already_set(); } } - explicit capsule(void (*destructor)()) { - m_ptr = PyCapsule_New(reinterpret_cast(destructor), nullptr, [](PyObject *o) { - auto destructor = reinterpret_cast(PyCapsule_GetPointer(o, nullptr)); - if (destructor == nullptr) { - throw error_already_set(); - } - destructor(); - }); + explicit capsule(void (*destructor)()) + : object(PyCapsule_New(reinterpret_cast(destructor), + nullptr, + [](PyObject *o) { + auto destructor = reinterpret_cast( + PyCapsule_GetPointer(o, nullptr)); + if (destructor == nullptr) { + throw error_already_set(); + } + destructor(); + }), + stolen_t{}) { if (!m_ptr) { throw error_already_set(); From e690fb03fa97e475418c690da4114f582c27acf1 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Tue, 5 Apr 2022 12:54:17 -0400 Subject: [PATCH 10/11] Make obj move ctor same as nb --- include/pybind11/pytypes.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index b726eec643..ba0fda0a8c 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -268,7 +268,7 @@ class object : public handle { /// Copy constructor; always increases the reference count object(const object &o) : handle(o) { inc_ref(); } /// Move constructor; steals the object from ``other`` and preserves its reference count - object(object &&other) noexcept : handle(other.m_ptr) { other.m_ptr = nullptr; } + object(object &&other) noexcept : handle(other) { other.m_ptr = nullptr; } /// Destructor; automatically calls `handle::dec_ref()` ~object() { dec_ref(); } From 83d53522709ec44f48016be51509e6c357d40ab6 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Tue, 5 Apr 2022 13:24:17 -0400 Subject: [PATCH 11/11] Revert changes which break MSVC --- include/pybind11/pytypes.h | 54 ++++++++++++++++---------------------- 1 file changed, 23 insertions(+), 31 deletions(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index e4f2ea4bc6..ba0fda0a8c 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -1579,43 +1579,35 @@ class capsule : public object { } } - capsule(const void *value, void (*destructor)(void *)) - : object(PyCapsule_New(const_cast(value), - nullptr, - [](PyObject *o) { - auto destructor = reinterpret_cast( - PyCapsule_GetContext(o)); - if (destructor == nullptr) { - if (PyErr_Occurred()) { - throw error_already_set(); - } - pybind11_fail("Unable to get capsule context"); - } - void *ptr = PyCapsule_GetPointer(o, nullptr); - if (ptr == nullptr) { - throw error_already_set(); - } - destructor(ptr); - }), - stolen_t{}) { + capsule(const void *value, void (*destructor)(void *)) { + m_ptr = PyCapsule_New(const_cast(value), nullptr, [](PyObject *o) { + auto destructor = reinterpret_cast(PyCapsule_GetContext(o)); + if (destructor == nullptr) { + if (PyErr_Occurred()) { + throw error_already_set(); + } + pybind11_fail("Unable to get capsule context"); + } + void *ptr = PyCapsule_GetPointer(o, nullptr); + if (ptr == nullptr) { + throw error_already_set(); + } + destructor(ptr); + }); if (!m_ptr || PyCapsule_SetContext(m_ptr, (void *) destructor) != 0) { throw error_already_set(); } } - explicit capsule(void (*destructor)()) - : object(PyCapsule_New(reinterpret_cast(destructor), - nullptr, - [](PyObject *o) { - auto destructor = reinterpret_cast( - PyCapsule_GetPointer(o, nullptr)); - if (destructor == nullptr) { - throw error_already_set(); - } - destructor(); - }), - stolen_t{}) { + explicit capsule(void (*destructor)()) { + m_ptr = PyCapsule_New(reinterpret_cast(destructor), nullptr, [](PyObject *o) { + auto destructor = reinterpret_cast(PyCapsule_GetPointer(o, nullptr)); + if (destructor == nullptr) { + throw error_already_set(); + } + destructor(); + }); if (!m_ptr) { throw error_already_set();