Correct options on Eigen::MappedSparseMatrix & adding MSVC C4127 suppression around Eigen includes. #3352
Conversation
|
Looks like it needs to be around the offending test_eigen.cpp . Disappointing. |
Indeed, BUT apparently MSVC 2015 only. That's good enough for me: I'll add the C4127 at the top of test_eigen.cpp, but for MSVC 2015 only. We'll still have plenty of coverage for C4127 issues in the test code itself via the more modern MSVC versions. |
| #if _MSC_VER < 1910 // VS 2015's MSVC | ||
| # pragma warning(disable: 4127) // C4127: conditional expression is constant | ||
| #endif |
There was a problem hiding this comment.
Why do you need this here? Isn't it suppressed in the include in pybind11? If that doesn't work, why are we suppressing it in the include?
There was a problem hiding this comment.
@henryiii It works except for MSCV2015. So we need to do both.
There was a problem hiding this comment.
For MSVC 2015 only the suppression in eigen.h is not sufficient:
https://github.com/pybind/pybind11/runs/3862647055?check_suite_focus=true
The suppression in eigen.h is for all MSVC versions, but only applies to the Eigen includes.
The suppression here is only for MSVC 2015, but global to this .cpp file.
…o not run past 99 columns (our desired but currently not enforced limit).
|
Appveyor is happy now (yay!) but there is a known flake (sigh). |
|
Known flakes are fine, not blockers. |
rwgk
changed the title
|
Thanks all! @cantonios I'm including your change here to keep things simple. |
Description
This PR combines the original change from PR #3343 (by @cantonios, with extra line breaks) with newly added MSVC warning suppressions to enable full testing with Eigen 3.4.0.
Suggested changelog entry:
A long-standing bug in eigen.h was fixed (originally PR #3343). The bug was unmasked by newly added ``static_assert``s in the Eigen 3.4.0 release.