fix: make PYBIND11_WARNING_POP actually pop clang diagnostics (#5448)

* fix: make PYBIND11_WARNING_POP actually pop clang diagnostics * fix: ignore -Wgnu-zero-variadic-macro-arguments on clang * Revert "fix: ignore -Wgnu-zero-variadic-macro-arguments on clang" This reverts commit d310959. * C++20 modernization: Use `__VA_OPT__(, ) __VA_ARGS__` in `PYBIND11_DECLARE_HOLDER_TYPE()` * Disable `__VA_OPT__(, ) __VA_ARGS__` usage for MSVC (it is unclear to rwgk why it does not work). This is the beginning of the error message: ``` D:\a\pybind11\pybind11\tests\test_smart_ptr.cpp(285,1): error C2143: syntax error: missing ')' before ',' [D:\a\pybind11\pybind11\build\tests\pybind11_tests.vcxproj] D:\a\pybind11\pybind11\tests\test_smart_ptr.cpp(285,1): error C2059: syntax error: ')' [D:\a\pybind11\pybind11\build\tests\pybind11_tests.vcxproj] ``` * Add `PYBIND11_WARNING_DISABLE_CLANG("-Wgnu-zero-variadic-macro-arguments")` in test_smart_ptr.cpp This is the error message: ``` /__w/pybind11/pybind11/tests/test_smart_ptr.cpp:287:51: error: must specify at least one argument for '...' parameter of variadic macro [-Werror,-Wgnu-zero-variadic-macro-arguments] PYBIND11_DECLARE_HOLDER_TYPE(T, std::shared_ptr<T>) ^ /__w/pybind11/pybind11/include/pybind11/cast.h:885:13: note: macro 'PYBIND11_DECLARE_HOLDER_TYPE' defined here ^ ``` * Also add `PYBIND11_WARNING_DISABLE_CLANG("-Wgnu-zero-variadic-macro-arguments")` in test_virtual_functions.cpp * Also add `PYBIND11_WARNING_DISABLE_CLANG("-Wgnu-zero-variadic-macro-arguments")` in test_embed/test_interpreter.cpp * Undo all changes except the original push -> pop fix. * 1. Add `PYBIND11_WARNING_DISABLE_CLANG("-Wgnu-zero-variadic-macro-arguments")` near the top of pybind11/pybind11.h; 2. Change `PYBIND11_DECLARE_HOLDER_TYPE` macro to side-step the only remaining clang warning-as-error (this is still needed even for clang 18). Alternatively the warning suppression could be moved into pybind11/cast.h, but this commit limits the warning suppression to smaller scope within include/pybind11. --------- Co-authored-by: Ralf W. Grosse-Kunstleve <[email protected]>
pybind · Dec 8, 2024 · a6d1ff2 · a6d1ff2
1 parent e7c9b90
commit a6d1ff2
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 5 deletions.
diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h
@@ -863,18 +863,20 @@ using type_caster_holder = conditional_t<is_copy_constructible<holder_type>::val
                                          copyable_holder_caster<type, holder_type>,
                                          move_only_holder_caster<type, holder_type>>;
 
-template <typename T, bool Value = false>
-struct always_construct_holder {
+template <bool Value = false>
+struct always_construct_holder_value {
     static constexpr bool value = Value;
 };
 
+template <typename T, bool Value = false>
+struct always_construct_holder : always_construct_holder_value<Value> {};
+
 /// 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)                                                  \
     namespace detail {                                                                            \
     template <typename type>                                                                      \
-    struct always_construct_holder<holder_type> : always_construct_holder<void, ##__VA_ARGS__> {  \
-    };                                                                                            \
+    struct always_construct_holder<holder_type> : always_construct_holder_value<__VA_ARGS__> {};  \
     template <typename type>                                                                      \
     class type_caster<holder_type, enable_if_t<!is_shared_ptr<holder_type>::value>>               \
         : public type_caster_holder<type, holder_type> {};                                        \

diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h
@@ -46,7 +46,7 @@
 #    define PYBIND11_COMPILER_CLANG
 #    define PYBIND11_PRAGMA(...) _Pragma(#__VA_ARGS__)
 #    define PYBIND11_WARNING_PUSH PYBIND11_PRAGMA(clang diagnostic push)
-#    define PYBIND11_WARNING_POP PYBIND11_PRAGMA(clang diagnostic push)
+#    define PYBIND11_WARNING_POP PYBIND11_PRAGMA(clang diagnostic pop)
 #elif defined(__GNUC__)
 #    define PYBIND11_COMPILER_GCC
 #    define PYBIND11_PRAGMA(...) _Pragma(#__VA_ARGS__)

diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h
@@ -26,6 +26,12 @@
 #include <utility>
 #include <vector>
 
+// See PR #5448. This warning suppression is needed for the PYBIND11_OVERRIDE macro family.
+// NOTE that this is NOT embedded in a push/pop pair because that is very difficult to achieve.
+#if defined(__clang_major__) && __clang_major__ < 14
+PYBIND11_WARNING_DISABLE_CLANG("-Wgnu-zero-variadic-macro-arguments")
+#endif
+
 #if defined(__cpp_lib_launder) && !(defined(_MSC_VER) && (_MSC_VER < 1914))
 #    define PYBIND11_STD_LAUNDER std::launder
 #    define PYBIND11_HAS_STD_LAUNDER 1