From 6ec6bf0b0d405ec8c597368d089a292d12f9b39e Mon Sep 17 00:00:00 2001 From: Gael Guennebaud Date: Tue, 15 Jan 2019 15:21:14 +0100 Subject: [PATCH] Enable visitor on empty matrices (the visitor is left unchanged), and protect min/maxCoeff(Index*,Index*) on empty matrices by an assertion (+ doc & unit tests) --- Eigen/src/Core/Redux.h | 4 ++++ Eigen/src/Core/Visitor.h | 33 +++++++++++++++++++++++++++++++++ test/zerosized.cpp | 5 +++++ 3 files changed, 42 insertions(+) diff --git a/Eigen/src/Core/Redux.h b/Eigen/src/Core/Redux.h index 720b6030c..e231a7d7d 100644 --- a/Eigen/src/Core/Redux.h +++ b/Eigen/src/Core/Redux.h @@ -397,6 +397,8 @@ public: * The template parameter \a BinaryOp is the type of the functor \a func which must be * an associative operator. Both current C++98 and C++11 functor styles are handled. * + * \warning the matrix must be not empty, otherwise an assertion is triggered. + * * \sa DenseBase::sum(), DenseBase::minCoeff(), DenseBase::maxCoeff(), MatrixBase::colwise(), MatrixBase::rowwise() */ template @@ -415,6 +417,7 @@ DenseBase::redux(const Func& func) const } /** \returns the minimum of all coefficients of \c *this. + * \warning the matrix must be not empty, otherwise an assertion is triggered. * \warning the result is undefined if \c *this contains NaN. */ template @@ -425,6 +428,7 @@ DenseBase::minCoeff() const } /** \returns the maximum of all coefficients of \c *this. + * \warning the matrix must be not empty, otherwise an assertion is triggered. * \warning the result is undefined if \c *this contains NaN. */ template diff --git a/Eigen/src/Core/Visitor.h b/Eigen/src/Core/Visitor.h index 54c1883d9..b11d87e9f 100644 --- a/Eigen/src/Core/Visitor.h +++ b/Eigen/src/Core/Visitor.h @@ -40,6 +40,14 @@ struct visitor_impl } }; +// This specialization enables visitors on empty matrices at compile-time +template +struct visitor_impl { + EIGEN_DEVICE_FUNC + static inline void run(const Derived &/*mat*/, Visitor& /*visitor*/) + {} +}; + template struct visitor_impl { @@ -98,6 +106,8 @@ protected: * * \note compared to one or two \em for \em loops, visitors offer automatic * unrolling for small fixed size matrix. + * + * \note if the matrix is empty, then the visitor is left unchanged. * * \sa minCoeff(Index*,Index*), maxCoeff(Index*,Index*), DenseBase::redux() */ @@ -106,6 +116,9 @@ template EIGEN_DEVICE_FUNC void DenseBase::visit(Visitor& visitor) const { + if(size()==0) + return; + typedef typename internal::visitor_evaluator ThisEvaluator; ThisEvaluator thisEval(derived()); @@ -196,6 +209,9 @@ struct functor_traits > { /** \fn DenseBase::minCoeff(IndexType* rowId, IndexType* colId) const * \returns the minimum of all coefficients of *this and puts in *row and *col its location. + * + * \warning the matrix must be not empty, otherwise an assertion is triggered. + * * \warning the result is undefined if \c *this contains NaN. * * \sa DenseBase::minCoeff(Index*), DenseBase::maxCoeff(Index*,Index*), DenseBase::visit(), DenseBase::minCoeff() @@ -206,6 +222,8 @@ EIGEN_DEVICE_FUNC typename internal::traits::Scalar DenseBase::minCoeff(IndexType* rowId, IndexType* colId) const { + eigen_assert(this->rows()>0 && this->cols()>0 && "you are using an empty matrix"); + internal::min_coeff_visitor minVisitor; this->visit(minVisitor); *rowId = minVisitor.row; @@ -214,6 +232,9 @@ DenseBase::minCoeff(IndexType* rowId, IndexType* colId) const } /** \returns the minimum of all coefficients of *this and puts in *index its location. + * + * \warning the matrix must be not empty, otherwise an assertion is triggered. + * * \warning the result is undefined if \c *this contains NaN. * * \sa DenseBase::minCoeff(IndexType*,IndexType*), DenseBase::maxCoeff(IndexType*,IndexType*), DenseBase::visit(), DenseBase::minCoeff() @@ -224,6 +245,8 @@ EIGEN_DEVICE_FUNC typename internal::traits::Scalar DenseBase::minCoeff(IndexType* index) const { + eigen_assert(this->rows()>0 && this->cols()>0 && "you are using an empty matrix"); + EIGEN_STATIC_ASSERT_VECTOR_ONLY(Derived) internal::min_coeff_visitor minVisitor; this->visit(minVisitor); @@ -233,6 +256,9 @@ DenseBase::minCoeff(IndexType* index) const /** \fn DenseBase::maxCoeff(IndexType* rowId, IndexType* colId) const * \returns the maximum of all coefficients of *this and puts in *row and *col its location. + * + * \warning the matrix must be not empty, otherwise an assertion is triggered. + * * \warning the result is undefined if \c *this contains NaN. * * \sa DenseBase::minCoeff(IndexType*,IndexType*), DenseBase::visit(), DenseBase::maxCoeff() @@ -243,6 +269,8 @@ EIGEN_DEVICE_FUNC typename internal::traits::Scalar DenseBase::maxCoeff(IndexType* rowPtr, IndexType* colPtr) const { + eigen_assert(this->rows()>0 && this->cols()>0 && "you are using an empty matrix"); + internal::max_coeff_visitor maxVisitor; this->visit(maxVisitor); *rowPtr = maxVisitor.row; @@ -251,6 +279,9 @@ DenseBase::maxCoeff(IndexType* rowPtr, IndexType* colPtr) const } /** \returns the maximum of all coefficients of *this and puts in *index its location. + * + * \warning the matrix must be not empty, otherwise an assertion is triggered. + * * \warning the result is undefined if \c *this contains NaN. * * \sa DenseBase::maxCoeff(IndexType*,IndexType*), DenseBase::minCoeff(IndexType*,IndexType*), DenseBase::visitor(), DenseBase::maxCoeff() @@ -261,6 +292,8 @@ EIGEN_DEVICE_FUNC typename internal::traits::Scalar DenseBase::maxCoeff(IndexType* index) const { + eigen_assert(this->rows()>0 && this->cols()>0 && "you are using an empty matrix"); + EIGEN_STATIC_ASSERT_VECTOR_ONLY(Derived) internal::max_coeff_visitor maxVisitor; this->visit(maxVisitor); diff --git a/test/zerosized.cpp b/test/zerosized.cpp index 6be136e25..07afd0f86 100644 --- a/test/zerosized.cpp +++ b/test/zerosized.cpp @@ -23,6 +23,11 @@ template void zeroReduction(const MatrixType& m) { VERIFY(!m.hasNaN()); VERIFY_RAISES_ASSERT( m.minCoeff() ); VERIFY_RAISES_ASSERT( m.maxCoeff() ); + Index i,j; + VERIFY_RAISES_ASSERT( m.minCoeff(&i,&j) ); + VERIFY_RAISES_ASSERT( m.maxCoeff(&i,&j) ); + VERIFY_RAISES_ASSERT( m.reshaped().minCoeff(&i) ); + VERIFY_RAISES_ASSERT( m.reshaped().maxCoeff(&i) ); }