Skip to content

Commit

Permalink
Correct options on Eigen::MappedSparseMatrix & adding MSVC C4127 supp…
Browse files Browse the repository at this point in the history
…ression around Eigen includes. (#3352)

* Adding MSVC C4127 suppression around Eigen includes.

* For MSVC 2015 only: also adding the C4127 suppression to test_eigen.cpp

* Copying original change from PR #3343, with extra line breaks to not run past 99 columns (our desired but currently not enforced limit).
  • Loading branch information
Ralf W. Grosse-Kunstleve authored Oct 11, 2021
1 parent 02c0557 commit 7c58058
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 3 deletions.
18 changes: 17 additions & 1 deletion include/pybind11/eigen.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,23 @@

#include "numpy.h"

// The C4127 suppression was introduced for Eigen 3.4.0. In theory we could
// make it version specific, or even remove it later, but considering that
// 1. C4127 is generally far more distracting than useful for modern template code, and
// 2. we definitely want to ignore any MSVC warnings originating from Eigen code,
// it is probably best to keep this around indefinitely.
#if defined(_MSC_VER)
# pragma warning(push)
# pragma warning(disable: 4127) // C4127: conditional expression is constant
#endif

#include <Eigen/Core>
#include <Eigen/SparseCore>

#if defined(_MSC_VER)
# pragma warning(pop)
#endif

// Eigen prior to 3.2.7 doesn't have proper move constructors--but worse, some classes get implicit
// move constructors that break things. We could detect this an explicitly copy, but an extra copy
// of matrices seems highly undesirable.
Expand Down Expand Up @@ -559,7 +573,9 @@ struct type_caster<Type, enable_if_t<is_eigen_sparse<Type>::value>> {
if (!values || !innerIndices || !outerIndices)
return false;

value = Eigen::MappedSparseMatrix<Scalar, Type::Flags, StorageIndex>(
value = Eigen::MappedSparseMatrix<Scalar,
Type::Flags & (Eigen::RowMajor | Eigen::ColMajor),
StorageIndex>(
shape[0].cast<Index>(), shape[1].cast<Index>(), nnz,
outerIndices.mutable_data(), innerIndices.mutable_data(), values.mutable_data());

Expand Down
4 changes: 2 additions & 2 deletions tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -174,9 +174,9 @@ set(PYBIND11_CROSS_MODULE_GIL_TESTS test_gil_scoped.py)
set(PYBIND11_EIGEN_REPO
"https://gitlab.com/libeigen/eigen.git"
CACHE STRING "Eigen repository to use for tests")
# This hash is for 3.3.8, using a hash for security reasons
# This hash is for 3.4.0, using a hash for security reasons
set(PYBIND11_EIGEN_VERSION
"dc252fbf00079ccab57948a164b1421703fe4361"
"929bc0e191d0927b1735b9a1ddc0e8b77e3a25ec"
CACHE STRING "Eigen version to use for tests")

# Check if Eigen is available; if not, remove from PYBIND11_TEST_FILES (but
Expand Down
3 changes: 3 additions & 0 deletions tests/test_eigen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@
#include <pybind11/stl.h>

#if defined(_MSC_VER)
#if _MSC_VER < 1910 // VS 2015's MSVC
# pragma warning(disable: 4127) // C4127: conditional expression is constant
#endif
# pragma warning(disable: 4996) // C4996: std::unary_negation is deprecated
#endif

Expand Down

0 comments on commit 7c58058

Please sign in to comment.