From c951e0ad28c9e8823a7db23b3c8b6b357a0f1a20 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Wed, 28 Jul 2021 17:19:02 -0700 Subject: [PATCH 1/4] Removing pragma for 4127 (to see what is still broken with the latest code). --- include/pybind11/pybind11.h | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index ec6bd8d351..ec568e32dd 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -10,10 +10,7 @@ #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 ignored "-Wunused-but-set-parameter" # pragma GCC diagnostic ignored "-Wattributes" @@ -2388,8 +2385,6 @@ PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE) # pragma GCC diagnostic pop // -Wnoexcept-type #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 #endif From a1e181c65dbc594b0a42e3edd02766055464e3c8 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Wed, 28 Jul 2021 17:53:35 -0700 Subject: [PATCH 2/4] Using new constexpr_bool() to suppress warning C4127. --- include/pybind11/cast.h | 6 +++--- include/pybind11/detail/common.h | 3 +++ include/pybind11/detail/init.h | 6 +++--- include/pybind11/pybind11.h | 2 +- include/pybind11/pytypes.h | 4 ++-- 5 files changed, 12 insertions(+), 9 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 5ff0355a6f..a5dbf118ad 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -384,7 +384,7 @@ template struct string_caster { const auto *buffer = reinterpret_cast(PYBIND11_BYTES_AS_STRING(utfNbytes.ptr())); size_t length = (size_t) PYBIND11_BYTES_SIZE(utfNbytes.ptr()) / sizeof(CharT); - if (UTF_N > 8) { buffer++; length--; } // Skip BOM for UTF-16/32 + if (constexpr_bool(UTF_N > 8)) { buffer++; length--; } // Skip BOM for UTF-16/32 value = StringType(buffer, length); // If we're loading a string_view we need to keep the encoded Python object alive: @@ -499,7 +499,7 @@ template struct type_caster 1 && str_len <= 4) { + if (constexpr_bool(StringCaster::UTF_N == 8) && str_len > 1 && str_len <= 4) { auto v0 = static_cast(value[0]); // low bits only: 0-127 // 0b110xxxxx - start of 2-byte sequence @@ -524,7 +524,7 @@ template struct type_caster(value[0]); if (one_char >= 0xD800 && one_char < 0xE000) throw value_error("Character code point not in range(0x10000)"); diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index d1e9e6591f..3d8ea8459e 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -941,5 +941,8 @@ inline constexpr void workaround_incorrect_msvc_c4100(Args &&...) {} # define PYBIND11_WORKAROUND_INCORRECT_MSVC_C4100(...) #endif +// Suppresses MSVC warning C4127: Conditional expression is constant +constexpr inline bool constexpr_bool(bool cond) { return cond; } + PYBIND11_NAMESPACE_END(detail) PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE) diff --git a/include/pybind11/detail/init.h b/include/pybind11/detail/init.h index 3ebec041f0..a87f237d24 100644 --- a/include/pybind11/detail/init.h +++ b/include/pybind11/detail/init.h @@ -96,7 +96,7 @@ template void construct(value_and_holder &v_h, Cpp *ptr, bool need_alias) { PYBIND11_WORKAROUND_INCORRECT_MSVC_C4100(need_alias); no_nullptr(ptr); - if (Class::has_alias && need_alias && !is_alias(ptr)) { + if (constexpr_bool(Class::has_alias) && need_alias && !is_alias(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 @@ -136,7 +136,7 @@ void construct(value_and_holder &v_h, Holder holder, bool need_alias) { auto *ptr = holder_helper>::get(holder); no_nullptr(ptr); // If we need an alias, check that the held pointer is actually an alias instance - if (Class::has_alias && need_alias && !is_alias(ptr)) + if (constexpr_bool(Class::has_alias) && need_alias && !is_alias(ptr)) throw type_error("pybind11::init(): construction failed: returned holder-wrapped instance " "is not an alias instance"); @@ -153,7 +153,7 @@ void construct(value_and_holder &v_h, Cpp &&result, bool need_alias) { PYBIND11_WORKAROUND_INCORRECT_MSVC_C4100(need_alias); static_assert(std::is_move_constructible>::value, "pybind11::init() return-by-value factory function requires a movable class"); - if (Class::has_alias && need_alias) + if (constexpr_bool(Class::has_alias) && need_alias) construct_alias_from_cpp(is_alias_constructible{}, v_h, std::move(result)); else v_h.value_ptr() = new Cpp(std::move(result)); diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index ec568e32dd..6b5354d3be 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -163,7 +163,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 (sizeof(capture) <= sizeof(rec->data)) { + if (constexpr_bool(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. */ diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 4cf606e8d0..6286adebf9 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -1191,7 +1191,7 @@ PYBIND11_NAMESPACE_BEGIN(detail) // unsigned type: (A)-1 != (B)-1 when A and B are unsigned types of different sizes). template Unsigned as_unsigned(PyObject *o) { - if (sizeof(Unsigned) <= sizeof(unsigned long) + if (constexpr_bool(sizeof(Unsigned) <= sizeof(unsigned long)) #if PY_VERSION_HEX < 0x03000000 || PyInt_Check(o) #endif @@ -1212,7 +1212,7 @@ class int_ : public object { template ::value, int> = 0> int_(T value) { - if (sizeof(T) <= sizeof(long)) { + if (detail::constexpr_bool(sizeof(T) <= sizeof(long))) { if (std::is_signed::value) m_ptr = PyLong_FromLong((long) value); else From e87f9c447ce888f80fa6dd9a9f31e24f20700a53 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Wed, 28 Jul 2021 18:28:13 -0700 Subject: [PATCH 3/4] One missed case, Python 2 only. --- 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 a5dbf118ad..991c88a244 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -778,7 +778,7 @@ struct pyobject_caster { // For Python 2, without this implicit conversion, Python code would // need to be cluttered with six.ensure_text() or similar, only to be // un-cluttered later after Python 2 support is dropped. - if (std::is_same::value && isinstance(src)) { + if (constexpr_bool(std::is_same::value) && isinstance(src)) { PyObject *str_from_bytes = PyUnicode_FromEncodedObject(src.ptr(), "utf-8", nullptr); if (!str_from_bytes) throw error_already_set(); value = reinterpret_steal(str_from_bytes); From ced75c83e1773fc18d9d09cdc9b4019ab38d9fc0 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Thu, 29 Jul 2021 17:57:32 -0700 Subject: [PATCH 4/4] PYBIND11_SILENCE_MSVC_C4127 (more similar to the approach for C4100). --- include/pybind11/cast.h | 12 ++++++++---- include/pybind11/detail/common.h | 12 ++++++++++-- include/pybind11/detail/init.h | 6 +++--- include/pybind11/pybind11.h | 2 +- include/pybind11/pytypes.h | 6 +++--- 5 files changed, 25 insertions(+), 13 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 991c88a244..211cf09bcd 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -384,7 +384,11 @@ template struct string_caster { const auto *buffer = reinterpret_cast(PYBIND11_BYTES_AS_STRING(utfNbytes.ptr())); size_t length = (size_t) PYBIND11_BYTES_SIZE(utfNbytes.ptr()) / sizeof(CharT); - if (constexpr_bool(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); // If we're loading a string_view we need to keep the encoded Python object alive: @@ -499,7 +503,7 @@ template struct type_caster 1 && str_len <= 4) { + if (PYBIND11_SILENCE_MSVC_C4127(StringCaster::UTF_N == 8) && str_len > 1 && str_len <= 4) { auto v0 = static_cast(value[0]); // low bits only: 0-127 // 0b110xxxxx - start of 2-byte sequence @@ -524,7 +528,7 @@ template struct type_caster(value[0]); if (one_char >= 0xD800 && one_char < 0xE000) 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 // need to be cluttered with six.ensure_text() or similar, only to be // un-cluttered later after Python 2 support is dropped. - if (constexpr_bool(std::is_same::value) && isinstance(src)) { + if (PYBIND11_SILENCE_MSVC_C4127(std::is_same::value) && isinstance(src)) { PyObject *str_from_bytes = PyUnicode_FromEncodedObject(src.ptr(), "utf-8", nullptr); if (!str_from_bytes) throw error_already_set(); value = reinterpret_steal(str_from_bytes); diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index 3d8ea8459e..e8d83ae0db 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -941,8 +941,16 @@ inline constexpr void workaround_incorrect_msvc_c4100(Args &&...) {} # define PYBIND11_WORKAROUND_INCORRECT_MSVC_C4100(...) #endif -// Suppresses MSVC warning C4127: Conditional expression is constant -constexpr inline bool constexpr_bool(bool cond) { return cond; } +#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(PYBIND11_NAMESPACE) diff --git a/include/pybind11/detail/init.h b/include/pybind11/detail/init.h index a87f237d24..e795da7d75 100644 --- a/include/pybind11/detail/init.h +++ b/include/pybind11/detail/init.h @@ -96,7 +96,7 @@ template void construct(value_and_holder &v_h, Cpp *ptr, bool need_alias) { PYBIND11_WORKAROUND_INCORRECT_MSVC_C4100(need_alias); no_nullptr(ptr); - if (constexpr_bool(Class::has_alias) && need_alias && !is_alias(ptr)) { + if (PYBIND11_SILENCE_MSVC_C4127(Class::has_alias) && need_alias && !is_alias(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 @@ -136,7 +136,7 @@ void construct(value_and_holder &v_h, Holder holder, bool need_alias) { auto *ptr = holder_helper>::get(holder); no_nullptr(ptr); // If we need an alias, check that the held pointer is actually an alias instance - if (constexpr_bool(Class::has_alias) && need_alias && !is_alias(ptr)) + if (PYBIND11_SILENCE_MSVC_C4127(Class::has_alias) && need_alias && !is_alias(ptr)) throw type_error("pybind11::init(): construction failed: returned holder-wrapped instance " "is not an alias instance"); @@ -153,7 +153,7 @@ void construct(value_and_holder &v_h, Cpp &&result, bool need_alias) { PYBIND11_WORKAROUND_INCORRECT_MSVC_C4100(need_alias); static_assert(std::is_move_constructible>::value, "pybind11::init() return-by-value factory function requires a movable class"); - if (constexpr_bool(Class::has_alias) && need_alias) + if (PYBIND11_SILENCE_MSVC_C4127(Class::has_alias) && need_alias) construct_alias_from_cpp(is_alias_constructible{}, v_h, std::move(result)); else v_h.value_ptr() = new Cpp(std::move(result)); diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 6b5354d3be..aeeb3368a8 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -163,7 +163,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 (constexpr_bool(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 enough space to use the placement new operator. However, the 'if' statement above ensures that this is the case. */ diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 6286adebf9..c7b2501feb 100644 --- a/include/pybind11/pytypes.h +++ b/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). template Unsigned as_unsigned(PyObject *o) { - if (constexpr_bool(sizeof(Unsigned) <= sizeof(unsigned long)) + if (PYBIND11_SILENCE_MSVC_C4127(sizeof(Unsigned) <= sizeof(unsigned long)) #if PY_VERSION_HEX < 0x03000000 - || PyInt_Check(o) + || PyInt_Check(o) #endif ) { unsigned long v = PyLong_AsUnsignedLong(o); @@ -1212,7 +1212,7 @@ class int_ : public object { template ::value, int> = 0> int_(T value) { - if (detail::constexpr_bool(sizeof(T) <= sizeof(long))) { + if (PYBIND11_SILENCE_MSVC_C4127(sizeof(T) <= sizeof(long))) { if (std::is_signed::value) m_ptr = PyLong_FromLong((long) value); else