Removing MSVC C4127 from pragma block at the top of pybind11.h (#3152)

* Removing pragma for 4127 (to see what is still broken with the latest code).

* Using new constexpr_bool() to suppress warning C4127.

* One missed case, Python 2 only.

* PYBIND11_SILENCE_MSVC_C4127 (more similar to the approach for C4100).

include/pybind11/cast.h

@@ -384,7 +384,11 @@ template <typename StringType, bool IsView = false> struct string_caster {
 
 
         const auto *buffer = reinterpret_cast<const CharT *>(PYBIND11_BYTES_AS_STRING(utfNbytes.ptr()));
         const auto *buffer = reinterpret_cast<const CharT *>(PYBIND11_BYTES_AS_STRING(utfNbytes.ptr()));
         size_t length = (size_t) PYBIND11_BYTES_SIZE(utfNbytes.ptr()) / sizeof(CharT);
         size_t length = (size_t) PYBIND11_BYTES_SIZE(utfNbytes.ptr()) / sizeof(CharT);
-        if (UTF_N > 8) { buffer++; length--; } // Skip BOM for UTF-16/32
+        // Skip BOM for UTF-16/32
+        if (PYBIND11_SILENCE_MSVC_C4127(UTF_N > 8)) {
+            buffer++;
+            length--;
+        }
         value = StringType(buffer, length);
         value = StringType(buffer, length);
 
 
         // If we're loading a string_view we need to keep the encoded Python object alive:
         // If we're loading a string_view we need to keep the encoded Python object alive:
@@ -499,7 +503,7 @@ public:
         // out how long the first encoded character is in bytes to distinguish between these two
         // 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
         // errors.  We also allow want to allow unicode characters U+0080 through U+00FF, as those
         // can fit into a single char value.
         // can fit into a single char value.
-        if (StringCaster::UTF_N == 8 && str_len > 1 && str_len <= 4) {
+        if (PYBIND11_SILENCE_MSVC_C4127(StringCaster::UTF_N == 8) && str_len > 1 && str_len <= 4) {
             auto v0 = static_cast<unsigned char>(value[0]);
             auto v0 = static_cast<unsigned char>(value[0]);
             // low bits only: 0-127
             // low bits only: 0-127
             // 0b110xxxxx - start of 2-byte sequence
             // 0b110xxxxx - start of 2-byte sequence
@@ -524,7 +528,7 @@ public:
         // UTF-16 is much easier: we can only have a surrogate pair for values above U+FFFF, thus a
         // 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
         // surrogate pair with total length 2 instantly indicates a range error (but not a "your
         // string was too long" error).
         // string was too long" error).
-        else if (StringCaster::UTF_N == 16 && str_len == 2) {
+        else if (PYBIND11_SILENCE_MSVC_C4127(StringCaster::UTF_N == 16) && str_len == 2) {
             one_char = static_cast<CharT>(value[0]);
             one_char = static_cast<CharT>(value[0]);
             if (one_char >= 0xD800 && one_char < 0xE000)
             if (one_char >= 0xD800 && one_char < 0xE000)
                 throw value_error("Character code point not in range(0x10000)");
                 throw value_error("Character code point not in range(0x10000)");
@@ -778,7 +782,7 @@ struct pyobject_caster {
         // For Python 2, without this implicit conversion, Python code would
         // For Python 2, without this implicit conversion, Python code would
         // need to be cluttered with six.ensure_text() or similar, only to be
         // need to be cluttered with six.ensure_text() or similar, only to be
         // un-cluttered later after Python 2 support is dropped.
         // un-cluttered later after Python 2 support is dropped.
-        if (std::is_same<T, str>::value && isinstance<bytes>(src)) {
+        if (PYBIND11_SILENCE_MSVC_C4127(std::is_same<T, str>::value) && isinstance<bytes>(src)) {
             PyObject *str_from_bytes = PyUnicode_FromEncodedObject(src.ptr(), "utf-8", nullptr);
             PyObject *str_from_bytes = PyUnicode_FromEncodedObject(src.ptr(), "utf-8", nullptr);
             if (!str_from_bytes) throw error_already_set();
             if (!str_from_bytes) throw error_already_set();
             value = reinterpret_steal<type>(str_from_bytes);
             value = reinterpret_steal<type>(str_from_bytes);

include/pybind11/detail/common.h

@@ -941,5 +941,16 @@ inline constexpr void workaround_incorrect_msvc_c4100(Args &&...) {}
 #    define PYBIND11_WORKAROUND_INCORRECT_MSVC_C4100(...)
 #    define PYBIND11_WORKAROUND_INCORRECT_MSVC_C4100(...)
 #endif
 #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(...) detail::silence_msvc_c4127(__VA_ARGS__)
+
+#else
+#    define PYBIND11_SILENCE_MSVC_C4127(...) __VA_ARGS__
+#endif
+
 PYBIND11_NAMESPACE_END(detail)
 PYBIND11_NAMESPACE_END(detail)
 PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE)
 PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE)

include/pybind11/detail/init.h

@@ -96,7 +96,7 @@ template <typename Class>
 void construct(value_and_holder &v_h, Cpp<Class> *ptr, bool need_alias) {
 void construct(value_and_holder &v_h, Cpp<Class> *ptr, bool need_alias) {
     PYBIND11_WORKAROUND_INCORRECT_MSVC_C4100(need_alias);
     PYBIND11_WORKAROUND_INCORRECT_MSVC_C4100(need_alias);
     no_nullptr(ptr);
     no_nullptr(ptr);
-    if (Class::has_alias && need_alias && !is_alias<Class>(ptr)) {
+    if (PYBIND11_SILENCE_MSVC_C4127(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
         // 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
         // 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
         // moved away leftover, if the alias construction works, or the value itself if we
@@ -136,7 +136,7 @@ void construct(value_and_holder &v_h, Holder<Class> holder, bool need_alias) {
     auto *ptr = holder_helper<Holder<Class>>::get(holder);
     auto *ptr = holder_helper<Holder<Class>>::get(holder);
     no_nullptr(ptr);
     no_nullptr(ptr);
     // If we need an alias, check that the held pointer is actually an alias instance
     // If we need an alias, check that the held pointer is actually an alias instance
-    if (Class::has_alias && need_alias && !is_alias<Class>(ptr))
+    if (PYBIND11_SILENCE_MSVC_C4127(Class::has_alias) && need_alias && !is_alias<Class>(ptr))
         throw type_error("pybind11::init(): construction failed: returned holder-wrapped instance "
         throw type_error("pybind11::init(): construction failed: returned holder-wrapped instance "
                          "is not an alias instance");
                          "is not an alias instance");
 
 
@@ -153,7 +153,7 @@ void construct(value_and_holder &v_h, Cpp<Class> &&result, bool need_alias) {
     PYBIND11_WORKAROUND_INCORRECT_MSVC_C4100(need_alias);
     PYBIND11_WORKAROUND_INCORRECT_MSVC_C4100(need_alias);
     static_assert(std::is_move_constructible<Cpp<Class>>::value,
     static_assert(std::is_move_constructible<Cpp<Class>>::value,
         "pybind11::init() return-by-value factory function requires a movable class");
         "pybind11::init() return-by-value factory function requires a movable class");
-    if (Class::has_alias && need_alias)
+    if (PYBIND11_SILENCE_MSVC_C4127(Class::has_alias) && need_alias)
         construct_alias_from_cpp<Class>(is_alias_constructible<Class>{}, v_h, std::move(result));
         construct_alias_from_cpp<Class>(is_alias_constructible<Class>{}, v_h, std::move(result));
     else
     else
         v_h.value_ptr() = new Cpp<Class>(std::move(result));
         v_h.value_ptr() = new Cpp<Class>(std::move(result));

include/pybind11/pybind11.h

@@ -10,10 +10,7 @@
 
 
 #pragma once
 #pragma once
 
 
-#if defined(_MSC_VER) && !defined(__INTEL_COMPILER)
-#  pragma warning(push)
-#  pragma warning(disable: 4127) // warning C4127: Conditional expression is constant
-#elif defined(__GNUG__) && !defined(__clang__) && !defined(__INTEL_COMPILER)
+#if defined(__GNUG__) && !defined(__clang__) && !defined(__INTEL_COMPILER)
 #  pragma GCC diagnostic push
 #  pragma GCC diagnostic push
 #  pragma GCC diagnostic ignored "-Wunused-but-set-parameter"
 #  pragma GCC diagnostic ignored "-Wunused-but-set-parameter"
 #  pragma GCC diagnostic ignored "-Wattributes"
 #  pragma GCC diagnostic ignored "-Wattributes"
@@ -166,7 +163,7 @@ protected:
         auto rec = unique_rec.get();
         auto rec = unique_rec.get();
 
 
         /* Store the capture object directly in the function record if there is enough space */
         /* Store the capture object directly in the function record if there is enough space */
-        if (sizeof(capture) <= sizeof(rec->data)) {
+        if (PYBIND11_SILENCE_MSVC_C4127(sizeof(capture) <= sizeof(rec->data))) {
             /* Without these pragmas, GCC warns that there might not be
             /* Without these pragmas, GCC warns that there might not be
                enough space to use the placement new operator. However, the
                enough space to use the placement new operator. However, the
                'if' statement above ensures that this is the case. */
                'if' statement above ensures that this is the case. */
@@ -2388,8 +2385,6 @@ PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE)
 #    pragma GCC diagnostic pop // -Wnoexcept-type
 #    pragma GCC diagnostic pop // -Wnoexcept-type
 #endif
 #endif
 
 
-#if defined(_MSC_VER) && !defined(__INTEL_COMPILER)
-#  pragma warning(pop)
-#elif defined(__GNUG__) && !defined(__clang__) && !defined(__INTEL_COMPILER)
+#if defined(__GNUG__) && !defined(__clang__) && !defined(__INTEL_COMPILER)
 #  pragma GCC diagnostic pop
 #  pragma GCC diagnostic pop
 #endif
 #endif

include/pybind11/pytypes.h

@@ -1191,9 +1191,9 @@ PYBIND11_NAMESPACE_BEGIN(detail)
 // unsigned type: (A)-1 != (B)-1 when A and B are unsigned types of different sizes).
 // unsigned type: (A)-1 != (B)-1 when A and B are unsigned types of different sizes).
 template <typename Unsigned>
 template <typename Unsigned>
 Unsigned as_unsigned(PyObject *o) {
 Unsigned as_unsigned(PyObject *o) {
-    if (sizeof(Unsigned) <= sizeof(unsigned long)
+    if (PYBIND11_SILENCE_MSVC_C4127(sizeof(Unsigned) <= sizeof(unsigned long))
 #if PY_VERSION_HEX < 0x03000000
 #if PY_VERSION_HEX < 0x03000000
-            || PyInt_Check(o)
+        || PyInt_Check(o)
 #endif
 #endif
     ) {
     ) {
         unsigned long v = PyLong_AsUnsignedLong(o);
         unsigned long v = PyLong_AsUnsignedLong(o);
@@ -1212,7 +1212,7 @@ public:
     template <typename T,
     template <typename T,
               detail::enable_if_t<std::is_integral<T>::value, int> = 0>
               detail::enable_if_t<std::is_integral<T>::value, int> = 0>
     int_(T value) {
     int_(T value) {
-        if (sizeof(T) <= sizeof(long)) {
+        if (PYBIND11_SILENCE_MSVC_C4127(sizeof(T) <= sizeof(long))) {
             if (std::is_signed<T>::value)
             if (std::is_signed<T>::value)
                 m_ptr = PyLong_FromLong((long) value);
                 m_ptr = PyLong_FromLong((long) value);
             else
             else