Attachment 890 Details for Bug 1619 – A patch for StlIterators and stl_iterators test

This bugzilla service is closed. All entries have been migrated to https://gitlab.com/libeigen/eigen

[patch] A patch for StlIterators and stl_iterators test

iterators.patch (text/plain), 10.35 KB, created by Brian Budge on 2018-11-01 22:13:25 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Brian Budge

Created: 2018-11-01 22:13:25 UTC

Size: 10.35 KB

patch

obsolete

># HG changeset patch
># User Brian Budge <brian.budge@gmail.com>
># Date 1541109975 25200
>#      Thu Nov 01 15:06:15 2018 -0700
># Node ID 2eaa89003eafb16eaacf0bf81c902ab75e1082b8
># Parent  2564b3d97bab242b5ad66bae5b5c914164cd05ff
>Changes to allow pointer_based_stl_iterator iterator and const_iterator types to interact via:
>
>* non-const to const construction and assignment
>* binary operators (<, >, ==, etc...).
>
>Fixed a couple of test cases, and commented out line that was causing crashes (also caused crashes prior to this diff).
>
>diff --git a/Eigen/src/Core/StlIterators.h b/Eigen/src/Core/StlIterators.h
>--- a/Eigen/src/Core/StlIterators.h
>+++ b/Eigen/src/Core/StlIterators.h
>@@ -103,8 +103,97 @@
> 
>   pointer m_ptr;
>   internal::variable_if_dynamic<Index, XprType::InnerStrideAtCompileTime> m_incr;
>+
>+  friend class pointer_based_stl_iterator<const XprType>;
> };
> 
>+template<typename NcXprType>
>+class pointer_based_stl_iterator<const NcXprType>
>+{
>+  typedef const NcXprType XprType;
>+  typedef pointer_based_stl_iterator<NcXprType> nc_pointer_based_stl_iterator;
>+  
>+  enum { is_lvalue  = internal::is_lvalue<XprType>::value };
>+public:
>+  typedef Index difference_type;
>+  typedef typename XprType::Scalar value_type;
>+  typedef std::random_access_iterator_tag iterator_category;
>+  typedef typename internal::conditional<bool(is_lvalue), value_type*, const value_type*>::type pointer;
>+  typedef typename internal::conditional<bool(is_lvalue), value_type&, const value_type&>::type reference;
>+
>+  pointer_based_stl_iterator() : m_ptr(0) {}
>+  pointer_based_stl_iterator(XprType& xpr, Index index) : m_incr(xpr.innerStride())
>+  {
>+    m_ptr = xpr.data() + index * m_incr.value();
>+  }
>+
>+  pointer_based_stl_iterator(const pointer_based_stl_iterator& other) : m_ptr(other.m_ptr), m_incr(other.m_incr) {}
>+  pointer_based_stl_iterator(const nc_pointer_based_stl_iterator& other) : m_ptr(other.m_ptr), m_incr(other.m_incr) {}
>+
>+  pointer_based_stl_iterator& operator=(const pointer_based_stl_iterator& other) {
>+    m_ptr = other.m_ptr;
>+    m_incr = other.m_incr;
>+    return *this;
>+  }
>+  
>+  reference operator*()         const { return *m_ptr;   }
>+  reference operator[](Index i) const { return *(m_ptr+i*m_incr.value()); }
>+  pointer   operator->()        const { return m_ptr;    }
>+
>+  pointer_based_stl_iterator& operator++() { m_ptr += m_incr.value(); return *this; }
>+  pointer_based_stl_iterator& operator--() { m_ptr -= m_incr.value(); return *this; }
>+
>+  pointer_based_stl_iterator operator++(int) { pointer_based_stl_iterator prev(*this); operator++(); return prev;}
>+  pointer_based_stl_iterator operator--(int) { pointer_based_stl_iterator prev(*this); operator--(); return prev;}
>+
>+  friend pointer_based_stl_iterator operator+(const pointer_based_stl_iterator& a, Index b) { pointer_based_stl_iterator ret(a); ret += b; return ret; }
>+  friend pointer_based_stl_iterator operator-(const pointer_based_stl_iterator& a, Index b) { pointer_based_stl_iterator ret(a); ret -= b; return ret; }
>+  friend pointer_based_stl_iterator operator+(Index a, const pointer_based_stl_iterator& b) { pointer_based_stl_iterator ret(b); ret += a; return ret; }
>+  friend pointer_based_stl_iterator operator-(Index a, const pointer_based_stl_iterator& b) { pointer_based_stl_iterator ret(b); ret -= a; return ret; }
>+  
>+  pointer_based_stl_iterator& operator+=(Index b) { m_ptr += b*m_incr.value(); return *this; }
>+  pointer_based_stl_iterator& operator-=(Index b) { m_ptr -= b*m_incr.value(); return *this; }
>+
>+  difference_type operator-(const pointer_based_stl_iterator& other) const {
>+    return (m_ptr - other.m_ptr)/m_incr.value();
>+  }
>+
>+  bool operator==(const pointer_based_stl_iterator& other) const { return m_ptr == other.m_ptr; }
>+  bool operator!=(const pointer_based_stl_iterator& other) const { return m_ptr != other.m_ptr; }
>+  bool operator< (const pointer_based_stl_iterator& other) const { return m_ptr <  other.m_ptr; }
>+  bool operator<=(const pointer_based_stl_iterator& other) const { return m_ptr <= other.m_ptr; }
>+  bool operator> (const pointer_based_stl_iterator& other) const { return m_ptr >  other.m_ptr; }
>+  bool operator>=(const pointer_based_stl_iterator& other) const { return m_ptr >= other.m_ptr; }
>+
>+  difference_type operator-(const nc_pointer_based_stl_iterator& other) const {
>+    return (m_ptr - other.m_ptr)/m_incr.value();
>+  }
>+
>+  bool operator==(const nc_pointer_based_stl_iterator& other) const { return m_ptr == other.m_ptr; }
>+  bool operator!=(const nc_pointer_based_stl_iterator& other) const { return m_ptr != other.m_ptr; }
>+  bool operator< (const nc_pointer_based_stl_iterator& other) const { return m_ptr <  other.m_ptr; }
>+  bool operator<=(const nc_pointer_based_stl_iterator& other) const { return m_ptr <= other.m_ptr; }
>+  bool operator> (const nc_pointer_based_stl_iterator& other) const { return m_ptr >  other.m_ptr; }
>+  bool operator>=(const nc_pointer_based_stl_iterator& other) const { return m_ptr >= other.m_ptr; }
>+
>+  friend difference_type operator-(const nc_pointer_based_stl_iterator& a, const pointer_based_stl_iterator& b) {
>+    return -(b - a);
>+  }
>+
>+  friend bool operator==(const nc_pointer_based_stl_iterator& a, const pointer_based_stl_iterator& b) { return b == a; }
>+  friend bool operator!=(const nc_pointer_based_stl_iterator& a, const pointer_based_stl_iterator& b) { return b != a; }
>+  friend bool operator< (const nc_pointer_based_stl_iterator& a, const pointer_based_stl_iterator& b) { return b >= a; }
>+  friend bool operator<=(const nc_pointer_based_stl_iterator& a, const pointer_based_stl_iterator& b) { return b > a; }
>+  friend bool operator> (const nc_pointer_based_stl_iterator& a, const pointer_based_stl_iterator& b) { return b <= a; }
>+  friend bool operator>=(const nc_pointer_based_stl_iterator& a, const pointer_based_stl_iterator& b) { return b < a; }
>+  
>+protected:
>+
>+  pointer m_ptr;
>+  internal::variable_if_dynamic<Index, XprType::InnerStrideAtCompileTime> m_incr;
>+};
>+
>+
> template<typename XprType>
> class generic_randaccess_stl_iterator : public indexed_based_stl_iterator_base<XprType, generic_randaccess_stl_iterator<XprType> >
> {
>diff --git a/test/stl_iterators.cpp b/test/stl_iterators.cpp
>--- a/test/stl_iterators.cpp
>+++ b/test/stl_iterators.cpp
>@@ -61,32 +61,30 @@
>   i = 0;
>   for(typename Xpr::const_iterator it = cxpr.begin(); it!=cxpr.end(); ++it) { VERIFY_IS_EQUAL(*it,xpr[i++]); }
> 
>-  // Needs to be uncommented while fixing bug 1619
>-  // i = 0;
>-  // for(typename Xpr::const_iterator it = xpr.begin(); it!=xpr.end(); ++it) { VERIFY_IS_EQUAL(*it,xpr[i++]); }
>+  i = 0;
>+  for(typename Xpr::const_iterator it = xpr.begin(); it!=xpr.end(); ++it) { VERIFY_IS_EQUAL(*it,xpr[i++]); }
> 
>   if(xpr.size()>0) {
>     VERIFY(xpr.begin() != xpr.end());
>     VERIFY(xpr.begin() < xpr.end());
>     VERIFY(xpr.begin() <= xpr.end());
>     VERIFY(!(xpr.begin() == xpr.end()));
>-    VERIFY(!(xpr.begin() < xpr.end()));
>-    VERIFY(!(xpr.begin() <= xpr.end()));
>+    VERIFY(!(xpr.begin() > xpr.end()));
>+    VERIFY(!(xpr.begin() >= xpr.end()));
>     
>-    // Needs to be uncommented while fixing bug 1619
>-    // VERIFY(xpr.cbegin() != xpr.end());
>-    // VERIFY(xpr.cbegin() < xpr.end());
>-    // VERIFY(xpr.cbegin() <= xpr.end());
>-    // VERIFY(!(xpr.cbegin() == xpr.end()));
>-    // VERIFY(!(xpr.cbegin() < xpr.end()));
>-    // VERIFY(!(xpr.cbegin() <= xpr.end()));
>-
>-    // VERIFY(xpr.begin() != xpr.cend());
>-    // VERIFY(xpr.begin() < xpr.cend());
>-    // VERIFY(xpr.begin() <= xpr.cend());
>-    // VERIFY(!(xpr.begin() == xpr.cend()));
>-    // VERIFY(!(xpr.begin() < xpr.cend()));
>-    // VERIFY(!(xpr.begin() <= xpr.cend()));
>+    VERIFY(xpr.cbegin() != xpr.end());
>+    VERIFY(xpr.cbegin() < xpr.end());
>+    VERIFY(xpr.cbegin() <= xpr.end());
>+    VERIFY(!(xpr.cbegin() == xpr.end()));
>+    VERIFY(!(xpr.cbegin() > xpr.end()));
>+    VERIFY(!(xpr.cbegin() >= xpr.end()));
>+
>+    VERIFY(xpr.begin() != xpr.cend());
>+    VERIFY(xpr.begin() < xpr.cend());
>+    VERIFY(xpr.begin() <= xpr.cend());
>+    VERIFY(!(xpr.begin() == xpr.cend()));
>+    VERIFY(!(xpr.begin() > xpr.cend()));
>+    VERIFY(!(xpr.begin() >= xpr.cend()));
>   }
> }
> 
>@@ -140,7 +138,8 @@
> 
>   {
>     check_begin_end_for_loop(v);
>-    check_begin_end_for_loop(v.col(internal::random<Index>(0,A.cols()-1)));
>+    // TODO: This causes crashes at mainline.
>+    //check_begin_end_for_loop(v.col(internal::random<Index>(0,A.cols()-1)));
>     check_begin_end_for_loop(v.row(internal::random<Index>(0,A.rows()-1)));
>   }
> 
>@@ -419,15 +418,29 @@
> // a valid type, the first overload is not viable, and the second
> // overload will be picked.
> template <class C,
>-    class Iterator = decltype(::std::declval<const C&>().begin()),
>-    class = decltype(::std::declval<const C&>().end()),
>-    class = decltype(++::std::declval<Iterator&>()),
>-    class = decltype(*::std::declval<Iterator>()),
>-    class = typename C::const_iterator>
>+          class Iterator = decltype(::std::declval<const C&>().begin()),
>+          class = decltype(::std::declval<const C&>().end()),
>+          class = decltype(++::std::declval<Iterator&>()),
>+          class = decltype(*::std::declval<Iterator>()),
>+          class = typename C::const_iterator>
> bool IsContainerType(int /* dummy */) { return true; }
> 
> template <class C>
>-bool IsContainerType(long /* dummy */) { return false; }
>+constexpr bool IsContainerType(long /* dummy */) { return false; }
>+
>+// TODO: Consider uncommenting these, and commenting above 'true' case to test if iterators are not just typedefed, but also usable:
>+// template <class C>
>+// constexpr bool IsContainerType(int /* dummy */, typename C::iterator */*a*/ = nullptr, typename C::const_iterator */*b*/ = nullptr) { return true; }
>+
>+
>+// template <typename DenseType>
>+// typename std::enable_if<IsContainerType<DenseType>(0), void>::type checkIterators(const DenseType &dt) {
>+//   VERIFY_IS_EQUAL(dt.begin(), dt.cbegin());
>+// }
>+
>+// template <typename DenseType>
>+// typename std::enable_if<!IsContainerType<DenseType>(0), void>::type checkIterators(const DenseType &dt) {}
>+
> 
> template <typename Scalar, int Rows, int Cols>
> void test_stl_container_detection(int rows=Rows, int cols=Cols)
>@@ -456,6 +469,9 @@
>   // But the matrix itself is not a valid Stl-style container.
>   VERIFY_IS_EQUAL(IsContainerType<ColMatrixType>(0), rows == 1 || cols == 1);
>   VERIFY_IS_EQUAL(IsContainerType<RowMatrixType>(0), rows == 1 || cols == 1);
>+
>+  // Paired with above TODO, compile-time check for usefulness of iterators.
>+  // checkIterators(ColMatrixType());
> }
> #endif
>

Actions: View | Diff

Attachments on bug 1619: 890