From 976fea05a36585958d067a26092b48288a2af977 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20Gr=C3=BCninger?= Date: Sat, 30 Dec 2023 06:59:47 +0100 Subject: [PATCH 1/7] Fix Clazy warnings (#4988) * stl.h: Use C++11 range-loops with const reference This saves copy-ctor and dtor for non-trivial types by value Found by clazy (range-loop-reference) * test_smart_ptr.cpp cleanup Introduce `pointer_set` https://github.com/boostorg/unordered/issues/139 > Based on the standard, the first move should leave a in a "valid but unspecified state"; --------- Co-authored-by: Ralf W. Grosse-Kunstleve --- include/pybind11/stl.h | 4 ++-- tests/test_smart_ptr.cpp | 27 +++++++++++++++------------ 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index 6eb485991..f0334b8f8 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -172,7 +172,7 @@ struct list_caster { auto s = reinterpret_borrow(src); value.clear(); reserve_maybe(s, &value); - for (auto it : s) { + for (const auto &it : s) { value_conv conv; if (!conv.load(it, convert)) { return false; @@ -247,7 +247,7 @@ public: return false; } size_t ctr = 0; - for (auto it : l) { + for (const auto &it : l) { value_conv conv; if (!conv.load(it, convert)) { return false; diff --git a/tests/test_smart_ptr.cpp b/tests/test_smart_ptr.cpp index 6d9efcedc..496073b3c 100644 --- a/tests/test_smart_ptr.cpp +++ b/tests/test_smart_ptr.cpp @@ -103,21 +103,26 @@ private: int value; }; +template +std::unordered_set &pointer_set() { + // https://google.github.io/styleguide/cppguide.html#Static_and_Global_Variables + static auto singleton = new std::unordered_set(); + return *singleton; +} + // test_unique_nodelete // Object with a private destructor -class MyObject4; -std::unordered_set myobject4_instances; class MyObject4 { public: explicit MyObject4(int value) : value{value} { print_created(this); - myobject4_instances.insert(this); + pointer_set().insert(this); } int value; static void cleanupAllInstances() { - auto tmp = std::move(myobject4_instances); - myobject4_instances.clear(); + auto tmp = std::move(pointer_set()); + pointer_set().clear(); for (auto *o : tmp) { delete o; } @@ -125,7 +130,7 @@ public: private: ~MyObject4() { - myobject4_instances.erase(this); + pointer_set().erase(this); print_destroyed(this); } }; @@ -133,19 +138,17 @@ private: // test_unique_deleter // Object with std::unique_ptr where D is not matching the base class // Object with a protected destructor -class MyObject4a; -std::unordered_set myobject4a_instances; class MyObject4a { public: explicit MyObject4a(int i) : value{i} { print_created(this); - myobject4a_instances.insert(this); + pointer_set().insert(this); }; int value; static void cleanupAllInstances() { - auto tmp = std::move(myobject4a_instances); - myobject4a_instances.clear(); + auto tmp = std::move(pointer_set()); + pointer_set().clear(); for (auto *o : tmp) { delete o; } @@ -153,7 +156,7 @@ public: protected: virtual ~MyObject4a() { - myobject4a_instances.erase(this); + pointer_set().erase(this); print_destroyed(this); } }; From b583336cf762e24c81912aab2c3f74f14396e384 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 1 Jan 2024 18:51:02 -0800 Subject: [PATCH 2/7] chore(deps): bump ilammy/msvc-dev-cmd from 1.12.1 to 1.13.0 (#4995) Bumps [ilammy/msvc-dev-cmd](https://github.com/ilammy/msvc-dev-cmd) from 1.12.1 to 1.13.0. - [Release notes](https://github.com/ilammy/msvc-dev-cmd/releases) - [Commits](https://github.com/ilammy/msvc-dev-cmd/compare/v1.12.1...v1.13.0) --- updated-dependencies: - dependency-name: ilammy/msvc-dev-cmd dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7aead4570..9d4c8e1f4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -800,7 +800,7 @@ jobs: uses: jwlawson/actions-setup-cmake@v1.14 - name: Prepare MSVC - uses: ilammy/msvc-dev-cmd@v1.12.1 + uses: ilammy/msvc-dev-cmd@v1.13.0 with: arch: x86 @@ -853,7 +853,7 @@ jobs: uses: jwlawson/actions-setup-cmake@v1.14 - name: Prepare MSVC - uses: ilammy/msvc-dev-cmd@v1.12.1 + uses: ilammy/msvc-dev-cmd@v1.13.0 with: arch: x86 From f29def9ea467c7fde754440733047953fa4b990c Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 1 Jan 2024 19:25:58 -0800 Subject: [PATCH 3/7] chore(deps): update pre-commit hooks (#4994) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit updates: - [github.com/astral-sh/ruff-pre-commit: v0.1.6 → v0.1.9](https://github.com/astral-sh/ruff-pre-commit/compare/v0.1.6...v0.1.9) - [github.com/pre-commit/mirrors-mypy: v1.7.1 → v1.8.0](https://github.com/pre-commit/mirrors-mypy/compare/v1.7.1...v1.8.0) - [github.com/PyCQA/pylint: v3.0.1 → v3.0.3](https://github.com/PyCQA/pylint/compare/v3.0.1...v3.0.3) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- .pre-commit-config.yaml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 53f1a5ea1..cb28515d8 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -32,7 +32,7 @@ repos: # Ruff, the Python auto-correcting linter/formatter written in Rust - repo: https://github.com/astral-sh/ruff-pre-commit - rev: v0.1.6 + rev: v0.1.9 hooks: - id: ruff args: ["--fix", "--show-fixes"] @@ -40,7 +40,7 @@ repos: # Check static types with mypy - repo: https://github.com/pre-commit/mirrors-mypy - rev: "v1.7.1" + rev: "v1.8.0" hooks: - id: mypy args: [] @@ -142,7 +142,7 @@ repos: # PyLint has native support - not always usable, but works for us - repo: https://github.com/PyCQA/pylint - rev: "v3.0.1" + rev: "v3.0.3" hooks: - id: pylint files: ^pybind11 From aec6cc5406edb076f5a489c2d7f84bb07052c4a3 Mon Sep 17 00:00:00 2001 From: Ilya Lavrenov Date: Mon, 8 Jan 2024 17:40:41 +0400 Subject: [PATCH 4/7] fix(cmake): skip empty PYBIND11_PYTHON_EXECUTABLE_LAST for the first cmake run (#4856) * fix(cmake): skip empty PYBIND11_PYTHON_EXECUTABLE_LAST for the first cmake run * style: pre-commit fixes * Update pybind11NewTools.cmake * style: pre-commit fixes --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Henry Schreiner --- tools/pybind11NewTools.cmake | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tools/pybind11NewTools.cmake b/tools/pybind11NewTools.cmake index cd88a6450..9fe2eb08d 100644 --- a/tools/pybind11NewTools.cmake +++ b/tools/pybind11NewTools.cmake @@ -110,15 +110,17 @@ if(NOT DEFINED ${_Python}_EXECUTABLE) endif() -if(NOT ${_Python}_EXECUTABLE STREQUAL PYBIND11_PYTHON_EXECUTABLE_LAST) +if(DEFINED PYBIND11_PYTHON_EXECUTABLE_LAST AND NOT ${_Python}_EXECUTABLE STREQUAL + PYBIND11_PYTHON_EXECUTABLE_LAST) # Detect changes to the Python version/binary in subsequent CMake runs, and refresh config if needed unset(PYTHON_IS_DEBUG CACHE) unset(PYTHON_MODULE_EXTENSION CACHE) - set(PYBIND11_PYTHON_EXECUTABLE_LAST - "${${_Python}_EXECUTABLE}" - CACHE INTERNAL "Python executable during the last CMake run") endif() +set(PYBIND11_PYTHON_EXECUTABLE_LAST + "${${_Python}_EXECUTABLE}" + CACHE INTERNAL "Python executable during the last CMake run") + if(NOT DEFINED PYTHON_IS_DEBUG) # Debug check - see https://stackoverflow.com/questions/646518/python-how-to-detect-debug-Interpreter execute_process( From 31b7e14052b547a5ddf97c07840a830ec27524b7 Mon Sep 17 00:00:00 2001 From: Huanchen Zhai <44282896+hczhai@users.noreply.github.com> Date: Sat, 13 Jan 2024 11:07:36 -0800 Subject: [PATCH 5/7] bugfix: removing typing and duplicate ``class_`` for KeysView/ValuesView/ItemsView. Fix #4529 (#4985) * remove typing for KeysView/ValuesView/ItemsView * add tests for map view types --- include/pybind11/stl_bind.h | 79 ++++++++++++------------------------- tests/test_stl_binders.cpp | 10 +++++ tests/test_stl_binders.py | 30 ++++++++++++-- 3 files changed, 62 insertions(+), 57 deletions(-) diff --git a/include/pybind11/stl_bind.h b/include/pybind11/stl_bind.h index 4d0854f6a..a226cbc0e 100644 --- a/include/pybind11/stl_bind.h +++ b/include/pybind11/stl_bind.h @@ -645,49 +645,50 @@ auto map_if_insertion_operator(Class_ &cl, std::string const &name) "Return the canonical string representation of this map."); } -template struct keys_view { virtual size_t len() = 0; virtual iterator iter() = 0; - virtual bool contains(const KeyType &k) = 0; - virtual bool contains(const object &k) = 0; + virtual bool contains(const handle &k) = 0; virtual ~keys_view() = default; }; -template struct values_view { virtual size_t len() = 0; virtual iterator iter() = 0; virtual ~values_view() = default; }; -template struct items_view { virtual size_t len() = 0; virtual iterator iter() = 0; virtual ~items_view() = default; }; -template -struct KeysViewImpl : public KeysView { +template +struct KeysViewImpl : public detail::keys_view { explicit KeysViewImpl(Map &map) : map(map) {} size_t len() override { return map.size(); } iterator iter() override { return make_key_iterator(map.begin(), map.end()); } - bool contains(const typename Map::key_type &k) override { return map.find(k) != map.end(); } - bool contains(const object &) override { return false; } + bool contains(const handle &k) override { + try { + return map.find(k.template cast()) != map.end(); + } catch (const cast_error &) { + return false; + } + } Map ↦ }; -template -struct ValuesViewImpl : public ValuesView { +template +struct ValuesViewImpl : public detail::values_view { explicit ValuesViewImpl(Map &map) : map(map) {} size_t len() override { return map.size(); } iterator iter() override { return make_value_iterator(map.begin(), map.end()); } Map ↦ }; -template -struct ItemsViewImpl : public ItemsView { +template +struct ItemsViewImpl : public detail::items_view { explicit ItemsViewImpl(Map &map) : map(map) {} size_t len() override { return map.size(); } iterator iter() override { return make_iterator(map.begin(), map.end()); } @@ -700,11 +701,9 @@ template , typename... class_ bind_map(handle scope, const std::string &name, Args &&...args) { using KeyType = typename Map::key_type; using MappedType = typename Map::mapped_type; - using StrippedKeyType = detail::remove_cvref_t; - using StrippedMappedType = detail::remove_cvref_t; - using KeysView = detail::keys_view; - using ValuesView = detail::values_view; - using ItemsView = detail::items_view; + using KeysView = detail::keys_view; + using ValuesView = detail::values_view; + using ItemsView = detail::items_view; using Class_ = class_; // If either type is a non-module-local bound type then make the map binding non-local as well; @@ -718,39 +717,20 @@ class_ bind_map(handle scope, const std::string &name, Args && } Class_ cl(scope, name.c_str(), pybind11::module_local(local), std::forward(args)...); - static constexpr auto key_type_descr = detail::make_caster::name; - static constexpr auto mapped_type_descr = detail::make_caster::name; - std::string key_type_name(key_type_descr.text), mapped_type_name(mapped_type_descr.text); - // If key type isn't properly wrapped, fall back to C++ names - if (key_type_name == "%") { - key_type_name = detail::type_info_description(typeid(KeyType)); - } - // Similarly for value type: - if (mapped_type_name == "%") { - mapped_type_name = detail::type_info_description(typeid(MappedType)); - } - - // Wrap KeysView[KeyType] if it wasn't already wrapped + // Wrap KeysView if it wasn't already wrapped if (!detail::get_type_info(typeid(KeysView))) { - class_ keys_view( - scope, ("KeysView[" + key_type_name + "]").c_str(), pybind11::module_local(local)); + class_ keys_view(scope, "KeysView", pybind11::module_local(local)); keys_view.def("__len__", &KeysView::len); keys_view.def("__iter__", &KeysView::iter, keep_alive<0, 1>() /* Essential: keep view alive while iterator exists */ ); - keys_view.def("__contains__", - static_cast(&KeysView::contains)); - // Fallback for when the object is not of the key type - keys_view.def("__contains__", - static_cast(&KeysView::contains)); + keys_view.def("__contains__", &KeysView::contains); } // Similarly for ValuesView: if (!detail::get_type_info(typeid(ValuesView))) { - class_ values_view(scope, - ("ValuesView[" + mapped_type_name + "]").c_str(), - pybind11::module_local(local)); + class_ values_view(scope, "ValuesView", pybind11::module_local(local)); values_view.def("__len__", &ValuesView::len); values_view.def("__iter__", &ValuesView::iter, @@ -759,10 +739,7 @@ class_ bind_map(handle scope, const std::string &name, Args && } // Similarly for ItemsView: if (!detail::get_type_info(typeid(ItemsView))) { - class_ items_view( - scope, - ("ItemsView[" + key_type_name + ", ").append(mapped_type_name + "]").c_str(), - pybind11::module_local(local)); + class_ items_view(scope, "ItemsView", pybind11::module_local(local)); items_view.def("__len__", &ItemsView::len); items_view.def("__iter__", &ItemsView::iter, @@ -788,25 +765,19 @@ class_ bind_map(handle scope, const std::string &name, Args && cl.def( "keys", - [](Map &m) { - return std::unique_ptr(new detail::KeysViewImpl(m)); - }, + [](Map &m) { return std::unique_ptr(new detail::KeysViewImpl(m)); }, keep_alive<0, 1>() /* Essential: keep map alive while view exists */ ); cl.def( "values", - [](Map &m) { - return std::unique_ptr(new detail::ValuesViewImpl(m)); - }, + [](Map &m) { return std::unique_ptr(new detail::ValuesViewImpl(m)); }, keep_alive<0, 1>() /* Essential: keep map alive while view exists */ ); cl.def( "items", - [](Map &m) { - return std::unique_ptr(new detail::ItemsViewImpl(m)); - }, + [](Map &m) { return std::unique_ptr(new detail::ItemsViewImpl(m)); }, keep_alive<0, 1>() /* Essential: keep map alive while view exists */ ); diff --git a/tests/test_stl_binders.cpp b/tests/test_stl_binders.cpp index e52a03b6d..96af9a4c4 100644 --- a/tests/test_stl_binders.cpp +++ b/tests/test_stl_binders.cpp @@ -192,6 +192,16 @@ TEST_SUBMODULE(stl_binders, m) { py::bind_map>(m, "UnorderedMapStringDoubleConst"); + // test_map_view_types + py::bind_map>(m, "MapStringFloat"); + py::bind_map>(m, "UnorderedMapStringFloat"); + + py::bind_map, int32_t>>(m, "MapPairDoubleIntInt32"); + py::bind_map, int64_t>>(m, "MapPairDoubleIntInt64"); + + py::bind_map>(m, "MapIntObject"); + py::bind_map>(m, "MapStringObject"); + py::class_(m, "ENC").def(py::init()).def_readwrite("value", &E_nc::value); // test_noncopyable_containers diff --git a/tests/test_stl_binders.py b/tests/test_stl_binders.py index c00d45b92..d1bf64aa0 100644 --- a/tests/test_stl_binders.py +++ b/tests/test_stl_binders.py @@ -317,9 +317,9 @@ def test_map_view_types(): map_string_double_const = m.MapStringDoubleConst() unordered_map_string_double_const = m.UnorderedMapStringDoubleConst() - assert map_string_double.keys().__class__.__name__ == "KeysView[str]" - assert map_string_double.values().__class__.__name__ == "ValuesView[float]" - assert map_string_double.items().__class__.__name__ == "ItemsView[str, float]" + assert map_string_double.keys().__class__.__name__ == "KeysView" + assert map_string_double.values().__class__.__name__ == "ValuesView" + assert map_string_double.items().__class__.__name__ == "ItemsView" keys_type = type(map_string_double.keys()) assert type(unordered_map_string_double.keys()) is keys_type @@ -336,6 +336,30 @@ def test_map_view_types(): assert type(map_string_double_const.items()) is items_type assert type(unordered_map_string_double_const.items()) is items_type + map_string_float = m.MapStringFloat() + unordered_map_string_float = m.UnorderedMapStringFloat() + + assert type(map_string_float.keys()) is keys_type + assert type(unordered_map_string_float.keys()) is keys_type + assert type(map_string_float.values()) is values_type + assert type(unordered_map_string_float.values()) is values_type + assert type(map_string_float.items()) is items_type + assert type(unordered_map_string_float.items()) is items_type + + map_pair_double_int_int32 = m.MapPairDoubleIntInt32() + map_pair_double_int_int64 = m.MapPairDoubleIntInt64() + + assert type(map_pair_double_int_int32.values()) is values_type + assert type(map_pair_double_int_int64.values()) is values_type + + map_int_object = m.MapIntObject() + map_string_object = m.MapStringObject() + + assert type(map_int_object.keys()) is keys_type + assert type(map_string_object.keys()) is keys_type + assert type(map_int_object.items()) is items_type + assert type(map_string_object.items()) is items_type + def test_recursive_vector(): recursive_vector = m.RecursiveVector() From 39e65e10d05bfdc80413a6290bdae3391f0f93d7 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Sat, 13 Jan 2024 15:28:39 -0500 Subject: [PATCH 6/7] ci: group dependabot updates (#4986) --- .github/dependabot.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 2c7d17083..6c4b36953 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -4,4 +4,8 @@ updates: - package-ecosystem: "github-actions" directory: "/" schedule: - interval: "daily" + interval: "weekly" + groups: + actions: + patterns: + - "*" From 869cc1ff085dd405635b00eb46e5c84f50f26099 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Tue, 16 Jan 2024 21:09:20 -0800 Subject: [PATCH 7/7] install mingw-w64-${{matrix.env}}-python-scipy only for mingw64 (#5006) --- .github/workflows/ci.yml | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9d4c8e1f4..9def058f5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -959,7 +959,6 @@ jobs: mingw-w64-${{matrix.env}}-gcc mingw-w64-${{matrix.env}}-python-pip mingw-w64-${{matrix.env}}-python-numpy - mingw-w64-${{matrix.env}}-python-scipy mingw-w64-${{matrix.env}}-cmake mingw-w64-${{matrix.env}}-make mingw-w64-${{matrix.env}}-python-pytest @@ -967,6 +966,14 @@ jobs: mingw-w64-${{matrix.env}}-boost mingw-w64-${{matrix.env}}-catch + - uses: msys2/setup-msys2@v2 + if: matrix.sys == 'mingw64' + with: + msystem: ${{matrix.sys}} + install: >- + git + mingw-w64-${{matrix.env}}-python-scipy + - uses: actions/checkout@v4 - name: Configure C++11