From 722916e19d86b224ec6c705e24d376def524154b Mon Sep 17 00:00:00 2001 From: Gael Guennebaud Date: Thu, 6 Nov 2014 09:25:26 +0100 Subject: bug #903: clean swap API regarding extra enable_if parameters, and add failtests for swap --- Eigen/src/Core/Array.h | 7 ------- Eigen/src/Core/DenseBase.h | 6 +++--- Eigen/src/Core/Matrix.h | 9 --------- Eigen/src/Core/PlainObjectBase.h | 31 ++++++++++++++++++++----------- Eigen/src/Core/Ref.h | 2 +- failtest/CMakeLists.txt | 3 +++ failtest/swap_1.cpp | 14 ++++++++++++++ failtest/swap_2.cpp | 14 ++++++++++++++ test/swap.cpp | 6 ++++++ 9 files changed, 61 insertions(+), 31 deletions(-) create mode 100644 failtest/swap_1.cpp create mode 100644 failtest/swap_2.cpp diff --git a/Eigen/src/Core/Array.h b/Eigen/src/Core/Array.h index eaee8847b..337086615 100644 --- a/Eigen/src/Core/Array.h +++ b/Eigen/src/Core/Array.h @@ -258,13 +258,6 @@ class Array *this = other; } - /** Override MatrixBase::swap() since for dynamic-sized matrices of same type it is enough to swap the - * data pointers. - */ - template - void swap(ArrayBase const & other) - { this->_swap(other.derived()); } - EIGEN_DEVICE_FUNC inline Index innerStride() const { return 1; } EIGEN_DEVICE_FUNC inline Index outerStride() const { return this->innerSize(); } diff --git a/Eigen/src/Core/DenseBase.h b/Eigen/src/Core/DenseBase.h index 14c1902e4..e81b58481 100644 --- a/Eigen/src/Core/DenseBase.h +++ b/Eigen/src/Core/DenseBase.h @@ -167,7 +167,7 @@ template class DenseBase OuterStrideAtCompileTime = internal::outer_stride_at_compile_time::ret }; - enum { ThisConstantIsPrivateInPlainObjectBase }; + enum { IsPlainObjectBase = 0 }; /** \returns the number of nonzero coefficients which is in practice the number * of stored coefficients. */ @@ -380,9 +380,9 @@ template class DenseBase */ template EIGEN_DEVICE_FUNC - void swap(const DenseBase& other, - int = OtherDerived::ThisConstantIsPrivateInPlainObjectBase) + void swap(const DenseBase& other) { + EIGEN_STATIC_ASSERT(!OtherDerived::IsPlainObjectBase,THIS_EXPRESSION_IS_NOT_A_LVALUE__IT_IS_READ_ONLY); eigen_assert(rows()==other.rows() && cols()==other.cols()); call_assignment(derived(), other.const_cast_derived(), internal::swap_assign_op()); } diff --git a/Eigen/src/Core/Matrix.h b/Eigen/src/Core/Matrix.h index bb6c75387..0b3d90786 100644 --- a/Eigen/src/Core/Matrix.h +++ b/Eigen/src/Core/Matrix.h @@ -360,15 +360,6 @@ class Matrix *this = other; } - /** \internal - * \brief Override MatrixBase::swap() since for dynamic-sized matrices - * of same type it is enough to swap the data pointers. - */ - template - EIGEN_DEVICE_FUNC - void swap(MatrixBase const & other) - { this->_swap(other.derived()); } - EIGEN_DEVICE_FUNC inline Index innerStride() const { return 1; } EIGEN_DEVICE_FUNC inline Index outerStride() const { return this->innerSize(); } diff --git a/Eigen/src/Core/PlainObjectBase.h b/Eigen/src/Core/PlainObjectBase.h index 58fe0f6e0..06e326a05 100644 --- a/Eigen/src/Core/PlainObjectBase.h +++ b/Eigen/src/Core/PlainObjectBase.h @@ -795,23 +795,33 @@ class PlainObjectBase : public internal::dense_xpr_base::type { Base::setConstant(val0); } - + template friend struct internal::matrix_swap_impl; - /** \internal generic implementation of swap for dense storage since for dynamic-sized matrices of same type it is enough to swap the - * data pointers. + public: + +#ifndef EIGEN_PARSED_BY_DOXYGEN + /** \internal + * \brief Override DenseBase::swap() since for dynamic-sized matrices + * of same type it is enough to swap the data pointers. */ template EIGEN_DEVICE_FUNC - void _swap(DenseBase const & other) + void swap(DenseBase & other) { enum { SwapPointers = internal::is_same::value && Base::SizeAtCompileTime==Dynamic }; - internal::matrix_swap_impl::run(this->derived(), other.const_cast_derived()); + internal::matrix_swap_impl::run(this->derived(), other.derived()); } - - public: -#ifndef EIGEN_PARSED_BY_DOXYGEN + + /** \internal + * \brief const version forwarded to DenseBase::swap + */ + template + EIGEN_DEVICE_FUNC + void swap(DenseBase const & other) + { Base::swap(other.derived()); } + EIGEN_DEVICE_FUNC static EIGEN_STRONG_INLINE void _check_template_params() { @@ -826,10 +836,9 @@ class PlainObjectBase : public internal::dense_xpr_base::type && (Options & (DontAlign|RowMajor)) == Options), INVALID_MATRIX_TEMPLATE_PARAMETERS) } -#endif -private: - enum { ThisConstantIsPrivateInPlainObjectBase }; + enum { IsPlainObjectBase = 1 }; +#endif }; namespace internal { diff --git a/Eigen/src/Core/Ref.h b/Eigen/src/Core/Ref.h index bc7ce9a14..6e6adbd31 100644 --- a/Eigen/src/Core/Ref.h +++ b/Eigen/src/Core/Ref.h @@ -208,7 +208,7 @@ template class Ref { EIGEN_STATIC_ASSERT(bool(internal::is_lvalue::value), THIS_EXPRESSION_IS_NOT_A_LVALUE__IT_IS_READ_ONLY); EIGEN_STATIC_ASSERT(bool(Traits::template match::MatchAtCompileTime), STORAGE_LAYOUT_DOES_NOT_MATCH); - enum { THIS_EXPRESSION_IS_NOT_A_LVALUE__IT_IS_READ_ONLY = Derived::ThisConstantIsPrivateInPlainObjectBase}; + EIGEN_STATIC_ASSERT(!Derived::IsPlainObjectBase,THIS_EXPRESSION_IS_NOT_A_LVALUE__IT_IS_READ_ONLY); Base::construct(expr.const_cast_derived()); } diff --git a/failtest/CMakeLists.txt b/failtest/CMakeLists.txt index 5343dc152..d2fea7bdc 100644 --- a/failtest/CMakeLists.txt +++ b/failtest/CMakeLists.txt @@ -38,6 +38,9 @@ ei_add_failtest("ref_3") ei_add_failtest("ref_4") ei_add_failtest("ref_5") +ei_add_failtest("swap_1") +ei_add_failtest("swap_2") + if (EIGEN_FAILTEST_FAILURE_COUNT) message(FATAL_ERROR "${EIGEN_FAILTEST_FAILURE_COUNT} out of ${EIGEN_FAILTEST_COUNT} failtests FAILED. " diff --git a/failtest/swap_1.cpp b/failtest/swap_1.cpp new file mode 100644 index 000000000..106379720 --- /dev/null +++ b/failtest/swap_1.cpp @@ -0,0 +1,14 @@ +#include "../Eigen/Core" + +using namespace Eigen; + +int main() +{ + VectorXf a(10), b(10); +#ifdef EIGEN_SHOULD_FAIL_TO_BUILD + const DenseBase &ac(a); +#else + DenseBase &ac(a); +#endif + b.swap(ac); +} diff --git a/failtest/swap_2.cpp b/failtest/swap_2.cpp new file mode 100644 index 000000000..c130ba6e4 --- /dev/null +++ b/failtest/swap_2.cpp @@ -0,0 +1,14 @@ +#include "../Eigen/Core" + +using namespace Eigen; + +int main() +{ + VectorXf a(10), b(10); + VectorXf const &ac(a); +#ifdef EIGEN_SHOULD_FAIL_TO_BUILD + b.swap(ac); +#else + b.swap(ac.const_cast_derived()); +#endif +} \ No newline at end of file diff --git a/test/swap.cpp b/test/swap.cpp index 36b353148..dc3610085 100644 --- a/test/swap.cpp +++ b/test/swap.cpp @@ -41,9 +41,15 @@ template void swap(const MatrixType& m) OtherMatrixType m3_copy = m3; // test swapping 2 matrices of same type + Scalar *d1=m1.data(), *d2=m2.data(); m1.swap(m2); VERIFY_IS_APPROX(m1,m2_copy); VERIFY_IS_APPROX(m2,m1_copy); + if(MatrixType::SizeAtCompileTime==Dynamic) + { + VERIFY(m1.data()==d2); + VERIFY(m2.data()==d1); + } m1 = m1_copy; m2 = m2_copy; -- cgit v1.2.3