From 1a6fca367fff672ea5c25c8507750c40a3354e36 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Tue, 20 Aug 2024 16:53:44 -0400 Subject: [PATCH 1/5] fix: allow -Wpedantic again Signed-off-by: Henry Schreiner --- docs/advanced/cast/stl.rst | 6 +++--- docs/advanced/smart_ptrs.rst | 6 +++--- include/pybind11/cast.h | 4 ++++ include/pybind11/detail/common.h | 4 ++++ tests/CMakeLists.txt | 1 + tests/local_bindings.h | 16 ++++++++-------- tests/test_callbacks.cpp | 2 +- tests/test_eigen_matrix.cpp | 4 ++-- tests/test_opaque_types.cpp | 2 +- tests/test_sequences_and_iterators.cpp | 4 ++-- tests/test_smart_ptr.cpp | 12 ++++++------ tests/test_stl.cpp | 2 +- 12 files changed, 36 insertions(+), 27 deletions(-) diff --git a/docs/advanced/cast/stl.rst b/docs/advanced/cast/stl.rst index 03d49b2950..42b85532d8 100644 --- a/docs/advanced/cast/stl.rst +++ b/docs/advanced/cast/stl.rst @@ -162,7 +162,7 @@ the declaration .. code-block:: cpp - PYBIND11_MAKE_OPAQUE(std::vector); + PYBIND11_MAKE_OPAQUE(std::vector) before any binding code (e.g. invocations to ``class_::def()``, etc.). This macro must be specified at the top level (and outside of any namespaces), since @@ -207,8 +207,8 @@ The following example showcases usage of :file:`pybind11/stl_bind.h`: // Don't forget this #include - PYBIND11_MAKE_OPAQUE(std::vector); - PYBIND11_MAKE_OPAQUE(std::map); + PYBIND11_MAKE_OPAQUE(std::vector) + PYBIND11_MAKE_OPAQUE(std::map) // ... diff --git a/docs/advanced/smart_ptrs.rst b/docs/advanced/smart_ptrs.rst index 3c40ce1237..b9f100cf8e 100644 --- a/docs/advanced/smart_ptrs.rst +++ b/docs/advanced/smart_ptrs.rst @@ -124,7 +124,7 @@ top namespace level before any binding code: .. code-block:: cpp - PYBIND11_DECLARE_HOLDER_TYPE(T, SmartPtr); + PYBIND11_DECLARE_HOLDER_TYPE(T, SmartPtr) The first argument of :func:`PYBIND11_DECLARE_HOLDER_TYPE` should be a placeholder name that is used as a template parameter of the second argument. @@ -136,7 +136,7 @@ by default. Specify .. code-block:: cpp - PYBIND11_DECLARE_HOLDER_TYPE(T, SmartPtr, true); + PYBIND11_DECLARE_HOLDER_TYPE(T, SmartPtr, true) if ``SmartPtr`` can always be initialized from a ``T*`` pointer without the risk of inconsistencies (such as multiple independent ``SmartPtr`` instances @@ -154,7 +154,7 @@ specialized: .. code-block:: cpp // Always needed for custom holder types - PYBIND11_DECLARE_HOLDER_TYPE(T, SmartPtr); + PYBIND11_DECLARE_HOLDER_TYPE(T, SmartPtr) // Only needed if the type's `.get()` goes by another name namespace PYBIND11_NAMESPACE { namespace detail { diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 0c862e4bec..1e5353776c 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -868,6 +868,9 @@ struct always_construct_holder { static constexpr bool value = Value; }; +PYBIND11_WARNING_PUSH +PYBIND11_WARNING_DISABLE_GCC("-Wpedantic") +PYBIND11_WARNING_DISABLE_CLANG("-Wgnu-zero-variadic-macro-arguments") /// Create a specialization for custom holder types (silently ignores std::shared_ptr) #define PYBIND11_DECLARE_HOLDER_TYPE(type, holder_type, ...) \ PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) \ @@ -880,6 +883,7 @@ struct always_construct_holder { : public type_caster_holder {}; \ } \ PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE) +PYBIND11_WARNING_POP // PYBIND11_DECLARE_HOLDER_TYPE holder types: template diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index 643fd33d7e..88ab5a29aa 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -479,6 +479,9 @@ PYBIND11_WARNING_POP } \endrst */ +PYBIND11_WARNING_PUSH +PYBIND11_WARNING_DISABLE_GCC("-Wpedantic") +PYBIND11_WARNING_DISABLE_CLANG("-Wgnu-zero-variadic-macro-arguments") #define PYBIND11_MODULE(name, variable, ...) \ static ::pybind11::module_::module_def PYBIND11_CONCAT(pybind11_module_def_, name) \ PYBIND11_MAYBE_UNUSED; \ @@ -499,6 +502,7 @@ PYBIND11_WARNING_POP PYBIND11_CATCH_INIT_EXCEPTIONS \ } \ void PYBIND11_CONCAT(pybind11_init_, name)(::pybind11::module_ & (variable)) +PYBIND11_WARNING_POP PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 18308731ab..bfc2e94df9 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -377,6 +377,7 @@ function(pybind11_enable_warnings target_name) ${target_name} PRIVATE -Wall -Wextra + -Wpedantic -Wconversion -Wcast-qual -Wdeprecated diff --git a/tests/local_bindings.h b/tests/local_bindings.h index 01d2785353..7e109d915d 100644 --- a/tests/local_bindings.h +++ b/tests/local_bindings.h @@ -56,13 +56,13 @@ class LocalSimpleException : public std::exception { std::string message = ""; }; -PYBIND11_MAKE_OPAQUE(LocalVec); -PYBIND11_MAKE_OPAQUE(LocalVec2); -PYBIND11_MAKE_OPAQUE(LocalMap); -PYBIND11_MAKE_OPAQUE(NonLocalVec); -// PYBIND11_MAKE_OPAQUE(NonLocalVec2); // same type as LocalVec2 -PYBIND11_MAKE_OPAQUE(NonLocalMap); -PYBIND11_MAKE_OPAQUE(NonLocalMap2); +PYBIND11_MAKE_OPAQUE(LocalVec) +PYBIND11_MAKE_OPAQUE(LocalVec2) +PYBIND11_MAKE_OPAQUE(LocalMap) +PYBIND11_MAKE_OPAQUE(NonLocalVec) +// PYBIND11_MAKE_OPAQUE(NonLocalVec2) +PYBIND11_MAKE_OPAQUE(NonLocalMap) +PYBIND11_MAKE_OPAQUE(NonLocalMap2) // Simple bindings (used with the above): template @@ -70,7 +70,7 @@ py::class_ bind_local(Args &&...args) { return py::class_(std::forward(args)...).def(py::init()).def("get", [](T &i) { return i.i + Adjust; }); -}; +} // Simulate a foreign library base class (to match the example in the docs): namespace pets { diff --git a/tests/test_callbacks.cpp b/tests/test_callbacks.cpp index 2fd05dec72..e303e76567 100644 --- a/tests/test_callbacks.cpp +++ b/tests/test_callbacks.cpp @@ -148,7 +148,7 @@ TEST_SUBMODULE(callbacks, m) { m.def("dummy_function2", [](int i, int j) { return i + j; }); m.def( "roundtrip", - [](std::function f, bool expect_none = false) { + [](std::function f, bool expect_none) { if (expect_none && f) { throw std::runtime_error("Expected None to be converted to empty std::function"); } diff --git a/tests/test_eigen_matrix.cpp b/tests/test_eigen_matrix.cpp index 21261bfc2f..cb8e8c6259 100644 --- a/tests/test_eigen_matrix.cpp +++ b/tests/test_eigen_matrix.cpp @@ -55,7 +55,7 @@ void reset_refs() { } // Returns element 2,1 from a matrix (used to test copy/nocopy) -double get_elem(const Eigen::Ref &m) { return m(2, 1); }; +double get_elem(const Eigen::Ref &m) { return m(2, 1); } // Returns a matrix with 10*r + 100*c added to each matrix element (to help test that the matrix // reference is referencing rows/columns correctly). @@ -76,7 +76,7 @@ struct CustomOperatorNew { Eigen::Matrix4d a = Eigen::Matrix4d::Zero(); Eigen::Matrix4d b = Eigen::Matrix4d::Identity(); - EIGEN_MAKE_ALIGNED_OPERATOR_NEW; + EIGEN_MAKE_ALIGNED_OPERATOR_NEW }; TEST_SUBMODULE(eigen_matrix, m) { diff --git a/tests/test_opaque_types.cpp b/tests/test_opaque_types.cpp index 154d0a8d31..da3866cd00 100644 --- a/tests/test_opaque_types.cpp +++ b/tests/test_opaque_types.cpp @@ -18,7 +18,7 @@ // This also deliberately doesn't use the below StringList type alias to test // that MAKE_OPAQUE can handle a type containing a `,`. (The `std::allocator` // bit is just the default `std::vector` allocator). -PYBIND11_MAKE_OPAQUE(std::vector>); +PYBIND11_MAKE_OPAQUE(std::vector>) using StringList = std::vector>; diff --git a/tests/test_sequences_and_iterators.cpp b/tests/test_sequences_and_iterators.cpp index 4a1d37f4de..c997b73001 100644 --- a/tests/test_sequences_and_iterators.cpp +++ b/tests/test_sequences_and_iterators.cpp @@ -86,8 +86,8 @@ class NonCopyableInt { }; using NonCopyableIntPair = std::pair; -PYBIND11_MAKE_OPAQUE(std::vector); -PYBIND11_MAKE_OPAQUE(std::vector); +PYBIND11_MAKE_OPAQUE(std::vector) +PYBIND11_MAKE_OPAQUE(std::vector) template py::list test_random_access_iterator(PythonType x) { diff --git a/tests/test_smart_ptr.cpp b/tests/test_smart_ptr.cpp index 496073b3c1..385eb4e279 100644 --- a/tests/test_smart_ptr.cpp +++ b/tests/test_smart_ptr.cpp @@ -279,13 +279,13 @@ struct holder_helper> { } // namespace PYBIND11_NAMESPACE // Make pybind aware of the ref-counted wrapper type (s): -PYBIND11_DECLARE_HOLDER_TYPE(T, ref, true); +PYBIND11_DECLARE_HOLDER_TYPE(T, ref, true) // The following is not required anymore for std::shared_ptr, but it should compile without error: -PYBIND11_DECLARE_HOLDER_TYPE(T, std::shared_ptr); -PYBIND11_DECLARE_HOLDER_TYPE(T, huge_unique_ptr); -PYBIND11_DECLARE_HOLDER_TYPE(T, custom_unique_ptr); -PYBIND11_DECLARE_HOLDER_TYPE(T, shared_ptr_with_addressof_operator); -PYBIND11_DECLARE_HOLDER_TYPE(T, unique_ptr_with_addressof_operator); +PYBIND11_DECLARE_HOLDER_TYPE(T, std::shared_ptr) +PYBIND11_DECLARE_HOLDER_TYPE(T, huge_unique_ptr) +PYBIND11_DECLARE_HOLDER_TYPE(T, custom_unique_ptr) +PYBIND11_DECLARE_HOLDER_TYPE(T, shared_ptr_with_addressof_operator) +PYBIND11_DECLARE_HOLDER_TYPE(T, unique_ptr_with_addressof_operator) TEST_SUBMODULE(smart_ptr, m) { // Please do not interleave `struct` and `class` definitions with bindings code, diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp index 6240524a09..dd93d51d0a 100644 --- a/tests/test_stl.cpp +++ b/tests/test_stl.cpp @@ -59,7 +59,7 @@ struct visit_helper { } // namespace PYBIND11_NAMESPACE #endif -PYBIND11_MAKE_OPAQUE(std::vector>); +PYBIND11_MAKE_OPAQUE(std::vector>) /// Issue #528: templated constructor struct TplCtorClass { From b0e2ac38166f563caeb8cfcbf74338d9aafae5f4 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Wed, 21 Aug 2024 13:21:55 -0400 Subject: [PATCH 2/5] tests: ignore pedantic warning for PYBIND11_DECLARE_HOLDER_TYPE Signed-off-by: Henry Schreiner --- tests/test_smart_ptr.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/test_smart_ptr.cpp b/tests/test_smart_ptr.cpp index 385eb4e279..a8e779a8bb 100644 --- a/tests/test_smart_ptr.cpp +++ b/tests/test_smart_ptr.cpp @@ -281,11 +281,15 @@ struct holder_helper> { // Make pybind aware of the ref-counted wrapper type (s): PYBIND11_DECLARE_HOLDER_TYPE(T, ref, true) // The following is not required anymore for std::shared_ptr, but it should compile without error: +PYBIND11_WARNING_PUSH +PYBIND11_WARNING_DISABLE_GCC("-Wpedantic") +PYBIND11_WARNING_DISABLE_CLANG("-Wgnu-zero-variadic-macro-arguments") PYBIND11_DECLARE_HOLDER_TYPE(T, std::shared_ptr) PYBIND11_DECLARE_HOLDER_TYPE(T, huge_unique_ptr) PYBIND11_DECLARE_HOLDER_TYPE(T, custom_unique_ptr) PYBIND11_DECLARE_HOLDER_TYPE(T, shared_ptr_with_addressof_operator) PYBIND11_DECLARE_HOLDER_TYPE(T, unique_ptr_with_addressof_operator) +PYBIND11_WARNING_POP TEST_SUBMODULE(smart_ptr, m) { // Please do not interleave `struct` and `class` definitions with bindings code, From ba93f5385075bf539206783d5199a02251291d37 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Wed, 21 Aug 2024 13:33:08 -0400 Subject: [PATCH 3/5] tests: try just turning off pedantic for one file Signed-off-by: Henry Schreiner --- include/pybind11/cast.h | 4 ---- tests/test_smart_ptr.cpp | 7 +++---- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 1e5353776c..0c862e4bec 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -868,9 +868,6 @@ struct always_construct_holder { static constexpr bool value = Value; }; -PYBIND11_WARNING_PUSH -PYBIND11_WARNING_DISABLE_GCC("-Wpedantic") -PYBIND11_WARNING_DISABLE_CLANG("-Wgnu-zero-variadic-macro-arguments") /// Create a specialization for custom holder types (silently ignores std::shared_ptr) #define PYBIND11_DECLARE_HOLDER_TYPE(type, holder_type, ...) \ PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) \ @@ -883,7 +880,6 @@ PYBIND11_WARNING_DISABLE_CLANG("-Wgnu-zero-variadic-macro-arguments") : public type_caster_holder {}; \ } \ PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE) -PYBIND11_WARNING_POP // PYBIND11_DECLARE_HOLDER_TYPE holder types: template diff --git a/tests/test_smart_ptr.cpp b/tests/test_smart_ptr.cpp index a8e779a8bb..4ab43953f1 100644 --- a/tests/test_smart_ptr.cpp +++ b/tests/test_smart_ptr.cpp @@ -11,6 +11,9 @@ #include "object.h" #include "pybind11_tests.h" +// This breaks on PYBIND11_DECLARE_HOLDER_TYPE +PYBIND11_WARNING_DISABLE_GCC("-Wpedantic") + namespace { // This is just a wrapper around unique_ptr, but with extra fields to deliberately bloat up the @@ -281,15 +284,11 @@ struct holder_helper> { // Make pybind aware of the ref-counted wrapper type (s): PYBIND11_DECLARE_HOLDER_TYPE(T, ref, true) // The following is not required anymore for std::shared_ptr, but it should compile without error: -PYBIND11_WARNING_PUSH -PYBIND11_WARNING_DISABLE_GCC("-Wpedantic") -PYBIND11_WARNING_DISABLE_CLANG("-Wgnu-zero-variadic-macro-arguments") PYBIND11_DECLARE_HOLDER_TYPE(T, std::shared_ptr) PYBIND11_DECLARE_HOLDER_TYPE(T, huge_unique_ptr) PYBIND11_DECLARE_HOLDER_TYPE(T, custom_unique_ptr) PYBIND11_DECLARE_HOLDER_TYPE(T, shared_ptr_with_addressof_operator) PYBIND11_DECLARE_HOLDER_TYPE(T, unique_ptr_with_addressof_operator) -PYBIND11_WARNING_POP TEST_SUBMODULE(smart_ptr, m) { // Please do not interleave `struct` and `class` definitions with bindings code, From b3d504207d94e84235d6f01b07d090c19ac1bf4f Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Wed, 21 Aug 2024 13:44:22 -0400 Subject: [PATCH 4/5] tests: only run pedantic in C++20 mode Signed-off-by: Henry Schreiner --- include/pybind11/detail/common.h | 1 - tests/CMakeLists.txt | 4 +++- tests/test_tagbased_polymorphic.cpp | 2 +- tests/test_virtual_functions.cpp | 2 +- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index 88ab5a29aa..2a39e88f37 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -480,7 +480,6 @@ PYBIND11_WARNING_POP \endrst */ PYBIND11_WARNING_PUSH -PYBIND11_WARNING_DISABLE_GCC("-Wpedantic") PYBIND11_WARNING_DISABLE_CLANG("-Wgnu-zero-variadic-macro-arguments") #define PYBIND11_MODULE(name, variable, ...) \ static ::pybind11::module_::module_def PYBIND11_CONCAT(pybind11_module_def_, name) \ diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index bfc2e94df9..cd94ef3e53 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -377,12 +377,14 @@ function(pybind11_enable_warnings target_name) ${target_name} PRIVATE -Wall -Wextra - -Wpedantic -Wconversion -Wcast-qual -Wdeprecated -Wundef -Wnon-virtual-dtor) + if(DEFINED CMAKE_CXX_STANDARD AND NOT CMAKE_CXX_STANDARD VERSION_LESS 20) + target_compile_options(${target_name} PRIVATE -Wpedantic) + endif() endif() if(PYBIND11_WERROR) diff --git a/tests/test_tagbased_polymorphic.cpp b/tests/test_tagbased_polymorphic.cpp index 24b49021b4..13e5ed3198 100644 --- a/tests/test_tagbased_polymorphic.cpp +++ b/tests/test_tagbased_polymorphic.cpp @@ -145,4 +145,4 @@ TEST_SUBMODULE(tagbased_polymorphic, m) { .def(py::init()) .def("purr", &Panther::purr); m.def("create_zoo", &create_zoo); -}; +} diff --git a/tests/test_virtual_functions.cpp b/tests/test_virtual_functions.cpp index 93b136ad3c..a6164eb81d 100644 --- a/tests/test_virtual_functions.cpp +++ b/tests/test_virtual_functions.cpp @@ -589,4 +589,4 @@ void initialize_inherited_virtuals(py::module_ &m) { // Fix issue #1454 (crash when acquiring/releasing GIL on another thread in Python 2.7) m.def("test_gil", &test_gil); m.def("test_gil_from_thread", &test_gil_from_thread); -}; +} From 9ecee595c84b5b98451dd02bb7d28e2562b88de7 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Wed, 21 Aug 2024 15:18:40 -0400 Subject: [PATCH 5/5] Update tests/local_bindings.h --- tests/local_bindings.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/local_bindings.h b/tests/local_bindings.h index 7e109d915d..dea1813108 100644 --- a/tests/local_bindings.h +++ b/tests/local_bindings.h @@ -60,7 +60,7 @@ PYBIND11_MAKE_OPAQUE(LocalVec) PYBIND11_MAKE_OPAQUE(LocalVec2) PYBIND11_MAKE_OPAQUE(LocalMap) PYBIND11_MAKE_OPAQUE(NonLocalVec) -// PYBIND11_MAKE_OPAQUE(NonLocalVec2) +// PYBIND11_MAKE_OPAQUE(NonLocalVec2) // same type as LocalVec2 PYBIND11_MAKE_OPAQUE(NonLocalMap) PYBIND11_MAKE_OPAQUE(NonLocalMap2)