From 79f35a935697cdb117d7d7dcb041a0e599fe535b Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Tue, 24 May 2022 12:19:57 -0400 Subject: [PATCH 01/20] Add object rvalue overload for accessors. Enables reference stealing --- include/pybind11/pytypes.h | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 256a2441b1..9637cc1201 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -87,6 +87,8 @@ class object_api : public pyobject_tag { item_accessor operator[](handle key) const; /// See above (the only difference is that they key is provided as a string literal) item_accessor operator[](const char *key) const; + /// See above (the only difference is that they key's reference is stolen) + item_accessor operator[](object &&key) const; /** \rst Return an internal functor to access the object's attributes. Casting the @@ -97,6 +99,8 @@ class object_api : public pyobject_tag { obj_attr_accessor attr(handle key) const; /// See above (the only difference is that they key is provided as a string literal) str_attr_accessor attr(const char *key) const; + /// See below (only difference is that the key's reference is stolen) + obj_attr_accessor attr(object &&key) const; /** \rst Matches * unpacking in Python, e.g. to unpack arguments out of a ``tuple`` @@ -1683,6 +1687,7 @@ class tuple : public object { bool empty() const { return size() == 0; } detail::tuple_accessor operator[](size_t index) const { return {*this, index}; } detail::item_accessor operator[](handle h) const { return object::operator[](h); } + detail::item_accessor operator[](object &&k) const { return object::operator[](k); } detail::tuple_iterator begin() const { return {*this, 0}; } detail::tuple_iterator end() const { return {*this, PyTuple_GET_SIZE(m_ptr)}; } }; @@ -1743,6 +1748,7 @@ class sequence : public object { bool empty() const { return size() == 0; } detail::sequence_accessor operator[](size_t index) const { return {*this, index}; } detail::item_accessor operator[](handle h) const { return object::operator[](h); } + detail::item_accessor operator[](object &&k) const { return object::operator[](k); } detail::sequence_iterator begin() const { return {*this, 0}; } detail::sequence_iterator end() const { return {*this, PySequence_Size(m_ptr)}; } }; @@ -1762,6 +1768,7 @@ class list : public object { bool empty() const { return size() == 0; } detail::list_accessor operator[](size_t index) const { return {*this, index}; } detail::item_accessor operator[](handle h) const { return object::operator[](h); } + detail::item_accessor operator[](object &&k) const { return object::operator[](k); } detail::list_iterator begin() const { return {*this, 0}; } detail::list_iterator end() const { return {*this, PyList_GET_SIZE(m_ptr)}; } template @@ -2057,6 +2064,10 @@ iterator object_api::end() const { return iterator::sentinel(); } template +item_accessor object_api::operator[](object &&key) const { + return {derived(), std::move(key)}; +} +template item_accessor object_api::operator[](handle key) const { return {derived(), reinterpret_borrow(key)}; } @@ -2065,6 +2076,10 @@ item_accessor object_api::operator[](const char *key) const { return {derived(), pybind11::str(key)}; } template +obj_attr_accessor object_api::attr(object &&key) const { + return {derived(), std::move(key)}; +} +template obj_attr_accessor object_api::attr(handle key) const { return {derived(), reinterpret_borrow(key)}; } From df69ada4303ed3b77ec24cb350e4b52cce94a194 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Tue, 24 May 2022 12:28:29 -0400 Subject: [PATCH 02/20] Fix comments --- 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 9637cc1201..b919f296e2 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -87,7 +87,7 @@ class object_api : public pyobject_tag { item_accessor operator[](handle key) const; /// See above (the only difference is that they key is provided as a string literal) item_accessor operator[](const char *key) const; - /// See above (the only difference is that they key's reference is stolen) + /// See above (the only difference is that the key's reference is stolen) item_accessor operator[](object &&key) const; /** \rst @@ -99,7 +99,7 @@ class object_api : public pyobject_tag { obj_attr_accessor attr(handle key) const; /// See above (the only difference is that they key is provided as a string literal) str_attr_accessor attr(const char *key) const; - /// See below (only difference is that the key's reference is stolen) + /// See above (only difference is that the key's reference is stolen) obj_attr_accessor attr(object &&key) const; /** \rst From 7df50dc70e129b30d9459558c2201ea4d86ca31f Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Tue, 24 May 2022 12:34:53 -0400 Subject: [PATCH 03/20] Fix more comment typos --- include/pybind11/pytypes.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index b919f296e2..d82188181c 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -85,7 +85,7 @@ class object_api : public pyobject_tag { or `object` subclass causes a call to ``__setitem__``. \endrst */ item_accessor operator[](handle key) const; - /// See above (the only difference is that they key is provided as a string literal) + /// See above (the only difference is that the key is provided as a string literal) item_accessor operator[](const char *key) const; /// See above (the only difference is that the key's reference is stolen) item_accessor operator[](object &&key) const; @@ -97,9 +97,9 @@ class object_api : public pyobject_tag { or `object` subclass causes a call to ``setattr``. \endrst */ obj_attr_accessor attr(handle key) const; - /// See above (the only difference is that they key is provided as a string literal) + /// See above (the only difference is that the key is provided as a string literal) str_attr_accessor attr(const char *key) const; - /// See above (only difference is that the key's reference is stolen) + /// See above (the only difference is that the key's reference is stolen) obj_attr_accessor attr(object &&key) const; /** \rst From 9a14da25cb5ee2cd50240d52d1875d39a66774b8 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Tue, 24 May 2022 12:39:52 -0400 Subject: [PATCH 04/20] Fix bug --- include/pybind11/pytypes.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index d82188181c..a21726f93b 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -1687,7 +1687,7 @@ class tuple : public object { bool empty() const { return size() == 0; } detail::tuple_accessor operator[](size_t index) const { return {*this, index}; } detail::item_accessor operator[](handle h) const { return object::operator[](h); } - detail::item_accessor operator[](object &&k) const { return object::operator[](k); } + detail::item_accessor operator[](object &&o) const { return object::operator[](std::move(o)); } detail::tuple_iterator begin() const { return {*this, 0}; } detail::tuple_iterator end() const { return {*this, PyTuple_GET_SIZE(m_ptr)}; } }; @@ -1748,7 +1748,7 @@ class sequence : public object { bool empty() const { return size() == 0; } detail::sequence_accessor operator[](size_t index) const { return {*this, index}; } detail::item_accessor operator[](handle h) const { return object::operator[](h); } - detail::item_accessor operator[](object &&k) const { return object::operator[](k); } + detail::item_accessor operator[](object &&o) const { return object::operator[](std::move(o)); } detail::sequence_iterator begin() const { return {*this, 0}; } detail::sequence_iterator end() const { return {*this, PySequence_Size(m_ptr)}; } }; @@ -1768,7 +1768,7 @@ class list : public object { bool empty() const { return size() == 0; } detail::list_accessor operator[](size_t index) const { return {*this, index}; } detail::item_accessor operator[](handle h) const { return object::operator[](h); } - detail::item_accessor operator[](object &&k) const { return object::operator[](k); } + detail::item_accessor operator[](object &&o) const { return object::operator[](std::move(o)); } detail::list_iterator begin() const { return {*this, 0}; } detail::list_iterator end() const { return {*this, PyList_GET_SIZE(m_ptr)}; } template From 653eed3a1506d646d2d563049905c6dc0d4fa736 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Wed, 25 May 2022 12:29:29 -0400 Subject: [PATCH 05/20] reorder declarations for clarity --- include/pybind11/pytypes.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index a21726f93b..7108e26ef6 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -85,10 +85,10 @@ class object_api : public pyobject_tag { or `object` subclass causes a call to ``__setitem__``. \endrst */ item_accessor operator[](handle key) const; - /// See above (the only difference is that the key is provided as a string literal) - item_accessor operator[](const char *key) const; /// See above (the only difference is that the key's reference is stolen) item_accessor operator[](object &&key) const; + /// See above (the only difference is that the key is provided as a string literal) + item_accessor operator[](const char *key) const; /** \rst Return an internal functor to access the object's attributes. Casting the @@ -97,10 +97,10 @@ class object_api : public pyobject_tag { or `object` subclass causes a call to ``setattr``. \endrst */ obj_attr_accessor attr(handle key) const; - /// See above (the only difference is that the key is provided as a string literal) - str_attr_accessor attr(const char *key) const; /// See above (the only difference is that the key's reference is stolen) obj_attr_accessor attr(object &&key) const; + /// See above (the only difference is that the key is provided as a string literal) + str_attr_accessor attr(const char *key) const; /** \rst Matches * unpacking in Python, e.g. to unpack arguments out of a ``tuple`` From 8955551f82526bac7ecd3e28dd1c0b864ef0b7ce Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Wed, 25 May 2022 13:30:04 -0400 Subject: [PATCH 06/20] fix another perf bug --- include/pybind11/pytypes.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 7108e26ef6..e58bfb4a29 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -661,7 +661,7 @@ class accessor : public object_api> { } template void operator=(T &&value) & { - get_cache() = reinterpret_borrow(object_or_cast(std::forward(value))); + get_cache() = ensure_object(object_or_cast(std::forward(value))); } template @@ -689,6 +689,9 @@ class accessor : public object_api> { } private: + object ensure_object(object &&o) { return std::move(o); } + object ensure_object(handle h) { return reinterpret_borrow(h); } + object &get_cache() const { if (!cache) { cache = Policy::get(obj, key); From 65d511aa4ad2e1d78d08d26d7da9192860684f75 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Wed, 25 May 2022 13:49:55 -0400 Subject: [PATCH 07/20] should be static --- 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 e58bfb4a29..c6111f453e 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -689,8 +689,8 @@ class accessor : public object_api> { } private: - object ensure_object(object &&o) { return std::move(o); } - object ensure_object(handle h) { return reinterpret_borrow(h); } + static object ensure_object(object &&o) { return std::move(o); } + static object ensure_object(handle h) { return reinterpret_borrow(h); } object &get_cache() const { if (!cache) { From 382515d4a74f1f320e147b8d95485f312fb03355 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Thu, 26 May 2022 13:12:28 -0400 Subject: [PATCH 08/20] future proof operator overloads --- include/pybind11/pytypes.h | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index c6111f453e..244c95fd4f 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -1689,8 +1689,10 @@ class tuple : public object { size_t size() const { return (size_t) PyTuple_Size(m_ptr); } bool empty() const { return size() == 0; } detail::tuple_accessor operator[](size_t index) const { return {*this, index}; } - detail::item_accessor operator[](handle h) const { return object::operator[](h); } - detail::item_accessor operator[](object &&o) const { return object::operator[](std::move(o)); } + template ::value, int> = 0> + detail::item_accessor operator[](T o) const { + return object::operator[](std::forward(o)); + } detail::tuple_iterator begin() const { return {*this, 0}; } detail::tuple_iterator end() const { return {*this, PyTuple_GET_SIZE(m_ptr)}; } }; @@ -1750,8 +1752,10 @@ class sequence : public object { } bool empty() const { return size() == 0; } detail::sequence_accessor operator[](size_t index) const { return {*this, index}; } - detail::item_accessor operator[](handle h) const { return object::operator[](h); } - detail::item_accessor operator[](object &&o) const { return object::operator[](std::move(o)); } + template ::value, int> = 0> + detail::item_accessor operator[](T o) const { + return object::operator[](std::forward(o)); + } detail::sequence_iterator begin() const { return {*this, 0}; } detail::sequence_iterator end() const { return {*this, PySequence_Size(m_ptr)}; } }; @@ -1770,8 +1774,10 @@ class list : public object { size_t size() const { return (size_t) PyList_Size(m_ptr); } bool empty() const { return size() == 0; } detail::list_accessor operator[](size_t index) const { return {*this, index}; } - detail::item_accessor operator[](handle h) const { return object::operator[](h); } - detail::item_accessor operator[](object &&o) const { return object::operator[](std::move(o)); } + template ::value, int> = 0> + detail::item_accessor operator[](T o) const { + return object::operator[](std::forward(o)); + } detail::list_iterator begin() const { return {*this, 0}; } detail::list_iterator end() const { return {*this, PyList_GET_SIZE(m_ptr)}; } template From 191dc2c051bb23ae5fe0b79c3c95ca0a9917b4e2 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Fri, 27 May 2022 11:52:58 -0400 Subject: [PATCH 09/20] Fix perfect forwarding --- include/pybind11/pytypes.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 244c95fd4f..ad67ab83d8 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -1690,7 +1690,7 @@ class tuple : public object { bool empty() const { return size() == 0; } detail::tuple_accessor operator[](size_t index) const { return {*this, index}; } template ::value, int> = 0> - detail::item_accessor operator[](T o) const { + detail::item_accessor operator[](T &&o) const { return object::operator[](std::forward(o)); } detail::tuple_iterator begin() const { return {*this, 0}; } @@ -1753,7 +1753,7 @@ class sequence : public object { bool empty() const { return size() == 0; } detail::sequence_accessor operator[](size_t index) const { return {*this, index}; } template ::value, int> = 0> - detail::item_accessor operator[](T o) const { + detail::item_accessor operator[](T &&o) const { return object::operator[](std::forward(o)); } detail::sequence_iterator begin() const { return {*this, 0}; } @@ -1775,7 +1775,7 @@ class list : public object { bool empty() const { return size() == 0; } detail::list_accessor operator[](size_t index) const { return {*this, index}; } template ::value, int> = 0> - detail::item_accessor operator[](T o) const { + detail::item_accessor operator[](T &&o) const { return object::operator[](std::forward(o)); } detail::list_iterator begin() const { return {*this, 0}; } From 7e5fcc45849b613a6855d0f43d8973eb9210a3ea Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Fri, 27 May 2022 13:07:07 -0400 Subject: [PATCH 10/20] Add a couple of tests --- tests/test_pytypes.cpp | 22 ++++++++++++++++++++++ tests/test_pytypes.py | 12 ++++++++++++ 2 files changed, 34 insertions(+) diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp index 8d296f655a..dd627a164b 100644 --- a/tests/test_pytypes.cpp +++ b/tests/test_pytypes.cpp @@ -7,6 +7,7 @@ BSD-style license that can be found in the LICENSE file. */ +#include "pybind11/detail/common.h" #include "pybind11_tests.h" #include @@ -612,4 +613,25 @@ TEST_SUBMODULE(pytypes, m) { double v = x.get_value(); return v * v; }); + + m.def("tuple_rvalue_attr", [](const py::tuple &tup) { + // tests assigning rvalue objects to tuple + for (size_t i = 0; i < tup.size(); i++) { + auto o = py::handle(tup[py::int_(i)]); + if (!o) { + throw py::value_error("tuple is malformed"); + } + } + return tup; + }); + m.def("list_rvalue_attr", [](const py::list &l) { + // tests assigning rvalue objects to tuple + for (size_t i = 0; i < l.size(); i++) { + auto o = py::handle(l[py::int_(i)]); + if (!o) { + throw py::value_error("list is malformed"); + } + } + return l; + }); } diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py index 9afe62f424..af029749f2 100644 --- a/tests/test_pytypes.py +++ b/tests/test_pytypes.py @@ -698,3 +698,15 @@ def test_implementation_details(): def test_external_float_(): r1 = m.square_float_(2.0) assert r1 == 4.0 + + +def test_tuple_rvalue_attr(): + pop = 1000 + tup = tuple(range(pop)) + m.tuple_rvalue_attr(tup) + + +def test_list_rvalue_attr(): + pop = 1000 + my_list = list(range(pop)) + m.list_rvalue_attr(my_list) From 167649c2d4b2879a0c7151cb4d50a96b4a2f16f1 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Fri, 27 May 2022 13:07:49 -0400 Subject: [PATCH 11/20] Remove errant include --- tests/test_pytypes.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp index dd627a164b..d99ab417de 100644 --- a/tests/test_pytypes.cpp +++ b/tests/test_pytypes.cpp @@ -7,7 +7,6 @@ BSD-style license that can be found in the LICENSE file. */ -#include "pybind11/detail/common.h" #include "pybind11_tests.h" #include From cee6619187fff40b3dc3a9649efc9f731bf73c8c Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Fri, 27 May 2022 13:10:37 -0400 Subject: [PATCH 12/20] Improve test documentation --- tests/test_pytypes.cpp | 8 ++++---- tests/test_pytypes.py | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp index d99ab417de..51ac8bee2d 100644 --- a/tests/test_pytypes.cpp +++ b/tests/test_pytypes.cpp @@ -613,8 +613,8 @@ TEST_SUBMODULE(pytypes, m) { return v * v; }); - m.def("tuple_rvalue_attr", [](const py::tuple &tup) { - // tests assigning rvalue objects to tuple + m.def("tuple_rvalue_getter", [](const py::tuple &tup) { + // tests accessing tuple object with rvalue int for (size_t i = 0; i < tup.size(); i++) { auto o = py::handle(tup[py::int_(i)]); if (!o) { @@ -623,8 +623,8 @@ TEST_SUBMODULE(pytypes, m) { } return tup; }); - m.def("list_rvalue_attr", [](const py::list &l) { - // tests assigning rvalue objects to tuple + m.def("list_rvalue_getter", [](const py::list &l) { + // tests accessing list with rvalue int for (size_t i = 0; i < l.size(); i++) { auto o = py::handle(l[py::int_(i)]); if (!o) { diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py index af029749f2..66401020f0 100644 --- a/tests/test_pytypes.py +++ b/tests/test_pytypes.py @@ -700,13 +700,13 @@ def test_external_float_(): assert r1 == 4.0 -def test_tuple_rvalue_attr(): +def test_tuple_rvalue_getter(): pop = 1000 tup = tuple(range(pop)) - m.tuple_rvalue_attr(tup) + m.tuple_rvalue_getter(tup) -def test_list_rvalue_attr(): +def test_list_rvalue_getter(): pop = 1000 my_list = list(range(pop)) - m.list_rvalue_attr(my_list) + m.list_rvalue_getter(my_list) From 300d485a02693ca464dcb9e244c96608501d1c89 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Fri, 27 May 2022 13:18:55 -0400 Subject: [PATCH 13/20] Add dict test --- tests/test_pytypes.cpp | 7 +++++++ tests/test_pytypes.py | 6 ++++++ 2 files changed, 13 insertions(+) diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp index 51ac8bee2d..aa181517ec 100644 --- a/tests/test_pytypes.cpp +++ b/tests/test_pytypes.cpp @@ -633,4 +633,11 @@ TEST_SUBMODULE(pytypes, m) { } return l; }); + m.def("populate_dict_rvalue", [](int population) { + auto d = py::dict(); + for (int i = 0; i < population; i++) { + d[py::int_(i)] = py::int_(i); + } + return d; + }); } diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py index 66401020f0..407ded154c 100644 --- a/tests/test_pytypes.py +++ b/tests/test_pytypes.py @@ -710,3 +710,9 @@ def test_list_rvalue_getter(): pop = 1000 my_list = list(range(pop)) m.list_rvalue_getter(my_list) + + +def test_populate_dict_rvalue(): + pop = 1000 + my_dict = {i: i for i in range(pop)} + assert m.populate_dict_rvalue(pop) == my_dict From 29250abab49b860aea5375d68753adb4ff38e5d7 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Fri, 27 May 2022 13:50:21 -0400 Subject: [PATCH 14/20] add object attr tests --- tests/test_pytypes.cpp | 6 ++++++ tests/test_pytypes.py | 10 ++++++++++ 2 files changed, 16 insertions(+) diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp index aa181517ec..fe07e06136 100644 --- a/tests/test_pytypes.cpp +++ b/tests/test_pytypes.cpp @@ -640,4 +640,10 @@ TEST_SUBMODULE(pytypes, m) { } return d; }); + m.def("populate_obj_str_attrs", [](py::object &o, int population) { + for (int i = 0; i < population; i++) { + o.attr(py::str(py::int_(i))) = py::str(py::int_(i)); + } + return o; + }); } diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py index 407ded154c..56bd06c8a8 100644 --- a/tests/test_pytypes.py +++ b/tests/test_pytypes.py @@ -1,5 +1,6 @@ import contextlib import sys +import types import pytest @@ -716,3 +717,12 @@ def test_populate_dict_rvalue(): pop = 1000 my_dict = {i: i for i in range(pop)} assert m.populate_dict_rvalue(pop) == my_dict + + +def test_populate_obj_str_attrs(): + pop = 1000 + o = types.SimpleNamespace(**{str(i): i for i in range(pop)}) + new_o = m.populate_obj_str_attrs(o, pop) + new_attrs = {k: v for k, v in new_o.__dict__.items() if not k.startswith("_")} + assert all(isinstance(v, str) for v in new_attrs.values()) + assert len(new_attrs) == pop From 33fb072b63fd3ab1b9d27194c23d6c1de8cb02b5 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Sat, 28 May 2022 12:53:35 -0400 Subject: [PATCH 15/20] Optimize STL map caster and cleanup enum --- include/pybind11/pybind11.h | 7 ++++--- include/pybind11/stl.h | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 4c392713c9..a1af80f0f5 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -2057,17 +2057,18 @@ struct enum_base { [](const object &arg) { return int_(arg); }, name("__hash__"), is_method(m_base)); } - PYBIND11_NOINLINE void value(char const *name_, object value, const char *doc = nullptr) { + PYBIND11_NOINLINE void + value(char const *name_, const object &value, const char *doc = nullptr) { dict entries = m_base.attr("__entries"); str name(name_); if (entries.contains(name)) { std::string type_name = (std::string) str(m_base.attr("__name__")); - throw value_error(type_name + ": element \"" + std::string(name_) + throw value_error(std::move(type_name) + ": element \"" + std::string(name_) + "\" already exists!"); } entries[name] = std::make_pair(value, doc); - m_base.attr(name) = value; + m_base.attr(std::move(name)) = value; } PYBIND11_NOINLINE void export_values() { diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index 597bce61d5..ab30ecac0b 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -128,7 +128,7 @@ struct map_caster { if (!key || !value) { return handle(); } - d[key] = value; + d[std::move(key)] = std::move(value); } return d.release(); } From 6de828702bac7ccba5ca3ce305740841c01afd4e Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Tue, 31 May 2022 12:55:46 -0400 Subject: [PATCH 16/20] Reorder to match declarations --- include/pybind11/pytypes.h | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index ad67ab83d8..f6424e0be4 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -2073,26 +2073,26 @@ iterator object_api::end() const { return iterator::sentinel(); } template -item_accessor object_api::operator[](object &&key) const { - return {derived(), std::move(key)}; -} -template item_accessor object_api::operator[](handle key) const { return {derived(), reinterpret_borrow(key)}; } template -item_accessor object_api::operator[](const char *key) const { - return {derived(), pybind11::str(key)}; +item_accessor object_api::operator[](object &&key) const { + return {derived(), std::move(key)}; } template -obj_attr_accessor object_api::attr(object &&key) const { - return {derived(), std::move(key)}; +item_accessor object_api::operator[](const char *key) const { + return {derived(), pybind11::str(key)}; } template obj_attr_accessor object_api::attr(handle key) const { return {derived(), reinterpret_borrow(key)}; } template +obj_attr_accessor object_api::attr(object &&key) const { + return {derived(), std::move(key)}; +} +template str_attr_accessor object_api::attr(const char *key) const { return {derived(), key}; } From 3c3d3460498438a50de944a831b8d84804797806 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Tue, 31 May 2022 16:31:00 -0400 Subject: [PATCH 17/20] adjust increfs --- tests/test_pytypes.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py index d29d838d86..614061bf46 100644 --- a/tests/test_pytypes.py +++ b/tests/test_pytypes.py @@ -322,7 +322,7 @@ def test_accessor_moves(): inc_refs = m.accessor_moves() if inc_refs: # To be changed in PR #3970: [1, 0, 1, 0, ...] - assert inc_refs == [1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1] + assert inc_refs == [1, 0, 1, 0, 1, 0, 1, 0, 1, 0, 1, 0, 1, 0] else: pytest.skip("Not defined: PYBIND11_HANDLE_REF_DEBUG") From fbe240478e7a968eef345722854b91e3ae447f30 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Tue, 31 May 2022 16:32:01 -0400 Subject: [PATCH 18/20] Remove comment --- tests/test_pytypes.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py index 614061bf46..3e9d51a27c 100644 --- a/tests/test_pytypes.py +++ b/tests/test_pytypes.py @@ -321,7 +321,6 @@ def func(self, x, *args): def test_accessor_moves(): inc_refs = m.accessor_moves() if inc_refs: - # To be changed in PR #3970: [1, 0, 1, 0, ...] assert inc_refs == [1, 0, 1, 0, 1, 0, 1, 0, 1, 0, 1, 0, 1, 0] else: pytest.skip("Not defined: PYBIND11_HANDLE_REF_DEBUG") From 1603ac246f9c096dc40f152fae52c5c82826c351 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Wed, 1 Jun 2022 11:55:56 -0400 Subject: [PATCH 19/20] revert value change --- include/pybind11/pybind11.h | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 6965340f1f..f817f0aaa6 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -2064,8 +2064,7 @@ struct enum_base { [](const object &arg) { return int_(arg); }, name("__hash__"), is_method(m_base)); } - PYBIND11_NOINLINE void - value(char const *name_, const object &value, const char *doc = nullptr) { + PYBIND11_NOINLINE void value(char const *name_, object value, const char *doc = nullptr) { dict entries = m_base.attr("__entries"); str name(name_); if (entries.contains(name)) { @@ -2075,7 +2074,7 @@ struct enum_base { } entries[name] = std::make_pair(value, doc); - m_base.attr(std::move(name)) = value; + m_base.attr(std::move(name)) = std::move(value); } PYBIND11_NOINLINE void export_values() { From 75bf0bdc24c2273fc7eac2658f8f88d0aca5c459 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Wed, 1 Jun 2022 12:51:12 -0400 Subject: [PATCH 20/20] add missing move --- include/pybind11/pybind11.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index f817f0aaa6..cfa4420679 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -2610,7 +2610,7 @@ PYBIND11_NOINLINE void print(const tuple &args, const dict &kwargs) { } auto write = file.attr("write"); - write(line); + write(std::move(line)); write(kwargs.contains("end") ? kwargs["end"] : str("\n")); if (kwargs.contains("flush") && kwargs["flush"].cast()) {