diff --git a/Eigen/src/Core/PermutationMatrix.h b/Eigen/src/Core/PermutationMatrix.h index 4748b118a..eb8e79782 100644 --- a/Eigen/src/Core/PermutationMatrix.h +++ b/Eigen/src/Core/PermutationMatrix.h @@ -468,17 +468,17 @@ class PermutationWrapper : public PermutationBase -EIGEN_DEVICE_FUNC const Product operator*( +EIGEN_DEVICE_FUNC const Product operator*( const MatrixBase& matrix, const PermutationBase& permutation) { - return Product(matrix.derived(), permutation.derived()); + return Product(matrix.derived(), permutation.derived()); } /** \returns the matrix with the permutation applied to the rows. */ template -EIGEN_DEVICE_FUNC const Product operator*( +EIGEN_DEVICE_FUNC const Product operator*( const PermutationBase& permutation, const MatrixBase& matrix) { - return Product(permutation.derived(), matrix.derived()); + return Product(permutation.derived(), matrix.derived()); } template @@ -520,16 +520,16 @@ class InverseImpl : public EigenBase - friend const Product operator*(const MatrixBase& matrix, - const InverseType& trPerm) { - return Product(matrix.derived(), trPerm.derived()); + friend const Product operator*(const MatrixBase& matrix, + const InverseType& trPerm) { + return Product(matrix.derived(), trPerm.derived()); } /** \returns the matrix with the inverse permutation applied to the rows. */ template - const Product operator*(const MatrixBase& matrix) const { - return Product(derived(), matrix.derived()); + const Product operator*(const MatrixBase& matrix) const { + return Product(derived(), matrix.derived()); } }; diff --git a/test/permutationmatrices.cpp b/test/permutationmatrices.cpp index 2c8378301..ee794b24e 100644 --- a/test/permutationmatrices.cpp +++ b/test/permutationmatrices.cpp @@ -39,7 +39,8 @@ void permutationmatrices(const MatrixType& m) { RightTranspositionsType rt(rv); MatrixType m_permuted = MatrixType::Random(rows, cols); - VERIFY_EVALUATION_COUNT(m_permuted = lp * m_original * rp, 1); // 1 temp for sub expression "lp * m_original" + VERIFY_EVALUATION_COUNT(m_permuted.noalias() = lp * m_original * rp, + 1); // 1 temp for sub expression "lp * m_original" for (int i = 0; i < rows; i++) for (int j = 0; j < cols; j++) VERIFY_IS_APPROX(m_permuted(lv(i), j), m_original(i, rv(j))); @@ -50,7 +51,7 @@ void permutationmatrices(const MatrixType& m) { VERIFY_IS_APPROX(m_permuted, lm * m_original * rm); m_permuted = m_original; - VERIFY_EVALUATION_COUNT(m_permuted = lp * m_permuted * rp, 1); + VERIFY_EVALUATION_COUNT(m_permuted.noalias() = lp * m_permuted * rp, 1); VERIFY_IS_APPROX(m_permuted, lm * m_original * rm); LeftPermutationType lpi; @@ -169,6 +170,26 @@ void bug890() { VERIFY_IS_APPROX(v1, (P.inverse() * rhs).eval()); } +void test_aliasing() { + // Bug #2869. + auto P = Eigen::PermutationMatrix<4>{Eigen::Vector4i{0, 2, 3, 1}}; + { + Eigen::Vector z = {0.0f, 1.1f, 2.2f, 3.3f, 4.4f}; + Eigen::Vector expected = z; + z.tail<4>() = P * z.head<4>(); + expected.tail<4>() = (P * expected.head<4>()).eval(); + VERIFY_IS_APPROX(z, expected); + } + + { + // Obfuscate the aliasing in the RHS expression. + Eigen::Vector4f a = {1.1f, 2.2f, 3.3f, 4.4f}; + Eigen::Vector4f expected = P * (a + Eigen::Vector4f::Zero()).cwiseSqrt(); + a = P * (a + Eigen::Vector4f::Zero()).cwiseSqrt(); + VERIFY_IS_APPROX(a, expected); + } +} + EIGEN_DECLARE_TEST(permutationmatrices) { for (int i = 0; i < g_repeat; i++) { CALL_SUBTEST_1(permutationmatrices(Matrix())); @@ -182,4 +203,5 @@ EIGEN_DECLARE_TEST(permutationmatrices) { MatrixXcf(internal::random(1, EIGEN_TEST_MAX_SIZE), internal::random(1, EIGEN_TEST_MAX_SIZE)))); } CALL_SUBTEST_5(bug890()); + CALL_SUBTEST_4(test_aliasing()); }