Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jul 30, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions include/pybind11/cast.h
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
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);

// If we're loading a string_view we need to keep the encoded Python object alive:
Expand Down Expand Up @@ -499,7 +503,7 @@ template <typename CharT> struct type_caster<CharT, enable_if_t<is_std_char_type
// 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 (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]);
// low bits only: 0-127
// 0b110xxxxx - start of 2-byte sequence
Expand All @@ -524,7 +528,7 @@ template <typename CharT> struct type_caster<CharT, enable_if_t<is_std_char_type
// 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 (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]);
if (one_char >= 0xD800 && one_char < 0xE000)
throw value_error("Character code point not in range(0x10000)");
Expand Down Expand Up @@ -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 (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);
if (!str_from_bytes) throw error_already_set();
value = reinterpret_steal<type>(str_from_bytes);
Expand Down
11 changes: 11 additions & 0 deletions include/pybind11/detail/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -941,5 +941,16 @@ inline constexpr void workaround_incorrect_msvc_c4100(Args &&...) {}
# define PYBIND11_WORKAROUND_INCORRECT_MSVC_C4100(...)
#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(PYBIND11_NAMESPACE)
6 changes: 3 additions & 3 deletions include/pybind11/detail/init.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,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 (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
// 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
Expand Down Expand Up @@ -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);
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<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 "
"is not an alias instance");

Expand All @@ -153,7 +153,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 (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));
else
v_h.value_ptr() = new Cpp<Class>(std::move(result));
Expand Down
11 changes: 3 additions & 8 deletions include/pybind11/pybind11.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -166,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 (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. */
Expand Down Expand Up @@ -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
6 changes: 3 additions & 3 deletions include/pybind11/pytypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 <typename Unsigned>
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
|| PyInt_Check(o)
|| PyInt_Check(o)
#endif
) {
unsigned long v = PyLong_AsUnsignedLong(o);
Expand All @@ -1212,7 +1212,7 @@ class int_ : public object {
template <typename T,
detail::enable_if_t<std::is_integral<T>::value, int> = 0>
int_(T value) {
if (sizeof(T) <= sizeof(long)) {
if (PYBIND11_SILENCE_MSVC_C4127(sizeof(T) <= sizeof(long))) {
if (std::is_signed<T>::value)
m_ptr = PyLong_FromLong((long) value);
else
Expand Down