mirror of
https://gitlab.com/libeigen/eigen.git
synced 2024-12-15 07:10:37 +08:00
bug #1432: fix conservativeResize for non-relocatable scalar types. For those we need to by-pass realloc routines and fall-back to allocate as new - copy - delete. The remaining problem is that we don't have any mechanism to accurately determine whether a type is relocatable or not, so currently let's be super conservative using either RequireInitialization or std::is_trivially_copyable
This commit is contained in:
parent
053ed97c72
commit
863580fe88
@ -921,13 +921,19 @@ namespace internal {
|
||||
template <typename Derived, typename OtherDerived, bool IsVector>
|
||||
struct conservative_resize_like_impl
|
||||
{
|
||||
#if EIGEN_HAS_TYPE_TRAITS
|
||||
static const bool IsRelocatable = std::is_trivially_copyable<typename Derived::Scalar>::value;
|
||||
#else
|
||||
static const bool IsRelocatable = NumTraits<typename Derived::Scalar>::RequireInitialization;
|
||||
#endif
|
||||
static void run(DenseBase<Derived>& _this, Index rows, Index cols)
|
||||
{
|
||||
if (_this.rows() == rows && _this.cols() == cols) return;
|
||||
EIGEN_STATIC_ASSERT_DYNAMIC_SIZE(Derived)
|
||||
|
||||
if ( ( Derived::IsRowMajor && _this.cols() == cols) || // row-major and we change only the number of rows
|
||||
(!Derived::IsRowMajor && _this.rows() == rows) ) // column-major and we change only the number of columns
|
||||
if ( IsRelocatable
|
||||
&& (( Derived::IsRowMajor && _this.cols() == cols) || // row-major and we change only the number of rows
|
||||
(!Derived::IsRowMajor && _this.rows() == rows) )) // column-major and we change only the number of columns
|
||||
{
|
||||
internal::check_rows_cols_for_overflow<Derived::MaxSizeAtCompileTime>::run(rows, cols);
|
||||
_this.derived().m_storage.conservativeResize(rows*cols,rows,cols);
|
||||
@ -955,8 +961,9 @@ struct conservative_resize_like_impl
|
||||
EIGEN_STATIC_ASSERT_DYNAMIC_SIZE(Derived)
|
||||
EIGEN_STATIC_ASSERT_DYNAMIC_SIZE(OtherDerived)
|
||||
|
||||
if ( ( Derived::IsRowMajor && _this.cols() == other.cols()) || // row-major and we change only the number of rows
|
||||
(!Derived::IsRowMajor && _this.rows() == other.rows()) ) // column-major and we change only the number of columns
|
||||
if ( IsRelocatable &&
|
||||
(( Derived::IsRowMajor && _this.cols() == other.cols()) || // row-major and we change only the number of rows
|
||||
(!Derived::IsRowMajor && _this.rows() == other.rows()) )) // column-major and we change only the number of columns
|
||||
{
|
||||
const Index new_rows = other.rows() - _this.rows();
|
||||
const Index new_cols = other.cols() - _this.cols();
|
||||
@ -984,13 +991,18 @@ template <typename Derived, typename OtherDerived>
|
||||
struct conservative_resize_like_impl<Derived,OtherDerived,true>
|
||||
: conservative_resize_like_impl<Derived,OtherDerived,false>
|
||||
{
|
||||
using conservative_resize_like_impl<Derived,OtherDerived,false>::run;
|
||||
typedef conservative_resize_like_impl<Derived,OtherDerived,false> Base;
|
||||
using Base::run;
|
||||
using Base::IsRelocatable;
|
||||
|
||||
static void run(DenseBase<Derived>& _this, Index size)
|
||||
{
|
||||
const Index new_rows = Derived::RowsAtCompileTime==1 ? 1 : size;
|
||||
const Index new_cols = Derived::RowsAtCompileTime==1 ? size : 1;
|
||||
_this.derived().m_storage.conservativeResize(size,new_rows,new_cols);
|
||||
if(IsRelocatable)
|
||||
_this.derived().m_storage.conservativeResize(size,new_rows,new_cols);
|
||||
else
|
||||
Base::run(_this.derived(), new_rows, new_cols);
|
||||
}
|
||||
|
||||
static void run(DenseBase<Derived>& _this, const DenseBase<OtherDerived>& other)
|
||||
@ -1001,7 +1013,10 @@ struct conservative_resize_like_impl<Derived,OtherDerived,true>
|
||||
|
||||
const Index new_rows = Derived::RowsAtCompileTime==1 ? 1 : other.rows();
|
||||
const Index new_cols = Derived::RowsAtCompileTime==1 ? other.cols() : 1;
|
||||
_this.derived().m_storage.conservativeResize(other.size(),new_rows,new_cols);
|
||||
if(IsRelocatable)
|
||||
_this.derived().m_storage.conservativeResize(other.size(),new_rows,new_cols);
|
||||
else
|
||||
Base::run(_this.derived(), new_rows, new_cols);
|
||||
|
||||
if (num_new_elements > 0)
|
||||
_this.tail(num_new_elements) = other.tail(num_new_elements);
|
||||
|
@ -22,7 +22,7 @@ struct my_exception
|
||||
|
||||
// An AnnoyingScalar is a pseudo scalar type that:
|
||||
// - can randomly through an exception in operator +
|
||||
// - randomly allocate on the heap or initialize a reference to itself making it non trivially copyable, nor movable.
|
||||
// - randomly allocate on the heap or initialize a reference to itself making it non trivially copyable, nor movable, nor relocatable.
|
||||
|
||||
class AnnoyingScalar
|
||||
{
|
||||
|
@ -10,6 +10,7 @@
|
||||
#include "main.h"
|
||||
|
||||
#include <Eigen/Core>
|
||||
#include "AnnoyingScalar.h"
|
||||
|
||||
using namespace Eigen;
|
||||
|
||||
@ -109,6 +110,30 @@ void run_vector_tests()
|
||||
}
|
||||
}
|
||||
|
||||
// Basic memory leak check with a non-copyable scalar type
|
||||
template<int> void noncopyable()
|
||||
{
|
||||
typedef Eigen::Matrix<AnnoyingScalar,Dynamic,1> VectorType;
|
||||
typedef Eigen::Matrix<AnnoyingScalar,Dynamic,Dynamic> MatrixType;
|
||||
|
||||
{
|
||||
AnnoyingScalar::dont_throw = true;
|
||||
int n = 50;
|
||||
VectorType v0(n), v1(n);
|
||||
MatrixType m0(n,n), m1(n,n), m2(n,n);
|
||||
v0.setOnes(); v1.setOnes();
|
||||
m0.setOnes(); m1.setOnes(); m2.setOnes();
|
||||
VERIFY(m0==m1);
|
||||
m0.conservativeResize(2*n,2*n);
|
||||
VERIFY(m0.topLeftCorner(n,n) == m1);
|
||||
|
||||
VERIFY(v0.head(n) == v1);
|
||||
v0.conservativeResize(2*n);
|
||||
VERIFY(v0.head(n) == v1);
|
||||
}
|
||||
VERIFY(AnnoyingScalar::instances==0 && "global memory leak detected in noncopyable");
|
||||
}
|
||||
|
||||
EIGEN_DECLARE_TEST(conservative_resize)
|
||||
{
|
||||
for(int i=0; i<g_repeat; ++i)
|
||||
@ -122,12 +147,16 @@ EIGEN_DECLARE_TEST(conservative_resize)
|
||||
CALL_SUBTEST_4((run_matrix_tests<std::complex<float>, Eigen::RowMajor>()));
|
||||
CALL_SUBTEST_4((run_matrix_tests<std::complex<float>, Eigen::ColMajor>()));
|
||||
CALL_SUBTEST_5((run_matrix_tests<std::complex<double>, Eigen::RowMajor>()));
|
||||
CALL_SUBTEST_6((run_matrix_tests<std::complex<double>, Eigen::ColMajor>()));
|
||||
CALL_SUBTEST_5((run_matrix_tests<std::complex<double>, Eigen::ColMajor>()));
|
||||
|
||||
CALL_SUBTEST_1((run_vector_tests<int>()));
|
||||
CALL_SUBTEST_2((run_vector_tests<float>()));
|
||||
CALL_SUBTEST_3((run_vector_tests<double>()));
|
||||
CALL_SUBTEST_4((run_vector_tests<std::complex<float> >()));
|
||||
CALL_SUBTEST_5((run_vector_tests<std::complex<double> >()));
|
||||
|
||||
AnnoyingScalar::dont_throw = true;
|
||||
CALL_SUBTEST_6(( run_vector_tests<AnnoyingScalar>() ));
|
||||
CALL_SUBTEST_6(( noncopyable<0>() ));
|
||||
}
|
||||
}
|
||||
|
@ -8,6 +8,7 @@
|
||||
// with this file, You can obtain one at http://mozilla.org/MPL/2.0/.
|
||||
|
||||
#include "sparse.h"
|
||||
#include "AnnoyingScalar.h"
|
||||
|
||||
template<typename T>
|
||||
typename Eigen::internal::enable_if<(T::Flags&RowMajorBit)==RowMajorBit, typename T::RowXpr>::type
|
||||
@ -31,6 +32,7 @@ template<typename SparseMatrixType> void sparse_block(const SparseMatrixType& re
|
||||
const Index outer = ref.outerSize();
|
||||
|
||||
typedef typename SparseMatrixType::Scalar Scalar;
|
||||
typedef typename SparseMatrixType::RealScalar RealScalar;
|
||||
typedef typename SparseMatrixType::StorageIndex StorageIndex;
|
||||
|
||||
double density = (std::max)(8./(rows*cols), 0.01);
|
||||
@ -164,14 +166,14 @@ template<typename SparseMatrixType> void sparse_block(const SparseMatrixType& re
|
||||
{
|
||||
VERIFY(j==numext::real(m3.innerVector(j).nonZeros()));
|
||||
if(j>0)
|
||||
VERIFY(j==numext::real(m3.innerVector(j).lastCoeff()));
|
||||
VERIFY(RealScalar(j)==numext::real(m3.innerVector(j).lastCoeff()));
|
||||
}
|
||||
m3.makeCompressed();
|
||||
for(Index j=0; j<(std::min)(outer, inner); ++j)
|
||||
{
|
||||
VERIFY(j==numext::real(m3.innerVector(j).nonZeros()));
|
||||
if(j>0)
|
||||
VERIFY(j==numext::real(m3.innerVector(j).lastCoeff()));
|
||||
VERIFY(RealScalar(j)==numext::real(m3.innerVector(j).lastCoeff()));
|
||||
}
|
||||
|
||||
VERIFY(m3.innerVector(j0).nonZeros() == m3.transpose().innerVector(j0).nonZeros());
|
||||
@ -313,5 +315,8 @@ EIGEN_DECLARE_TEST(sparse_block)
|
||||
|
||||
CALL_SUBTEST_4(( sparse_block(SparseMatrix<double,ColMajor,short int>(short(r), short(c))) ));
|
||||
CALL_SUBTEST_4(( sparse_block(SparseMatrix<double,RowMajor,short int>(short(r), short(c))) ));
|
||||
|
||||
AnnoyingScalar::dont_throw = true;
|
||||
CALL_SUBTEST_5(( sparse_block(SparseMatrix<AnnoyingScalar>(r,c)) ));
|
||||
}
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user