From ba3f977946ed0ac5dde82a15a9c11784513155f2 Mon Sep 17 00:00:00 2001 From: Gael Guennebaud Date: Mon, 23 Jan 2017 22:06:08 +0100 Subject: bug #1376: add missing assertion on size mismatch with compound assignment operators (e.g., mat += mat.col(j)) --- Eigen/src/Core/AssignEvaluator.h | 25 +++++++++++++++++++++---- Eigen/src/Core/functors/AssignmentFunctors.h | 2 +- Eigen/src/SparseCore/SparseAssign.h | 5 +---- test/block.cpp | 8 ++++++++ test/diagonal.cpp | 17 +++++++++++++++++ test/sparse_basic.cpp | 10 +++++++++- 6 files changed, 57 insertions(+), 10 deletions(-) diff --git a/Eigen/src/Core/AssignEvaluator.h b/Eigen/src/Core/AssignEvaluator.h index 14400d246..7c7203ac6 100644 --- a/Eigen/src/Core/AssignEvaluator.h +++ b/Eigen/src/Core/AssignEvaluator.h @@ -701,6 +701,26 @@ protected: * Part 5 : Entry point for dense rectangular assignment ***************************************************************************/ +template +EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE +void resize_if_allowed(DstXprType &dst, const SrcXprType& src, const Functor &/*func*/) +{ + EIGEN_ONLY_USED_FOR_DEBUG(dst); + EIGEN_ONLY_USED_FOR_DEBUG(src); + eigen_assert(dst.rows() == src.rows() && dst.cols() == src.cols()); +}; + +template +EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE +void resize_if_allowed(DstXprType &dst, const SrcXprType& src, const internal::assign_op &/*func*/) +{ + Index dstRows = src.rows(); + Index dstCols = src.cols(); + if(((dst.rows()!=dstRows) || (dst.cols()!=dstCols))) + dst.resize(dstRows, dstCols); + eigen_assert(dst.rows() == dstRows && dst.cols() == dstCols); +}; + template EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE void call_dense_assignment_loop(DstXprType& dst, const SrcXprType& src, const Functor &func) { @@ -711,10 +731,7 @@ EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE void call_dense_assignment_loop(DstXprType // NOTE To properly handle A = (A*A.transpose())/s with A rectangular, // we need to resize the destination after the source evaluator has been created. - Index dstRows = src.rows(); - Index dstCols = src.cols(); - if((dst.rows()!=dstRows) || (dst.cols()!=dstCols)) - dst.resize(dstRows, dstCols); + resize_if_allowed(dst, src, func); DstEvaluatorType dstEvaluator(dst); diff --git a/Eigen/src/Core/functors/AssignmentFunctors.h b/Eigen/src/Core/functors/AssignmentFunctors.h index 9b373c783..4153b877c 100644 --- a/Eigen/src/Core/functors/AssignmentFunctors.h +++ b/Eigen/src/Core/functors/AssignmentFunctors.h @@ -28,7 +28,7 @@ template struct assign_op { { internal::pstoret(a,b); } }; -// Empty overload for void type (used by PermutationMatrix +// Empty overload for void type (used by PermutationMatrix) template struct assign_op {}; template diff --git a/Eigen/src/SparseCore/SparseAssign.h b/Eigen/src/SparseCore/SparseAssign.h index 83776645b..18352a847 100644 --- a/Eigen/src/SparseCore/SparseAssign.h +++ b/Eigen/src/SparseCore/SparseAssign.h @@ -143,10 +143,7 @@ struct Assignment dst.setZero(); internal::evaluator srcEval(src); - Index dstRows = src.rows(); - Index dstCols = src.cols(); - if((dst.rows()!=dstRows) || (dst.cols()!=dstCols)) - dst.resize(dstRows, dstCols); + resize_if_allowed(dst, src, func); internal::evaluator dstEval(dst); const Index outerEvaluationSize = (internal::evaluator::Flags&RowMajorBit) ? src.rows() : src.cols(); diff --git a/test/block.cpp b/test/block.cpp index 1eeb2da27..39565af83 100644 --- a/test/block.cpp +++ b/test/block.cpp @@ -186,6 +186,14 @@ template void block(const MatrixType& m) VERIFY_IS_EQUAL( (m1.template block<1,Dynamic>(0,1,1,0)), m1.block(0,1,1,0)); VERIFY_IS_EQUAL( ((m1*1).template block(1,0,0,1)), m1.block(1,0,0,1)); VERIFY_IS_EQUAL( ((m1*1).template block<1,Dynamic>(0,1,1,0)), m1.block(0,1,1,0)); + + if (rows>=2 && cols>=2) + { + VERIFY_RAISES_ASSERT( m1 += m1.col(0) ); + VERIFY_RAISES_ASSERT( m1 -= m1.col(0) ); + VERIFY_RAISES_ASSERT( m1.array() *= m1.col(0).array() ); + VERIFY_RAISES_ASSERT( m1.array() /= m1.col(0).array() ); + } } diff --git a/test/diagonal.cpp b/test/diagonal.cpp index ee00cad55..c1546e97d 100644 --- a/test/diagonal.cpp +++ b/test/diagonal.cpp @@ -68,6 +68,21 @@ template void diagonal(const MatrixType& m) } } +template void diagonal_assert(const MatrixType& m) { + Index rows = m.rows(); + Index cols = m.cols(); + + MatrixType m1 = MatrixType::Random(rows, cols); + + if (rows>=2 && cols>=2) + { + VERIFY_RAISES_ASSERT( m1 += m1.diagonal() ); + VERIFY_RAISES_ASSERT( m1 -= m1.diagonal() ); + VERIFY_RAISES_ASSERT( m1.array() *= m1.diagonal().array() ); + VERIFY_RAISES_ASSERT( m1.array() /= m1.diagonal().array() ); + } +} + void test_diagonal() { for(int i = 0; i < g_repeat; i++) { @@ -81,4 +96,6 @@ void test_diagonal() CALL_SUBTEST_1( diagonal(MatrixXf(internal::random(1,EIGEN_TEST_MAX_SIZE), internal::random(1,EIGEN_TEST_MAX_SIZE))) ); CALL_SUBTEST_1( diagonal(Matrix(3, 4)) ); } + + CALL_SUBTEST_1( diagonal_assert(MatrixXf(internal::random(1,EIGEN_TEST_MAX_SIZE), internal::random(1,EIGEN_TEST_MAX_SIZE))) ); } diff --git a/test/sparse_basic.cpp b/test/sparse_basic.cpp index 271e55eba..208a66f12 100644 --- a/test/sparse_basic.cpp +++ b/test/sparse_basic.cpp @@ -214,6 +214,14 @@ template void sparse_basic(const SparseMatrixType& re VERIFY_IS_APPROX(m1+=m2, refM1+=refM2); VERIFY_IS_APPROX(m1-=m2, refM1-=refM2); + if (rows>=2 && cols>=2) + { + VERIFY_RAISES_ASSERT( m1 += m1.innerVector(0) ); + VERIFY_RAISES_ASSERT( m1 -= m1.innerVector(0) ); + VERIFY_RAISES_ASSERT( refM1 -= m1.innerVector(0) ); + VERIFY_RAISES_ASSERT( refM1 += m1.innerVector(0) ); + } + // test aliasing VERIFY_IS_APPROX((m1 = -m1), (refM1 = -refM1)); VERIFY_IS_APPROX((m1 = m1.transpose()), (refM1 = refM1.transpose().eval())); @@ -428,7 +436,7 @@ template void sparse_basic(const SparseMatrixType& re m3 = m2.template triangularView(); VERIFY_IS_APPROX(m3, refMat3); - // check sparse-traingular to dense + // check sparse-triangular to dense refMat3 = m2.template triangularView(); VERIFY_IS_APPROX(refMat3, DenseMatrix(refMat2.template triangularView())); } -- cgit v1.2.3