Remove all uses of PYBIND11_SILENCE_MSVC_C4127 and the macro definition, too.

Author: rwgk

include/pybind11/cast.h

@@ -29,10 +29,11 @@
 #include <vector>
 
 PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
-PYBIND11_NAMESPACE_BEGIN(detail)
 
 PYBIND11_DISABLE_WARNING_MSVC(4127)
 
+PYBIND11_NAMESPACE_BEGIN(detail)
+
 template <typename type, typename SFINAE = void>
 class type_caster : public type_caster_base<type> {};
 template <typename type>
@@ -418,7 +419,7 @@ struct string_caster {
             = reinterpret_cast<const CharT *>(PYBIND11_BYTES_AS_STRING(utfNbytes.ptr()));
         size_t length = (size_t) PYBIND11_BYTES_SIZE(utfNbytes.ptr()) / sizeof(CharT);
         // Skip BOM for UTF-16/32
-        if (PYBIND11_SILENCE_MSVC_C4127(UTF_N > 8)) {
+        if (UTF_N > 8) {
             buffer++;
             length--;
         }
@@ -574,7 +575,7 @@ struct type_caster<CharT, enable_if_t<is_std_char_type<CharT>::value>> {
         // figure out how long the first encoded character is in bytes to distinguish between these
         // two errors.  We also allow want to allow unicode characters U+0080 through U+00FF, as
         // those can fit into a single char value.
-        if (PYBIND11_SILENCE_MSVC_C4127(StringCaster::UTF_N == 8) && str_len > 1 && str_len <= 4) {
+        if (StringCaster::UTF_N == 8 && str_len > 1 && str_len <= 4) {
             auto v0 = static_cast<unsigned char>(value[0]);
             // low bits only: 0-127
             // 0b110xxxxx - start of 2-byte sequence
@@ -600,7 +601,7 @@ struct type_caster<CharT, enable_if_t<is_std_char_type<CharT>::value>> {
         // UTF-16 is much easier: we can only have a surrogate pair for values above U+FFFF, thus a
         // surrogate pair with total length 2 instantly indicates a range error (but not a "your
         // string was too long" error).
-        else if (PYBIND11_SILENCE_MSVC_C4127(StringCaster::UTF_N == 16) && str_len == 2) {
+        else if (StringCaster::UTF_N == 16 && str_len == 2) {
             one_char = static_cast<CharT>(value[0]);
             if (one_char >= 0xD800 && one_char < 0xE000) {
                 throw value_error("Character code point not in range(0x10000)");

include/pybind11/detail/common.h

@@ -1187,17 +1187,6 @@ constexpr
 #    define PYBIND11_WORKAROUND_INCORRECT_GCC_UNUSED_BUT_SET_PARAMETER(...)
 #endif
 
-#if defined(_MSC_VER) // All versions (as of July 2021).
-
-// warning C4127: Conditional expression is constant
-constexpr inline bool silence_msvc_c4127(bool cond) { return cond; }
-
-#    define PYBIND11_SILENCE_MSVC_C4127(...) ::pybind11::detail::silence_msvc_c4127(__VA_ARGS__)
-
-#else
-#    define PYBIND11_SILENCE_MSVC_C4127(...) __VA_ARGS__
-#endif
-
 #if defined(__clang__)                                                                            \
     && (defined(__apple_build_version__) /* AppleClang 13.0.0.13000029 was the only data point    \
                                             available. */                                         \

include/pybind11/detail/init.h

@@ -12,6 +12,9 @@
 #include "class.h"
 
 PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
+
+PYBIND11_DISABLE_WARNING_MSVC(4127)
+
 PYBIND11_NAMESPACE_BEGIN(detail)
 
 template <>
@@ -115,7 +118,7 @@ template <typename Class>
 void construct(value_and_holder &v_h, Cpp<Class> *ptr, bool need_alias) {
     PYBIND11_WORKAROUND_INCORRECT_MSVC_C4100(need_alias);
     no_nullptr(ptr);
-    if (PYBIND11_SILENCE_MSVC_C4127(Class::has_alias) && need_alias && !is_alias<Class>(ptr)) {
+    if (Class::has_alias && need_alias && !is_alias<Class>(ptr)) {
         // We're going to try to construct an alias by moving the cpp type.  Whether or not
         // that succeeds, we still need to destroy the original cpp pointer (either the
         // moved away leftover, if the alias construction works, or the value itself if we
@@ -156,7 +159,7 @@ void construct(value_and_holder &v_h, Holder<Class> holder, bool need_alias) {
     auto *ptr = holder_helper<Holder<Class>>::get(holder);
     no_nullptr(ptr);
     // If we need an alias, check that the held pointer is actually an alias instance
-    if (PYBIND11_SILENCE_MSVC_C4127(Class::has_alias) && need_alias && !is_alias<Class>(ptr)) {
+    if (Class::has_alias && need_alias && !is_alias<Class>(ptr)) {
         throw type_error("pybind11::init(): construction failed: returned holder-wrapped instance "
                          "is not an alias instance");
     }
@@ -174,7 +177,7 @@ void construct(value_and_holder &v_h, Cpp<Class> &&result, bool need_alias) {
     PYBIND11_WORKAROUND_INCORRECT_MSVC_C4100(need_alias);
     static_assert(std::is_move_constructible<Cpp<Class>>::value,
                   "pybind11::init() return-by-value factory function requires a movable class");
-    if (PYBIND11_SILENCE_MSVC_C4127(Class::has_alias) && need_alias) {
+    if (Class::has_alias && need_alias) {
         construct_alias_from_cpp<Class>(is_alias_constructible<Class>{}, v_h, std::move(result));
     } else {
         v_h.value_ptr() = new Cpp<Class>(std::move(result));

include/pybind11/eigen/matrix.h

@@ -16,14 +16,8 @@
        https://stackoverflow.com/questions/2579576/i-dir-vs-isystem-dir
        https://stackoverflow.com/questions/1741816/isystem-for-ms-visual-studio-c-compiler
 */
-// The C4127 suppression was introduced for Eigen 3.4.0. In theory we could
-// make it version specific, or even remove it later, but considering that
-// 1. C4127 is generally far more distracting than useful for modern template code, and
-// 2. we definitely want to ignore any MSVC warnings originating from Eigen code,
-//    it is probably best to keep this around indefinitely.
 #if defined(_MSC_VER)
 #    pragma warning(push)
-#    pragma warning(disable : 4127) // C4127: conditional expression is constant
 #    pragma warning(disable : 5054) // https://github.com/pybind/pybind11/pull/3741
 //       C5054: operator '&': deprecated between enumerations of different types
 #elif defined(__MINGW32__)
@@ -48,6 +42,8 @@ static_assert(EIGEN_VERSION_AT_LEAST(3, 2, 7),
 
 PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
 
+PYBIND11_DISABLE_WARNING_MSVC(4127)
+
 // Provide a convenience alias for easier pass-by-ref usage with fully dynamic strides:
 using EigenDStride = Eigen::Stride<Eigen::Dynamic, Eigen::Dynamic>;
 template <typename MatrixType>
@@ -189,8 +185,7 @@ struct EigenProps {
             EigenIndex np_rows = a.shape(0), np_cols = a.shape(1),
                        np_rstride = a.strides(0) / static_cast<ssize_t>(sizeof(Scalar)),
                        np_cstride = a.strides(1) / static_cast<ssize_t>(sizeof(Scalar));
-            if ((PYBIND11_SILENCE_MSVC_C4127(fixed_rows) && np_rows != rows)
-                || (PYBIND11_SILENCE_MSVC_C4127(fixed_cols) && np_cols != cols)) {
+            if ((fixed_rows && np_rows != rows) || (fixed_cols && np_cols != cols)) {
                 return false;
             }
 
@@ -203,7 +198,7 @@ struct EigenProps {
                          stride = a.strides(0) / static_cast<ssize_t>(sizeof(Scalar));
 
         if (vector) { // Eigen type is a compile-time vector
-            if (PYBIND11_SILENCE_MSVC_C4127(fixed) && size != n) {
+            if (fixed && size != n) {
                 return false; // Vector size mismatch
             }
             return {rows == 1 ? 1 : n, cols == 1 ? 1 : n, stride};
@@ -220,7 +215,7 @@ struct EigenProps {
             }
             return {1, n, stride};
         } // Otherwise it's either fully dynamic, or column dynamic; both become a column vector
-        if (PYBIND11_SILENCE_MSVC_C4127(fixed_rows) && rows != n) {
+        if (fixed_rows && rows != n) {
             return false;
         }
         return {n, 1, stride};

include/pybind11/eigen/tensor.h

@@ -35,6 +35,8 @@ static_assert(EIGEN_VERSION_AT_LEAST(3, 3, 0),
 
 PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
 
+PYBIND11_DISABLE_WARNING_MSVC(4127)
+
 PYBIND11_NAMESPACE_BEGIN(detail)
 
 inline bool is_tensor_aligned(const void *data) {
@@ -389,7 +391,7 @@ struct type_caster<Eigen::TensorMap<Type, Options>,
 
         constexpr bool is_aligned = (Options & Eigen::Aligned) != 0;
 
-        if (PYBIND11_SILENCE_MSVC_C4127(is_aligned) && !is_tensor_aligned(arr.data())) {
+        if (is_aligned && !is_tensor_aligned(arr.data())) {
             return false;
         }
 
@@ -399,7 +401,7 @@ struct type_caster<Eigen::TensorMap<Type, Options>,
             return false;
         }
 
-        if (PYBIND11_SILENCE_MSVC_C4127(needs_writeable) && !arr.writeable()) {
+        if (needs_writeable && !arr.writeable()) {
             return false;
         }
 

include/pybind11/numpy.h

@@ -875,7 +875,7 @@ class array : public buffer {
      */
     template <typename T, ssize_t Dims = -1>
     detail::unchecked_mutable_reference<T, Dims> mutable_unchecked() & {
-        if (PYBIND11_SILENCE_MSVC_C4127(Dims >= 0) && ndim() != Dims) {
+        if (Dims >= 0 && ndim() != Dims) {
             throw std::domain_error("array has incorrect number of dimensions: "
                                     + std::to_string(ndim()) + "; expected "
                                     + std::to_string(Dims));
@@ -893,7 +893,7 @@ class array : public buffer {
      */
     template <typename T, ssize_t Dims = -1>
     detail::unchecked_reference<T, Dims> unchecked() const & {
-        if (PYBIND11_SILENCE_MSVC_C4127(Dims >= 0) && ndim() != Dims) {
+        if (Dims >= 0 && ndim() != Dims) {
             throw std::domain_error("array has incorrect number of dimensions: "
                                     + std::to_string(ndim()) + "; expected "
                                     + std::to_string(Dims));

include/pybind11/pybind11.h

@@ -49,6 +49,8 @@
 
 PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
 
+PYBIND11_DISABLE_WARNING_MSVC(4127)
+
 PYBIND11_NAMESPACE_BEGIN(detail)
 
 // Apply all the extensions translators from a list
@@ -177,7 +179,7 @@ class cpp_function : public function {
         auto *rec = unique_rec.get();
 
         /* Store the capture object directly in the function record if there is enough space */
-        if (PYBIND11_SILENCE_MSVC_C4127(sizeof(capture) <= sizeof(rec->data))) {
+        if (sizeof(capture) <= sizeof(rec->data)) {
             /* Without these pragmas, GCC warns that there might not be
                enough space to use the placement new operator. However, the
                'if' statement above ensures that this is the case. */
@@ -1841,8 +1843,7 @@ class class_ : public detail::generic_type {
         if (holder_ptr) {
             init_holder_from_existing(v_h, holder_ptr, std::is_copy_constructible<holder_type>());
             v_h.set_holder_constructed();
-        } else if (PYBIND11_SILENCE_MSVC_C4127(detail::always_construct_holder<holder_type>::value)
-                   || inst->owned) {
+        } else if (detail::always_construct_holder<holder_type>::value || inst->owned) {
             new (std::addressof(v_h.holder<holder_type>())) holder_type(v_h.value_ptr<type>());
             v_h.set_holder_constructed();
         }

include/pybind11/pytypes.h

@@ -33,6 +33,8 @@
 
 PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
 
+PYBIND11_DISABLE_WARNING_MSVC(4127)
+
 /* A few forward declarations */
 class handle;
 class object;
@@ -1693,7 +1695,7 @@ PYBIND11_NAMESPACE_BEGIN(detail)
 // unsigned type: (A)-1 != (B)-1 when A and B are unsigned types of different sizes).
 template <typename Unsigned>
 Unsigned as_unsigned(PyObject *o) {
-    if (PYBIND11_SILENCE_MSVC_C4127(sizeof(Unsigned) <= sizeof(unsigned long))) {
+    if (sizeof(Unsigned) <= sizeof(unsigned long)) {
         unsigned long v = PyLong_AsUnsignedLong(o);
         return v == (unsigned long) -1 && PyErr_Occurred() ? (Unsigned) -1 : (Unsigned) v;
     }
@@ -1710,7 +1712,7 @@ class int_ : public object {
     template <typename T, detail::enable_if_t<std::is_integral<T>::value, int> = 0>
     // NOLINTNEXTLINE(google-explicit-constructor)
     int_(T value) {
-        if (PYBIND11_SILENCE_MSVC_C4127(sizeof(T) <= sizeof(long))) {
+        if (sizeof(T) <= sizeof(long)) {
             if (std::is_signed<T>::value) {
                 m_ptr = PyLong_FromLong((long) value);
             } else {

tests/test_builtin_casters.cpp

@@ -72,6 +72,10 @@ class type_caster<ConstRefCasted> {
 PYBIND11_NAMESPACE_END(detail)
 PYBIND11_NAMESPACE_END(pybind11)
 
+PYBIND11_NAMESPACE_BEGIN(test_builtin_casters)
+
+PYBIND11_DISABLE_WARNING_MSVC(4127)
+
 TEST_SUBMODULE(builtin_casters, m) {
     // test_simple_string
     m.def("string_roundtrip", [](const char *s) { return s; });
@@ -86,7 +90,7 @@ TEST_SUBMODULE(builtin_casters, m) {
     std::wstring wstr;
     wstr.push_back(0x61);   // a
     wstr.push_back(0x2e18); // ⸘
-    if (PYBIND11_SILENCE_MSVC_C4127(sizeof(wchar_t) == 2)) {
+    if (sizeof(wchar_t) == 2) {
         wstr.push_back(mathbfA16_1);
         wstr.push_back(mathbfA16_2);
     } // 𝐀, utf16
@@ -113,7 +117,7 @@ TEST_SUBMODULE(builtin_casters, m) {
     // Under Python 2.7, invalid unicode UTF-32 characters didn't appear to trigger
     // UnicodeDecodeError
     m.def("bad_utf32_string", [=]() { return std::u32string({a32, char32_t(0xd800), z32}); });
-    if (PYBIND11_SILENCE_MSVC_C4127(sizeof(wchar_t) == 2)) {
+    if (sizeof(wchar_t) == 2) {
         m.def("bad_wchar_string", [=]() {
             return std::wstring({wchar_t(0x61), wchar_t(0xd800)});
         });
@@ -385,3 +389,5 @@ TEST_SUBMODULE(builtin_casters, m) {
     m.def("takes_const_ref_wrap",
           [](std::reference_wrapper<const ConstRefCasted> x) { return x.get().tag; });
 }
+
+PYBIND11_NAMESPACE_END(test_builtin_casters)

tests/test_eigen_tensor.inl

@@ -9,7 +9,9 @@
 
 #include "pybind11_tests.h"
 
-namespace PYBIND11_TEST_EIGEN_TENSOR_NAMESPACE {
+PYBIND11_NAMESPACE_BEGIN(PYBIND11_TEST_EIGEN_TENSOR_NAMESPACE)
+
+PYBIND11_DISABLE_WARNING_MSVC(4127)
 
 template <typename M>
 void reset_tensor(M &x) {
@@ -90,7 +92,7 @@ struct CustomExample {
 template <int Options>
 void init_tensor_module(pybind11::module &m) {
     const char *needed_options = "";
-    if (PYBIND11_SILENCE_MSVC_C4127(Options == Eigen::ColMajor)) {
+    if (Options == Eigen::ColMajor) {
         needed_options = "F";
     } else {
         needed_options = "C";
@@ -330,4 +332,4 @@ void test_module(py::module_ &m) {
     init_tensor_module<Eigen::RowMajor>(c_style);
 }
 
-} // namespace PYBIND11_TEST_EIGEN_TENSOR_NAMESPACE
+PYBIND11_NAMESPACE_END(PYBIND11_TEST_EIGEN_TENSOR_NAMESPACE)