From a398f7b2b6056706f14133c9d9d86424ad45b6b1 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Thu, 12 Aug 2021 13:05:56 -0400 Subject: [PATCH 1/6] Fix errant const methods --- include/pybind11/pytypes.h | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index c7b2501feb..7757ae005f 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -1393,7 +1393,8 @@ class dict : public object { bool empty() const { return size() == 0; } detail::dict_iterator begin() const { return {*this, 0}; } detail::dict_iterator end() const { return {}; } - void clear() const { PyDict_Clear(ptr()); } + //NOLINTNEXTLINE(readability-make-member-function-const) + void clear() { PyDict_Clear(ptr()); } template bool contains(T &&key) const { return PyDict_Contains(m_ptr, detail::object_or_cast(std::forward(key)).ptr()) == 1; } @@ -1435,10 +1436,12 @@ class list : public object { detail::item_accessor operator[](handle h) const { return object::operator[](h); } detail::list_iterator begin() const { return {*this, 0}; } detail::list_iterator end() const { return {*this, PyList_GET_SIZE(m_ptr)}; } - template void append(T &&val) const { + //NOLINTNEXTLINE(readability-make-member-function-const) + template void append(T &&val) { PyList_Append(m_ptr, detail::object_or_cast(std::forward(val)).ptr()); } - template void insert(size_t index, T &&val) const { + //NOLINTNEXTLINE(readability-make-member-function-const) + template void insert(size_t index, T &&val) { PyList_Insert(m_ptr, static_cast(index), detail::object_or_cast(std::forward(val)).ptr()); } @@ -1455,9 +1458,11 @@ class set : public object { } size_t size() const { return (size_t) PySet_Size(m_ptr); } bool empty() const { return size() == 0; } - template bool add(T &&val) const { + //NOLINTNEXTLINE(readability-make-member-function-const) + template bool add(T &&val) { return PySet_Add(m_ptr, detail::object_or_cast(std::forward(val)).ptr()) == 0; } + //NOLINTNEXTLINE(readability-make-member-function-const) void clear() const { PySet_Clear(m_ptr); } template bool contains(T &&val) const { return PySet_Contains(m_ptr, detail::object_or_cast(std::forward(val)).ptr()) == 1; From 3678aed71c56bd523ae3d4ed8f91894a587a2c9f Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Thu, 12 Aug 2021 13:42:14 -0400 Subject: [PATCH 2/6] Remove NOLINT since clang-tidy is pretty conservative --- include/pybind11/pytypes.h | 5 ----- 1 file changed, 5 deletions(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 7757ae005f..21c22f688f 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -1393,7 +1393,6 @@ class dict : public object { bool empty() const { return size() == 0; } detail::dict_iterator begin() const { return {*this, 0}; } detail::dict_iterator end() const { return {}; } - //NOLINTNEXTLINE(readability-make-member-function-const) void clear() { PyDict_Clear(ptr()); } template bool contains(T &&key) const { return PyDict_Contains(m_ptr, detail::object_or_cast(std::forward(key)).ptr()) == 1; @@ -1436,11 +1435,9 @@ class list : public object { detail::item_accessor operator[](handle h) const { return object::operator[](h); } detail::list_iterator begin() const { return {*this, 0}; } detail::list_iterator end() const { return {*this, PyList_GET_SIZE(m_ptr)}; } - //NOLINTNEXTLINE(readability-make-member-function-const) template void append(T &&val) { PyList_Append(m_ptr, detail::object_or_cast(std::forward(val)).ptr()); } - //NOLINTNEXTLINE(readability-make-member-function-const) template void insert(size_t index, T &&val) { PyList_Insert(m_ptr, static_cast(index), detail::object_or_cast(std::forward(val)).ptr()); @@ -1458,11 +1455,9 @@ class set : public object { } size_t size() const { return (size_t) PySet_Size(m_ptr); } bool empty() const { return size() == 0; } - //NOLINTNEXTLINE(readability-make-member-function-const) template bool add(T &&val) { return PySet_Add(m_ptr, detail::object_or_cast(std::forward(val)).ptr()) == 0; } - //NOLINTNEXTLINE(readability-make-member-function-const) void clear() const { PySet_Clear(m_ptr); } template bool contains(T &&val) const { return PySet_Contains(m_ptr, detail::object_or_cast(std::forward(val)).ptr()) == 1; From 8071bccd35cb20d3bd83f55d6aad6ce109f17fd0 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Thu, 12 Aug 2021 13:45:19 -0400 Subject: [PATCH 3/6] Missed one --- 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 21c22f688f..be0bf04358 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -1458,7 +1458,7 @@ class set : public object { template bool add(T &&val) { return PySet_Add(m_ptr, detail::object_or_cast(std::forward(val)).ptr()) == 0; } - void clear() const { PySet_Clear(m_ptr); } + void clear() { PySet_Clear(m_ptr); } template bool contains(T &&val) const { return PySet_Contains(m_ptr, detail::object_or_cast(std::forward(val)).ptr()) == 1; } From 97cb3878d300e68356fce25a50b5c606634a9b89 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Thu, 12 Aug 2021 14:18:28 -0400 Subject: [PATCH 4/6] Fix a few more errors --- include/pybind11/detail/common.h | 2 +- include/pybind11/detail/type_caster_base.h | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index e8d83ae0db..7d8df55e6b 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -507,7 +507,7 @@ struct instance { void allocate_layout(); /// Destroys/deallocates all of the above - void deallocate_layout() const; + void deallocate_layout(); /// Returns the value_and_holder wrapper for the given type (or the first, if `find_type` /// omitted). Returns a default-constructed (with `.inst = nullptr`) object on failure if diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h index e2d1bcb8cc..3987d1639d 100644 --- a/include/pybind11/detail/type_caster_base.h +++ b/include/pybind11/detail/type_caster_base.h @@ -241,7 +241,8 @@ struct value_and_holder { ? inst->simple_holder_constructed : (inst->nonsimple.status[index] & instance::status_holder_constructed) != 0u; } - void set_holder_constructed(bool v = true) const { + //NOLINTNEXTLINE(readability-make-member-function-const) + void set_holder_constructed(bool v = true) { if (inst->simple_layout) inst->simple_holder_constructed = v; else if (v) @@ -254,7 +255,8 @@ struct value_and_holder { ? inst->simple_instance_registered : ((inst->nonsimple.status[index] & instance::status_instance_registered) != 0); } - void set_instance_registered(bool v = true) const { + //NOLINTNEXTLINE(readability-make-member-function-const) + void set_instance_registered(bool v = true) { if (inst->simple_layout) inst->simple_instance_registered = v; else if (v) @@ -397,7 +399,8 @@ PYBIND11_NOINLINE inline void instance::allocate_layout() { owned = true; } -PYBIND11_NOINLINE inline void instance::deallocate_layout() const { +//NOLINTNEXTLINE(readability-make-member-function-const) +PYBIND11_NOINLINE inline void instance::deallocate_layout() { if (!simple_layout) PyMem_Free(nonsimple.values_and_holders); } From 77fc6dfad84df34def99df6e96ca1d4983b33661 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Fri, 13 Aug 2021 11:16:42 -0400 Subject: [PATCH 5/6] Add reviewer suggested comments --- include/pybind11/pytypes.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 6ecd81e0f4..dc1607ff26 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -1393,7 +1393,7 @@ class dict : public object { bool empty() const { return size() == 0; } detail::dict_iterator begin() const { return {*this, 0}; } detail::dict_iterator end() const { return {}; } - void clear() { PyDict_Clear(ptr()); } + void clear() /* py-non-const */ { PyDict_Clear(ptr()); } template bool contains(T &&key) const { return PyDict_Contains(m_ptr, detail::object_or_cast(std::forward(key)).ptr()) == 1; } @@ -1435,10 +1435,10 @@ class list : public object { detail::item_accessor operator[](handle h) const { return object::operator[](h); } detail::list_iterator begin() const { return {*this, 0}; } detail::list_iterator end() const { return {*this, PyList_GET_SIZE(m_ptr)}; } - template void append(T &&val) { + template void append(T &&val) /* py-non-const */ { PyList_Append(m_ptr, detail::object_or_cast(std::forward(val)).ptr()); } - template void insert(size_t index, T &&val) { + template void insert(size_t index, T &&val) /* py-non-const */ { PyList_Insert(m_ptr, static_cast(index), detail::object_or_cast(std::forward(val)).ptr()); } @@ -1455,10 +1455,10 @@ class set : public object { } size_t size() const { return (size_t) PySet_Size(m_ptr); } bool empty() const { return size() == 0; } - template bool add(T &&val) { + template bool add(T &&val) /* py-non-const */ { return PySet_Add(m_ptr, detail::object_or_cast(std::forward(val)).ptr()) == 0; } - void clear() { PySet_Clear(m_ptr); } + void clear() /* py-non-const */ { PySet_Clear(m_ptr); } template bool contains(T &&val) const { return PySet_Contains(m_ptr, detail::object_or_cast(std::forward(val)).ptr()) == 1; } From d9052cfb70739a3253b7f5517b66018ee1e92262 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Fri, 13 Aug 2021 16:20:18 -0400 Subject: [PATCH 6/6] Run clang-format --- include/pybind11/detail/type_caster_base.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h index 1671e93528..5a5acc2e6a 100644 --- a/include/pybind11/detail/type_caster_base.h +++ b/include/pybind11/detail/type_caster_base.h @@ -241,7 +241,7 @@ struct value_and_holder { ? inst->simple_holder_constructed : (inst->nonsimple.status[index] & instance::status_holder_constructed) != 0u; } - //NOLINTNEXTLINE(readability-make-member-function-const) + // NOLINTNEXTLINE(readability-make-member-function-const) void set_holder_constructed(bool v = true) { if (inst->simple_layout) inst->simple_holder_constructed = v; @@ -255,7 +255,7 @@ struct value_and_holder { ? inst->simple_instance_registered : ((inst->nonsimple.status[index] & instance::status_instance_registered) != 0); } - //NOLINTNEXTLINE(readability-make-member-function-const) + // NOLINTNEXTLINE(readability-make-member-function-const) void set_instance_registered(bool v = true) { if (inst->simple_layout) inst->simple_instance_registered = v; @@ -399,7 +399,7 @@ PYBIND11_NOINLINE void instance::allocate_layout() { owned = true; } -//NOLINTNEXTLINE(readability-make-member-function-const) +// NOLINTNEXTLINE(readability-make-member-function-const) PYBIND11_NOINLINE void instance::deallocate_layout() { if (!simple_layout) PyMem_Free(nonsimple.values_and_holders);