Moving pragma for MSVC warning C4505 from pybind11.h to existing list in detail/common.h #3160
Conversation
| @@ -13,7 +13,6 @@ | |||
| #if defined(_MSC_VER) && !defined(__INTEL_COMPILER) | |||
| # pragma warning(push) | |||
| # pragma warning(disable: 4127) // warning C4127: Conditional expression is constant | |||
| # pragma warning(disable: 4505) // warning C4505: 'PySlice_GetIndicesEx': unreferenced local function has been removed (PyPy only) | |||
There was a problem hiding this comment.
I don't like that we lose the comment here, especially the PyPy only part. Could we move the comment to just above the other place? Also, we could make this PyPy only, if we wanted to.
There was a problem hiding this comment.
I'll copy it over to the other place. It will look odd though as the only one. What do you think about adding the message for all diagnostics, for completeness?
Also, we could make this PyPy only, if we wanted to.
I was thinking about that but
-
how do we know that we have PyPy before including Python.h?
-
how much do we want to, given 1. that there are four other diagnostics already, 2. how useful is the warning, really? The suppression is not even for our code.
But in any case, if there is a practical method to make it PyPy only, I'd be happy to do it that way.
There was a problem hiding this comment.
What do you think about adding the message for all diagnostics, for completeness?
+1, if you want to do it :)
how do we know that we have PyPy before including Python.h?
This is probably why it's there without a check for PyPy currently. My only worry is that, like hiding any warning, you are also are losing sensitivity to what it was supposed to catch as well. Perfectly fine with not making it depend on PyPy then.
There was a problem hiding this comment.
@henryiii please take another look: it turns out none of the existing four suppressions is needed anymore.
rwgk
force-pushed
the
pragma_block_rm_msvc_c4505
branch
from
2c8aa67 to
d9d124b
Compare
… in detail/common.h (#3160) * Moving pragma for C4505 from pybind11.h to existing list in detail/common.h. * Removing 4 existing suppressions to 1. see what is still needed and 2. capture the MSVC messages. * It turns out none of the 4 pragmas are needed anymore.
Follow-on to PR #3127, based on results obtained under PR #3125.
The suggested changelog entry will be in the final PR of this cleanup series.
Worksheet