From 774b5ff90bbc2fac198f03a246e3517a53f8c637 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Tue, 17 Aug 2021 16:49:39 -0700 Subject: [PATCH 1/2] Removing obsolete eigen.h warning suppression pragmas. (#3198) * Removing all pragma from eigen.h * Removing -Werror or equivalent from tests/CMakeLists.txt * Restoring tests/CMakeLists.txt from master. * Adding 4 PYBIND11_SILENCE_MSVC_C4127. * Compatibility with -Wconversion, -Wdeprecated * Introducing PYBIND11_COMPATIBILITY_WDEPRECATED_COPY * Systematically using --verbose for compilations where possible (cmake 3.14 or newer). Also changing all `cmake -t` to `--target`, `-v` to `--verbose`, `check` to `pytest`, for consistency (to make it easier to pin-point all commands of a certain type). Also removing one `-j 2` for `pytest` in hopes of reducing flakes due to races in test_iostream and in prints from destructors. * Commenting out pragmas as an experiment to reproduce previous observation. * Removing all (newly added, but already commented-out) pragma code, adding HINT use -isystem (as cmake does). * Restoring ci.yml from master. Those changes are better handled separately. BTW: in the last CI run there was still a test_iostream flake, even without the -j 2 for running the tests (verfied by inspecting the log). --- include/pybind11/eigen.h | 41 ++++++++++------------------------------ 1 file changed, 10 insertions(+), 31 deletions(-) diff --git a/include/pybind11/eigen.h b/include/pybind11/eigen.h index 218fe2703..b03e15f62 100644 --- a/include/pybind11/eigen.h +++ b/include/pybind11/eigen.h @@ -9,30 +9,14 @@ #pragma once +/* HINT: To suppress warnings originating from the Eigen headers, use -isystem. + See also: + https://stackoverflow.com/questions/2579576/i-dir-vs-isystem-dir + https://stackoverflow.com/questions/1741816/isystem-for-ms-visual-studio-c-compiler +*/ + #include "numpy.h" -#if defined(__INTEL_COMPILER) -# pragma warning(disable: 1682) // implicit conversion of a 64-bit integral type to a smaller integral type (potential portability problem) -#elif defined(__GNUG__) || defined(__clang__) -# pragma GCC diagnostic push -# pragma GCC diagnostic ignored "-Wconversion" -# pragma GCC diagnostic ignored "-Wdeprecated-declarations" -# ifdef __clang__ -// Eigen generates a bunch of implicit-copy-constructor-is-deprecated warnings with -Wdeprecated -// under Clang, so disable that warning here: -# pragma GCC diagnostic ignored "-Wdeprecated" -# endif -# if __GNUC__ >= 7 -# pragma GCC diagnostic ignored "-Wint-in-bool-context" -# endif -#endif - -#if defined(_MSC_VER) -# pragma warning(push) -# pragma warning(disable: 4127) // warning C4127: Conditional expression is constant -# pragma warning(disable: 4996) // warning C4996: std::unary_negate is deprecated in C++17 -#endif - #include #include @@ -153,7 +137,8 @@ template struct EigenProps { np_cols = a.shape(1), np_rstride = a.strides(0) / static_cast(sizeof(Scalar)), np_cstride = a.strides(1) / static_cast(sizeof(Scalar)); - if ((fixed_rows && np_rows != rows) || (fixed_cols && np_cols != cols)) + if ((PYBIND11_SILENCE_MSVC_C4127(fixed_rows) && np_rows != rows) || + (PYBIND11_SILENCE_MSVC_C4127(fixed_cols) && np_cols != cols)) return false; return {np_rows, np_cols, np_rstride, np_cstride}; @@ -165,7 +150,7 @@ template struct EigenProps { stride = a.strides(0) / static_cast(sizeof(Scalar)); if (vector) { // Eigen type is a compile-time vector - if (fixed && size != n) + if (PYBIND11_SILENCE_MSVC_C4127(fixed) && size != n) return false; // Vector size mismatch return {rows == 1 ? 1 : n, cols == 1 ? 1 : n, stride}; } @@ -179,7 +164,7 @@ template struct EigenProps { if (cols != n) return false; return {1, n, stride}; } // Otherwise it's either fully dynamic, or column dynamic; both become a column vector - if (fixed_rows && rows != n) return false; + if (PYBIND11_SILENCE_MSVC_C4127(fixed_rows) && rows != n) return false; return {n, 1, stride}; } @@ -596,9 +581,3 @@ struct type_caster::value>> { PYBIND11_NAMESPACE_END(detail) PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE) - -#if defined(__GNUG__) || defined(__clang__) -# pragma GCC diagnostic pop -#elif defined(_MSC_VER) -# pragma warning(pop) -#endif From 998d45e4313067d752476338d090f388f0a5d9ed Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Thu, 19 Aug 2021 11:37:04 -0700 Subject: [PATCH 2/2] Cleanup of file-scoped and globally-scoped warning suppression pragmas across pybind11 header files. (#3201) * Removing all MSVC C4127 warning suppression pragmas. * Removing MSVC /WX (WERROR). To get a full list of all warnings. * Inserting PYBIND11_SILENCE_MSVC_C4127. Changing one runtime if to #if. * Changing PYBIND11_SILENCE_MSVC_C4127 macro to use absolute namespace (for use outside pybind11 include directory). * Restoring MSVC /WX (WERROR). * Removing globally-scoped suppression for clang -Wunsequenced. Based on an experiment under PR #3202 it is obsolete and can simply be removed. --- include/pybind11/detail/common.h | 2 +- include/pybind11/numpy.h | 13 ++----------- include/pybind11/operators.h | 14 ++------------ include/pybind11/stl.h | 9 --------- tests/test_builtin_casters.cpp | 14 +++++--------- 5 files changed, 10 insertions(+), 42 deletions(-) diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index 0d12d734c..cb52aa274 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -968,7 +968,7 @@ inline void silence_unused_warnings(Args &&...) {} // warning C4127: Conditional expression is constant constexpr inline bool silence_msvc_c4127(bool cond) { return cond; } -# define PYBIND11_SILENCE_MSVC_C4127(...) detail::silence_msvc_c4127(__VA_ARGS__) +# define PYBIND11_SILENCE_MSVC_C4127(...) ::pybind11::detail::silence_msvc_c4127(__VA_ARGS__) #else # define PYBIND11_SILENCE_MSVC_C4127(...) __VA_ARGS__ diff --git a/include/pybind11/numpy.h b/include/pybind11/numpy.h index 2dc0f29f0..7717059f4 100644 --- a/include/pybind11/numpy.h +++ b/include/pybind11/numpy.h @@ -25,11 +25,6 @@ #include #include -#if defined(_MSC_VER) -# pragma warning(push) -# pragma warning(disable: 4127) // warning C4127: Conditional expression is constant -#endif - /* This will be true on all flat address space platforms and allows us to reduce the whole npy_intp / ssize_t / Py_intptr_t business down to just ssize_t for all size and dimension types (e.g. shape, strides, indexing), instead of inflicting this @@ -747,7 +742,7 @@ public: * and the caller must take care not to access invalid dimensions or dimension indices. */ template detail::unchecked_mutable_reference mutable_unchecked() & { - if (Dims >= 0 && ndim() != Dims) + if (PYBIND11_SILENCE_MSVC_C4127(Dims >= 0) && ndim() != Dims) throw std::domain_error("array has incorrect number of dimensions: " + std::to_string(ndim()) + "; expected " + std::to_string(Dims)); return detail::unchecked_mutable_reference(mutable_data(), shape(), strides(), ndim()); @@ -761,7 +756,7 @@ public: * invalid dimensions or dimension indices. */ template detail::unchecked_reference unchecked() const & { - if (Dims >= 0 && ndim() != Dims) + if (PYBIND11_SILENCE_MSVC_C4127(Dims >= 0) && ndim() != Dims) throw std::domain_error("array has incorrect number of dimensions: " + std::to_string(ndim()) + "; expected " + std::to_string(Dims)); return detail::unchecked_reference(data(), shape(), strides(), ndim()); @@ -1708,7 +1703,3 @@ Helper vectorize(Return (Class::*f)(Args...) const) { } PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE) - -#if defined(_MSC_VER) -#pragma warning(pop) -#endif diff --git a/include/pybind11/operators.h b/include/pybind11/operators.h index 086cb4cfd..2a6153158 100644 --- a/include/pybind11/operators.h +++ b/include/pybind11/operators.h @@ -11,13 +11,6 @@ #include "pybind11.h" -#if defined(__clang__) && !defined(__INTEL_COMPILER) -# pragma clang diagnostic ignored "-Wunsequenced" // multiple unsequenced modifications to 'self' (when using def(py::self OP Type())) -#elif defined(_MSC_VER) -# pragma warning(push) -# pragma warning(disable: 4127) // warning C4127: Conditional expression is constant -#endif - PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) PYBIND11_NAMESPACE_BEGIN(detail) @@ -58,7 +51,8 @@ template struct op_ { using op = op_impl; cl.def(op::name(), &op::execute, is_operator(), extra...); #if PY_MAJOR_VERSION < 3 - if (id == op_truediv || id == op_itruediv) + if (PYBIND11_SILENCE_MSVC_C4127(id == op_truediv) || + PYBIND11_SILENCE_MSVC_C4127(id == op_itruediv)) cl.def(id == op_itruediv ? "__idiv__" : ot == op_l ? "__div__" : "__rdiv__", &op::execute, is_operator(), extra...); #endif @@ -167,7 +161,3 @@ using detail::self; using detail::hash; PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE) - -#if defined(_MSC_VER) -# pragma warning(pop) -#endif diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index 51994c656..fe391f70b 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -19,11 +19,6 @@ #include #include -#if defined(_MSC_VER) -#pragma warning(push) -#pragma warning(disable: 4127) // warning C4127: Conditional expression is constant -#endif - #ifdef __has_include // std::optional (but including it in c++14 mode isn't allowed) # if defined(PYBIND11_CPP17) && __has_include() @@ -390,7 +385,3 @@ inline std::ostream &operator<<(std::ostream &os, const handle &obj) { } PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE) - -#if defined(_MSC_VER) -#pragma warning(pop) -#endif diff --git a/tests/test_builtin_casters.cpp b/tests/test_builtin_casters.cpp index 51e410cad..be4431aad 100644 --- a/tests/test_builtin_casters.cpp +++ b/tests/test_builtin_casters.cpp @@ -10,11 +10,6 @@ #include "pybind11_tests.h" #include -#if defined(_MSC_VER) -# pragma warning(push) -# pragma warning(disable: 4127) // warning C4127: Conditional expression is constant -#endif - struct ConstRefCasted { int tag; }; @@ -73,7 +68,7 @@ TEST_SUBMODULE(builtin_casters, m) { std::wstring wstr; wstr.push_back(0x61); // a wstr.push_back(0x2e18); // ⸘ - if (sizeof(wchar_t) == 2) { wstr.push_back(mathbfA16_1); wstr.push_back(mathbfA16_2); } // 𝐀, utf16 + if (PYBIND11_SILENCE_MSVC_C4127(sizeof(wchar_t) == 2)) { wstr.push_back(mathbfA16_1); wstr.push_back(mathbfA16_2); } // 𝐀, utf16 else { wstr.push_back((wchar_t) mathbfA32); } // 𝐀, utf32 wstr.push_back(0x7a); // z @@ -83,11 +78,12 @@ TEST_SUBMODULE(builtin_casters, m) { m.def("good_wchar_string", [=]() { return wstr; }); // a‽𝐀z m.def("bad_utf8_string", []() { return std::string("abc\xd0" "def"); }); m.def("bad_utf16_string", [=]() { return std::u16string({ b16, char16_t(0xd800), z16 }); }); +#if PY_MAJOR_VERSION >= 3 // Under Python 2.7, invalid unicode UTF-32 characters don't appear to trigger UnicodeDecodeError - if (PY_MAJOR_VERSION >= 3) - m.def("bad_utf32_string", [=]() { return std::u32string({ a32, char32_t(0xd800), z32 }); }); - if (PY_MAJOR_VERSION >= 3 || sizeof(wchar_t) == 2) + m.def("bad_utf32_string", [=]() { return std::u32string({ a32, char32_t(0xd800), z32 }); }); + if (PYBIND11_SILENCE_MSVC_C4127(sizeof(wchar_t) == 2)) m.def("bad_wchar_string", [=]() { return std::wstring({ wchar_t(0x61), wchar_t(0xd800) }); }); +#endif m.def("u8_Z", []() -> char { return 'Z'; }); m.def("u8_eacute", []() -> char { return '\xe9'; }); m.def("u16_ibang", [=]() -> char16_t { return ib16; });