diff --git a/Eigen/src/Core/DenseBase.h b/Eigen/src/Core/DenseBase.h index c0699f97d..f69c0b2dc 100644 --- a/Eigen/src/Core/DenseBase.h +++ b/Eigen/src/Core/DenseBase.h @@ -572,12 +572,21 @@ template class DenseBase } EIGEN_DEVICE_FUNC void reverseInPlace(); - inline DenseStlIterator begin(); - inline DenseStlIterator begin() const; - inline DenseStlIterator cbegin() const; - inline DenseStlIterator end(); - inline DenseStlIterator end() const; - inline DenseStlIterator cend() const; + typedef typename internal::conditional< (Flags&DirectAccessBit)==DirectAccessBit, + PointerBasedStlIterator, + DenseStlIterator + >::type iterator; + + typedef typename internal::conditional< (Flags&DirectAccessBit)==DirectAccessBit, + PointerBasedStlIterator, + DenseStlIterator + >::type const_iterator; + inline iterator begin(); + inline const_iterator begin() const; + inline const_iterator cbegin() const; + inline iterator end(); + inline const_iterator end() const; + inline const_iterator cend() const; inline SubVectorsProxy allCols(); inline SubVectorsProxy allCols() const; inline SubVectorsProxy allRows(); diff --git a/Eigen/src/Core/StlIterators.h b/Eigen/src/Core/StlIterators.h index c5ed2f6b9..04d3d3c8d 100644 --- a/Eigen/src/Core/StlIterators.h +++ b/Eigen/src/Core/StlIterators.h @@ -56,6 +56,58 @@ protected: Index m_index; }; +template +class PointerBasedStlIterator +{ + enum { is_lvalue = internal::is_lvalue::value }; +public: + typedef Index difference_type; + typedef typename XprType::Scalar value_type; + typedef std::random_access_iterator_tag iterator_category; + typedef typename internal::conditional::type pointer; + typedef typename internal::conditional::type reference; + + PointerBasedStlIterator() : m_ptr(0) {} + PointerBasedStlIterator(XprType& xpr, Index index) : m_incr(xpr.innerStride()) + { + m_ptr = xpr.data() + index * m_incr.value(); + } + + reference operator*() const { return *m_ptr; } + reference operator[](Index i) const { return *(m_ptr+i*m_incr.value()); } + pointer operator->() const { return m_ptr; } + + PointerBasedStlIterator& operator++() { m_ptr += m_incr.value(); return *this; } + PointerBasedStlIterator& operator--() { m_ptr -= m_incr.value(); return *this; } + + PointerBasedStlIterator operator++(int) { PointerBasedStlIterator prev(*this); operator++(); return prev;} + PointerBasedStlIterator operator--(int) { PointerBasedStlIterator prev(*this); operator--(); return prev;} + + friend PointerBasedStlIterator operator+(const PointerBasedStlIterator& a, Index b) { PointerBasedStlIterator ret(a); ret += b; return ret; } + friend PointerBasedStlIterator operator-(const PointerBasedStlIterator& a, Index b) { PointerBasedStlIterator ret(a); ret -= b; return ret; } + friend PointerBasedStlIterator operator+(Index a, const PointerBasedStlIterator& b) { PointerBasedStlIterator ret(b); ret += a; return ret; } + friend PointerBasedStlIterator operator-(Index a, const PointerBasedStlIterator& b) { PointerBasedStlIterator ret(b); ret -= a; return ret; } + + PointerBasedStlIterator& operator+=(Index b) { m_ptr += b*m_incr.value(); return *this; } + PointerBasedStlIterator& operator-=(Index b) { m_ptr -= b*m_incr.value(); return *this; } + + difference_type operator-(const PointerBasedStlIterator& other) const { + return (m_ptr - other.m_ptr)/m_incr.value(); + } + + bool operator==(const PointerBasedStlIterator& other) { return m_ptr == other.m_ptr; } + bool operator!=(const PointerBasedStlIterator& other) { return m_ptr != other.m_ptr; } + bool operator< (const PointerBasedStlIterator& other) { return m_ptr < other.m_ptr; } + bool operator<=(const PointerBasedStlIterator& other) { return m_ptr <= other.m_ptr; } + bool operator> (const PointerBasedStlIterator& other) { return m_ptr > other.m_ptr; } + bool operator>=(const PointerBasedStlIterator& other) { return m_ptr >= other.m_ptr; } + +protected: + + pointer m_ptr; + internal::variable_if_dynamic m_incr; +}; + template class DenseStlIterator : public IndexedBasedStlIteratorBase > { @@ -66,19 +118,22 @@ protected: enum { has_direct_access = (internal::traits::Flags & DirectAccessBit) ? 1 : 0, - has_write_access = internal::is_lvalue::value + is_lvalue = internal::is_lvalue::value }; typedef IndexedBasedStlIteratorBase Base; using Base::m_index; using Base::mp_xpr; - typedef typename internal::conditional::type read_only_ref_t; + // TODO currently const Transpose/Reshape expressions never returns const references, + // so lets return by value too. + //typedef typename internal::conditional::type read_only_ref_t; + typedef const value_type read_only_ref_t; public: - typedef typename internal::conditional::type pointer; - typedef typename internal::conditional::type reference; + typedef typename internal::conditional::type pointer; + typedef typename internal::conditional::type reference; DenseStlIterator() : Base() {} DenseStlIterator(XprType& xpr, Index index) : Base(xpr,index) {} @@ -94,43 +149,43 @@ void swap(IndexedBasedStlIteratorBase& a, IndexedBasedStlIterat } template -inline DenseStlIterator DenseBase::begin() +inline typename DenseBase::iterator DenseBase::begin() { EIGEN_STATIC_ASSERT_VECTOR_ONLY(Derived); - return DenseStlIterator(derived(), 0); + return iterator(derived(), 0); } template -inline DenseStlIterator DenseBase::begin() const +inline typename DenseBase::const_iterator DenseBase::begin() const { return cbegin(); } template -inline DenseStlIterator DenseBase::cbegin() const +inline typename DenseBase::const_iterator DenseBase::cbegin() const { EIGEN_STATIC_ASSERT_VECTOR_ONLY(Derived); - return DenseStlIterator(derived(), 0); + return const_iterator(derived(), 0); } template -inline DenseStlIterator DenseBase::end() +inline typename DenseBase::iterator DenseBase::end() { EIGEN_STATIC_ASSERT_VECTOR_ONLY(Derived); - return DenseStlIterator(derived(), size()); + return iterator(derived(), size()); } template -inline DenseStlIterator DenseBase::end() const +inline typename DenseBase::const_iterator DenseBase::end() const { return cend(); } template -inline DenseStlIterator DenseBase::cend() const +inline typename DenseBase::const_iterator DenseBase::cend() const { EIGEN_STATIC_ASSERT_VECTOR_ONLY(Derived); - return DenseStlIterator(derived(), size()); + return const_iterator(derived(), size()); } template diff --git a/Eigen/src/Core/util/ForwardDeclarations.h b/Eigen/src/Core/util/ForwardDeclarations.h index 8ed8dd09b..5fa7e5267 100644 --- a/Eigen/src/Core/util/ForwardDeclarations.h +++ b/Eigen/src/Core/util/ForwardDeclarations.h @@ -134,6 +134,7 @@ template class MatrixWrapper; template class SolverBase; template class InnerIterator; template class DenseStlIterator; +template class PointerBasedStlIterator; template class SubVectorsProxy; namespace internal { diff --git a/test/stl_iterators.cpp b/test/stl_iterators.cpp index f56209f07..4e04d28b7 100644 --- a/test/stl_iterators.cpp +++ b/test/stl_iterators.cpp @@ -7,6 +7,7 @@ // Public License v. 2.0. If a copy of the MPL was not distributed // with this file, You can obtain one at http://mozilla.org/MPL/2.0/. +#include #include "main.h" template< class Iterator > @@ -16,6 +17,12 @@ make_reverse_iterator( Iterator i ) return std::reverse_iterator(i); } +template +bool is_PointerBasedStlIterator(const PointerBasedStlIterator &) { return true; } + +template +bool is_DenseStlIterator(const DenseStlIterator &) { return true; } + template void test_range_for_loop(int rows=Rows, int cols=Cols) { @@ -26,10 +33,37 @@ void test_range_for_loop(int rows=Rows, int cols=Cols) typedef Matrix ColMatrixType; typedef Matrix RowMatrixType; VectorType v = VectorType::Random(rows); + const VectorType& cv(v); ColMatrixType A = ColMatrixType::Random(rows,cols); + const ColMatrixType& cA(A); RowMatrixType B = RowMatrixType::Random(rows,cols); Index i, j; + + VERIFY( is_PointerBasedStlIterator(v.begin()) ); + VERIFY( is_PointerBasedStlIterator(v.end()) ); + VERIFY( is_PointerBasedStlIterator(cv.begin()) ); + VERIFY( is_PointerBasedStlIterator(cv.end()) ); + + j = internal::random(0,A.cols()-1); + VERIFY( is_PointerBasedStlIterator(A.col(j).begin()) ); + VERIFY( is_PointerBasedStlIterator(A.col(j).end()) ); + VERIFY( is_PointerBasedStlIterator(cA.col(j).begin()) ); + VERIFY( is_PointerBasedStlIterator(cA.col(j).end()) ); + + i = internal::random(0,A.rows()-1); + VERIFY( is_PointerBasedStlIterator(A.row(i).begin()) ); + VERIFY( is_PointerBasedStlIterator(A.row(i).end()) ); + VERIFY( is_PointerBasedStlIterator(cA.row(i).begin()) ); + VERIFY( is_PointerBasedStlIterator(cA.row(i).end()) ); + + VERIFY( is_PointerBasedStlIterator(A.reshaped().begin()) ); + VERIFY( is_PointerBasedStlIterator(A.reshaped().end()) ); + VERIFY( is_PointerBasedStlIterator(cA.reshaped().begin()) ); + VERIFY( is_PointerBasedStlIterator(cA.reshaped().end()) ); + + VERIFY( is_DenseStlIterator(A.template reshaped().begin()) ); + VERIFY( is_DenseStlIterator(A.template reshaped().end()) ); #if EIGEN_HAS_CXX11 i = 0; @@ -49,6 +83,19 @@ void test_range_for_loop(int rows=Rows, int cols=Cols) i = 0; for(auto x : A.reshaped()) { VERIFY_IS_EQUAL(x,A(i++)); } + // check const_iterator + { + i = 0; + for(auto x : cv) { VERIFY_IS_EQUAL(x,v[i++]); } + + i = 0; + for(auto x : cA.reshaped()) { VERIFY_IS_EQUAL(x,A(i++)); } + + j = 0; + i = internal::random(0,A.rows()-1); + for(auto x : cA.row(i)) { VERIFY_IS_EQUAL(x,A(i,j++)); } + } + Matrix Bc = B; i = 0; for(auto x : B.reshaped()) { VERIFY_IS_EQUAL(x,Bc(i++)); } @@ -57,6 +104,41 @@ void test_range_for_loop(int rows=Rows, int cols=Cols) i = 0; for(auto& x : w) { x = v(i++); } VERIFY_IS_EQUAL(v,w); + + { + j = internal::random(0,A.cols()-1); + auto it = A.col(j).begin(); + for(i=0;i(0,A.rows()-1); + auto it = A.row(i).begin(); + for(j=0;j(0,A.cols()-1); + // this would produce a dangling pointer: + // auto it = (A+2*A).col(j).begin(); + // we need to name the temporary expression: + auto tmp = (A+2*A).col(j); + auto it = tmp.begin(); + for(i=0;i(0,A.cols()-1); + // auto it = (A+2*A).col(j).begin(); + // for(i=0;i=3) { @@ -78,16 +160,45 @@ void test_range_for_loop(int rows=Rows, int cols=Cols) VERIFY(std::is_sorted(begin(v),end(v))); VERIFY(!std::is_sorted(make_reverse_iterator(end(v)),make_reverse_iterator(begin(v)))); + // std::sort with pointer-based iterator and default increment { j = internal::random(0,A.cols()-1); // std::sort(begin(A.col(j)),end(A.col(j))); // does not compile because this returns const iterators typename ColMatrixType::ColXpr Acol = A.col(j); std::sort(begin(Acol),end(Acol)); VERIFY(std::is_sorted(Acol.cbegin(),Acol.cend())); + A.setRandom(); - // This raises an assert because this creates a pair of iterator referencing two different proxy objects: - // std::sort(A.col(j).begin(),A.col(j).end()); - // VERIFY(std::is_sorted(A.col(j).cbegin(),A.col(j).cend())); // same issue + std::sort(A.col(j).begin(),A.col(j).end()); + VERIFY(std::is_sorted(A.col(j).cbegin(),A.col(j).cend())); + A.setRandom(); + } + + // std::sort with pointer-based iterator and runtime increment + { + i = internal::random(0,A.rows()-1); + typename ColMatrixType::RowXpr Arow = A.row(i); + VERIFY_IS_EQUAL( std::distance(begin(Arow),end(Arow)), cols); + std::sort(begin(Arow),end(Arow)); + VERIFY(std::is_sorted(Arow.cbegin(),Arow.cend())); + A.setRandom(); + + std::sort(A.row(i).begin(),A.row(i).end()); + VERIFY(std::is_sorted(A.row(i).cbegin(),A.row(i).cend())); + A.setRandom(); + } + + // std::sort with generic iterator + { + auto B1 = B.reshaped(); + std::sort(begin(B1),end(B1)); + VERIFY(std::is_sorted(B1.cbegin(),B1.cend())); + B.setRandom(); + + // assertion because nested expressions are different + // std::sort(B.reshaped().begin(),B.reshaped().end()); + // VERIFY(std::is_sorted(B.reshaped().cbegin(),B.reshaped().cend())); + // B.setRandom(); } { @@ -149,6 +260,7 @@ EIGEN_DECLARE_TEST(stl_iterators) for(int i = 0; i < g_repeat; i++) { CALL_SUBTEST_1(( test_range_for_loop() )); CALL_SUBTEST_1(( test_range_for_loop() )); + CALL_SUBTEST_1(( test_range_for_loop(internal::random(5,10), internal::random(5,10)) )); CALL_SUBTEST_1(( test_range_for_loop(internal::random(10,200), internal::random(10,200)) )); } }