Skip to content

Removing MSVC C4800 from pragma block at the top of pybind11.h #3141

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.

Sign up for GitHub

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 3 commits into from
Jul 26, 2021
Merged

Removing MSVC C4800 from pragma block at the top of pybind11.h #3141

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9db1da9
Adding PYBIND11_COMPAT_BOOL_CAST to appease MSVC 2015 warning C4800.
rwgk Jul 25, 2021
eecd3dd
Replacing PYBIND11_COMPAT_BOOL_CAST with simpler != 0
rwgk Jul 26, 2021
f9ef482
Extra parentheses (almost all compilers failed without these).
rwgk Jul 26, 2021
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
2 changes: 1 addition & 1 deletion include/pybind11/buffer_info.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ struct buffer_info {
view->strides
? std::vector<ssize_t>(view->strides, view->strides + view->ndim)
: detail::c_strides({view->shape, view->shape + view->ndim}, view->itemsize),
view->readonly) {
(view->readonly != 0)) {
this->m_view = view;
this->ownview = ownview;
}
Expand Down
2 changes: 1 addition & 1 deletion include/pybind11/cast.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ template <> class type_caster<bool> {
}
#endif
if (res == 0 || res == 1) {
value = (bool) res;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just make these value = res != 0; ? The problem is that (bool) int_value is poor for performance (according to the warning), so we shouldn't be pretending this is okay to cast.

Copy link
Collaborator Author

@rwgk rwgk Jul 26, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Applied globally. — I was aiming for a robust general solution before, but you're right, we only need specific solutions, and != 0 is simpler and therefore better.

value = (res != 0);
return true;
}
PyErr_Clear();
Expand Down
4 changes: 2 additions & 2 deletions include/pybind11/detail/type_caster_base.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ struct value_and_holder {
return reinterpret_cast<V *&>(vh[0]);
}
// True if this `value_and_holder` has a non-null value pointer
explicit operator bool() const { return value_ptr(); }
explicit operator bool() const { return value_ptr() != nullptr; }

template <typename H> H &holder() const {
return reinterpret_cast<H &>(vh[1]);
Expand All @@ -252,7 +252,7 @@ struct value_and_holder {
bool instance_registered() const {
return inst->simple_layout
? inst->simple_instance_registered
: inst->nonsimple.status[index] & instance::status_instance_registered;
: ((inst->nonsimple.status[index] & instance::status_instance_registered) != 0);
}
void set_instance_registered(bool v = true) const {
if (inst->simple_layout)
Expand Down
1 change: 0 additions & 1 deletion include/pybind11/pybind11.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
# pragma warning(push)
# pragma warning(disable: 4100) // warning C4100: Unreferenced formal parameter
# pragma warning(disable: 4127) // warning C4127: Conditional expression is constant
# pragma warning(disable: 4800) // warning C4800: 'int': forcing value to bool 'true' or 'false' (performance warning)
# pragma warning(disable: 4505) // warning C4505: 'PySlice_GetIndicesEx': unreferenced local function has been removed (PyPy only)
#elif defined(__GNUG__) && !defined(__clang__)
# pragma GCC diagnostic push
Expand Down
6 changes: 4 additions & 2 deletions include/pybind11/pytypes.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,9 @@ class PYBIND11_EXPORT error_already_set : public std::runtime_error {
/// Check if the currently trapped error type matches the given Python exception class (or a
/// subclass thereof). May also be passed a tuple to search for any exception class matches in
/// the given tuple.
bool matches(handle exc) const { return PyErr_GivenExceptionMatches(m_type.ptr(), exc.ptr()); }
bool matches(handle exc) const {
return (PyErr_GivenExceptionMatches(m_type.ptr(), exc.ptr()) != 0);
}

const object& type() const { return m_type; }
const object& value() const { return m_value; }
Expand Down Expand Up @@ -853,7 +855,7 @@ PYBIND11_NAMESPACE_END(detail)
Name(handle h, borrowed_t) : Parent(h, borrowed_t{}) { } \
Name(handle h, stolen_t) : Parent(h, stolen_t{}) { } \
PYBIND11_DEPRECATED("Use py::isinstance<py::python_type>(obj) instead") \
bool check() const { return m_ptr != nullptr && (bool) CheckFun(m_ptr); } \
bool check() const { return m_ptr != nullptr && (CheckFun(m_ptr) != 0); } \
static bool check_(handle h) { return h.ptr() != nullptr && CheckFun(h.ptr()); } \
template <typename Policy_> \
Name(const ::pybind11::detail::accessor<Policy_> &a) : Name(object(a)) { }
Expand Down