From d84c495037632d6594cfe555ba47590934743618 Mon Sep 17 00:00:00 2001 From: James Morris Date: Sat, 23 Nov 2024 20:12:33 -0500 Subject: [PATCH 01/10] fix: make PYBIND11_WARNING_POP actually pop clang diagnostics --- include/pybind11/detail/common.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index 310c3c3947..78173cad30 100644 --- 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__) From d310959bf5e6194033aa1825ab181a76289d790f Mon Sep 17 00:00:00 2001 From: James Morris Date: Thu, 28 Nov 2024 16:01:46 -0500 Subject: [PATCH 02/10] fix: ignore -Wgnu-zero-variadic-macro-arguments on clang --- tests/CMakeLists.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 01b6c0a3e6..779adbc9cf 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -389,6 +389,9 @@ function(pybind11_enable_warnings target_name) if(DEFINED CMAKE_CXX_STANDARD AND NOT CMAKE_CXX_STANDARD VERSION_LESS 20) target_compile_options(${target_name} PRIVATE -Wpedantic) endif() + if(CMAKE_CXX_COMPILER_ID MATCHES "Clang") + target_compile_options(${target_name} PRIVATE -Wno-gnu-zero-variadic-macro-arguments) + endif() endif() if(PYBIND11_WERROR) From 1a04f75395b5cbb928dee03c664a65cfe4732abd Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Fri, 29 Nov 2024 12:35:19 -0800 Subject: [PATCH 03/10] Revert "fix: ignore -Wgnu-zero-variadic-macro-arguments on clang" This reverts commit d310959bf5e6194033aa1825ab181a76289d790f. --- tests/CMakeLists.txt | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 779adbc9cf..01b6c0a3e6 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -389,9 +389,6 @@ function(pybind11_enable_warnings target_name) if(DEFINED CMAKE_CXX_STANDARD AND NOT CMAKE_CXX_STANDARD VERSION_LESS 20) target_compile_options(${target_name} PRIVATE -Wpedantic) endif() - if(CMAKE_CXX_COMPILER_ID MATCHES "Clang") - target_compile_options(${target_name} PRIVATE -Wno-gnu-zero-variadic-macro-arguments) - endif() endif() if(PYBIND11_WERROR) From 98b035a6a919325eaeda58862b92da1a7282cfb1 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Fri, 29 Nov 2024 14:44:14 -0800 Subject: [PATCH 04/10] C++20 modernization: Use `__VA_OPT__(, ) __VA_ARGS__` in `PYBIND11_DECLARE_HOLDER_TYPE()` --- include/pybind11/cast.h | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 2ae25c2ebf..a82f3d8230 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -869,17 +869,27 @@ struct always_construct_holder { }; /// Create a specialization for custom holder types (silently ignores std::shared_ptr) -#define PYBIND11_DECLARE_HOLDER_TYPE(type, holder_type, ...) \ +#define PYBIND11_DECLARE_HOLDER_TYPE_PART1(type, ...) \ PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) \ namespace detail { \ template \ - struct always_construct_holder : always_construct_holder { \ - }; \ + struct always_construct_holder<__VA_ARGS__> : always_construct_holder < +#define PYBIND11_DECLARE_HOLDER_TYPE_PART2(type, ...) \ + >{}; \ template \ - class type_caster::value>> \ - : public type_caster_holder {}; \ + class type_caster<__VA_ARGS__, enable_if_t::value>> \ + : public type_caster_holder {}; \ } \ PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE) +#if defined(PYBIND11_CPP20) +# define PYBIND11_DECLARE_HOLDER_TYPE(type, holder_type, ...) \ + PYBIND11_DECLARE_HOLDER_TYPE_PART1(type, holder_type) \ + void __VA_OPT__(, ) __VA_ARGS__ PYBIND11_DECLARE_HOLDER_TYPE_PART2(type, holder_type) +#else +# define PYBIND11_DECLARE_HOLDER_TYPE(type, holder_type, ...) \ + PYBIND11_DECLARE_HOLDER_TYPE_PART1(type, holder_type) \ + void, ##__VA_ARGS__ PYBIND11_DECLARE_HOLDER_TYPE_PART2(type, holder_type) +#endif // PYBIND11_DECLARE_HOLDER_TYPE holder types: template From 5c5dc8babf4c195728c5dd5ed30abda953ed95ba Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Fri, 29 Nov 2024 16:16:23 -0800 Subject: [PATCH 05/10] 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] ``` --- include/pybind11/cast.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index a82f3d8230..29cc6289d4 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -881,7 +881,7 @@ struct always_construct_holder { : public type_caster_holder {}; \ } \ PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE) -#if defined(PYBIND11_CPP20) +#if defined(PYBIND11_CPP20) && !defined(_MSC_VER) # define PYBIND11_DECLARE_HOLDER_TYPE(type, holder_type, ...) \ PYBIND11_DECLARE_HOLDER_TYPE_PART1(type, holder_type) \ void __VA_OPT__(, ) __VA_ARGS__ PYBIND11_DECLARE_HOLDER_TYPE_PART2(type, holder_type) From a575ad6f94d2c20ccb333abb99b75efba598214e Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Fri, 29 Nov 2024 16:19:36 -0800 Subject: [PATCH 06/10] 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) ^ /__w/pybind11/pybind11/include/pybind11/cast.h:885:13: note: macro 'PYBIND11_DECLARE_HOLDER_TYPE' defined here ^ ``` --- tests/test_smart_ptr.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_smart_ptr.cpp b/tests/test_smart_ptr.cpp index 4ab43953f1..87595177c8 100644 --- a/tests/test_smart_ptr.cpp +++ b/tests/test_smart_ptr.cpp @@ -11,8 +11,9 @@ #include "object.h" #include "pybind11_tests.h" -// This breaks on PYBIND11_DECLARE_HOLDER_TYPE +// These break on PYBIND11_DECLARE_HOLDER_TYPE PYBIND11_WARNING_DISABLE_GCC("-Wpedantic") +PYBIND11_WARNING_DISABLE_CLANG("-Wgnu-zero-variadic-macro-arguments") namespace { From be2c4cd1f4029378c0fef510755dee9c0e505a17 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Fri, 29 Nov 2024 16:40:42 -0800 Subject: [PATCH 07/10] Also add `PYBIND11_WARNING_DISABLE_CLANG("-Wgnu-zero-variadic-macro-arguments")` in test_virtual_functions.cpp --- tests/test_smart_ptr.cpp | 2 ++ tests/test_virtual_functions.cpp | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/tests/test_smart_ptr.cpp b/tests/test_smart_ptr.cpp index 87595177c8..824f463590 100644 --- a/tests/test_smart_ptr.cpp +++ b/tests/test_smart_ptr.cpp @@ -13,7 +13,9 @@ // These break on PYBIND11_DECLARE_HOLDER_TYPE PYBIND11_WARNING_DISABLE_GCC("-Wpedantic") +#if defined(__clang_major__) && __clang_major__ < 14 PYBIND11_WARNING_DISABLE_CLANG("-Wgnu-zero-variadic-macro-arguments") +#endif namespace { diff --git a/tests/test_virtual_functions.cpp b/tests/test_virtual_functions.cpp index a6164eb81d..40ff9027f2 100644 --- a/tests/test_virtual_functions.cpp +++ b/tests/test_virtual_functions.cpp @@ -14,6 +14,10 @@ #include +#if defined(__clang_major__) && __clang_major__ < 14 +PYBIND11_WARNING_DISABLE_CLANG("-Wgnu-zero-variadic-macro-arguments") +#endif + /* This is an example class that we'll want to be able to extend from Python */ class ExampleVirt { public: From 07859cab44113b89b7b741feadb247e84bb820eb Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Fri, 29 Nov 2024 17:30:09 -0800 Subject: [PATCH 08/10] Also add `PYBIND11_WARNING_DISABLE_CLANG("-Wgnu-zero-variadic-macro-arguments")` in test_embed/test_interpreter.cpp --- tests/test_embed/test_interpreter.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/test_embed/test_interpreter.cpp b/tests/test_embed/test_interpreter.cpp index c6c8a22d98..b5e1ed3aae 100644 --- a/tests/test_embed/test_interpreter.cpp +++ b/tests/test_embed/test_interpreter.cpp @@ -4,6 +4,10 @@ // catch 2.0.1; this should be fixed in the next catch release after 2.0.1). PYBIND11_WARNING_DISABLE_MSVC(4996) +#if defined(__clang_major__) && __clang_major__ < 14 +PYBIND11_WARNING_DISABLE_CLANG("-Wgnu-zero-variadic-macro-arguments") +#endif + #include #include #include From d01e3ef76a0102d2412d19bed795ddf7734d5b09 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Fri, 29 Nov 2024 23:57:47 -0800 Subject: [PATCH 09/10] Undo all changes except the original push -> pop fix. --- include/pybind11/cast.h | 20 +++++--------------- tests/test_embed/test_interpreter.cpp | 4 ---- tests/test_smart_ptr.cpp | 5 +---- tests/test_virtual_functions.cpp | 4 ---- 4 files changed, 6 insertions(+), 27 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 29cc6289d4..2ae25c2ebf 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -869,27 +869,17 @@ struct always_construct_holder { }; /// Create a specialization for custom holder types (silently ignores std::shared_ptr) -#define PYBIND11_DECLARE_HOLDER_TYPE_PART1(type, ...) \ +#define PYBIND11_DECLARE_HOLDER_TYPE(type, holder_type, ...) \ PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) \ namespace detail { \ template \ - struct always_construct_holder<__VA_ARGS__> : always_construct_holder < -#define PYBIND11_DECLARE_HOLDER_TYPE_PART2(type, ...) \ - >{}; \ + struct always_construct_holder : always_construct_holder { \ + }; \ template \ - class type_caster<__VA_ARGS__, enable_if_t::value>> \ - : public type_caster_holder {}; \ + class type_caster::value>> \ + : public type_caster_holder {}; \ } \ PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE) -#if defined(PYBIND11_CPP20) && !defined(_MSC_VER) -# define PYBIND11_DECLARE_HOLDER_TYPE(type, holder_type, ...) \ - PYBIND11_DECLARE_HOLDER_TYPE_PART1(type, holder_type) \ - void __VA_OPT__(, ) __VA_ARGS__ PYBIND11_DECLARE_HOLDER_TYPE_PART2(type, holder_type) -#else -# define PYBIND11_DECLARE_HOLDER_TYPE(type, holder_type, ...) \ - PYBIND11_DECLARE_HOLDER_TYPE_PART1(type, holder_type) \ - void, ##__VA_ARGS__ PYBIND11_DECLARE_HOLDER_TYPE_PART2(type, holder_type) -#endif // PYBIND11_DECLARE_HOLDER_TYPE holder types: template diff --git a/tests/test_embed/test_interpreter.cpp b/tests/test_embed/test_interpreter.cpp index b5e1ed3aae..c6c8a22d98 100644 --- a/tests/test_embed/test_interpreter.cpp +++ b/tests/test_embed/test_interpreter.cpp @@ -4,10 +4,6 @@ // catch 2.0.1; this should be fixed in the next catch release after 2.0.1). PYBIND11_WARNING_DISABLE_MSVC(4996) -#if defined(__clang_major__) && __clang_major__ < 14 -PYBIND11_WARNING_DISABLE_CLANG("-Wgnu-zero-variadic-macro-arguments") -#endif - #include #include #include diff --git a/tests/test_smart_ptr.cpp b/tests/test_smart_ptr.cpp index 824f463590..4ab43953f1 100644 --- a/tests/test_smart_ptr.cpp +++ b/tests/test_smart_ptr.cpp @@ -11,11 +11,8 @@ #include "object.h" #include "pybind11_tests.h" -// These break on PYBIND11_DECLARE_HOLDER_TYPE +// This breaks on PYBIND11_DECLARE_HOLDER_TYPE PYBIND11_WARNING_DISABLE_GCC("-Wpedantic") -#if defined(__clang_major__) && __clang_major__ < 14 -PYBIND11_WARNING_DISABLE_CLANG("-Wgnu-zero-variadic-macro-arguments") -#endif namespace { diff --git a/tests/test_virtual_functions.cpp b/tests/test_virtual_functions.cpp index 40ff9027f2..a6164eb81d 100644 --- a/tests/test_virtual_functions.cpp +++ b/tests/test_virtual_functions.cpp @@ -14,10 +14,6 @@ #include -#if defined(__clang_major__) && __clang_major__ < 14 -PYBIND11_WARNING_DISABLE_CLANG("-Wgnu-zero-variadic-macro-arguments") -#endif - /* This is an example class that we'll want to be able to extend from Python */ class ExampleVirt { public: From c9ce487d6f24c4c1ebd42ee29f624b0220f35e35 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Fri, 29 Nov 2024 23:44:53 -0800 Subject: [PATCH 10/10] 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. --- include/pybind11/cast.h | 10 ++++++---- include/pybind11/pybind11.h | 6 ++++++ 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 2ae25c2ebf..7ae9fa037f 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -863,18 +863,20 @@ using type_caster_holder = conditional_t::val copyable_holder_caster, move_only_holder_caster>; -template -struct always_construct_holder { +template +struct always_construct_holder_value { static constexpr bool value = Value; }; +template +struct always_construct_holder : always_construct_holder_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 \ - struct always_construct_holder : always_construct_holder { \ - }; \ + struct always_construct_holder : always_construct_holder_value<__VA_ARGS__> {}; \ template \ class type_caster::value>> \ : public type_caster_holder {}; \ diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index fe8384e57f..721808701f 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -26,6 +26,12 @@ #include #include +// 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