I was experimenting with the new STL-compatible iterator capability using VS 2017. The following code snippet compiles correctly: const int n = 6; typedef Eigen::VectorXd V; V v(n); v << 1, 4, 6, 17, 33, 8; cout << v.transpose() << endl; for (V::iterator it = v.begin(); it != v.end(); ++it) cout << *it << ", "; However, if I replace V::iterator with V::const_iterator, VS produces these compilation errors: 1>------ Build started: Project: eigenXTests, Configuration: Debug x64 ------ 1>eigenXTests.cpp 1>c:\wgreene\tests\c++\vs2017tests\eigenxtests\eigenxtests.cpp(18): error C2440: 'initializing': cannot convert from 'Eigen::internal::pointer_based_stl_iterator<Derived>' to 'Eigen::internal::pointer_based_stl_iterator<const Derived>' 1> with 1> [ 1> Derived=Eigen::Matrix<double,-1,1,0,-1,1> 1> ] 1>c:\wgreene\tests\c++\vs2017tests\eigenxtests\eigenxtests.cpp(18): note: No user-defined-conversion operator available that can perform this conversion, or the operator cannot be called 1>c:\wgreene\tests\c++\vs2017tests\eigenxtests\eigenxtests.cpp(18): error C2679: binary '!=': no operator found which takes a right-hand operand of type 'Eigen::internal::pointer_based_stl_iterator<Derived>' (or there is no acceptable conversion) 1> with 1> [ 1> Derived=Eigen::Matrix<double,-1,1,0,-1,1> 1> ] 1>c:\wgreene\src\eigen\eigen-3.3.x\eigen\src\core\stliterators.h(96): note: could be 'bool Eigen::internal::pointer_based_stl_iterator<const Derived>::operator !=(const Eigen::internal::pointer_based_stl_iterator<const Derived> &)' 1> with 1> [ 1> Derived=Eigen::Matrix<double,-1,1,0,-1,1> 1> ] 1>c:\wgreene\tests\c++\vs2017tests\eigenxtests\eigenxtests.cpp(18): note: while trying to match the argument list '(Eigen::internal::pointer_based_stl_iterator<const Derived>, Eigen::internal::pointer_based_stl_iterator<Derived>)' 1> with 1> [ 1> Derived=Eigen::Matrix<double,-1,1,0,-1,1> 1> ] 1>Done building project "eigenXTests.vcxproj" -- FAILED. ========== Build: 0 succeeded, 1 failed, 0 up-to-date, 0 skipped ==========
I can confirm with any compiler, the problem is that our iterator is not convertible to const_iterator. You can workaround using cbegin/cend.
I can also reproduce with gcc and clang. The proper solution would be to allow conversion from non-const iterator to const_iterator. Adding this is a partial solution: typedef pointer_based_stl_iterator< typename internal::remove_const<XprType>::type > non_const_iterator; template<typename XprType2> friend class pointer_based_stl_iterator; pointer_based_stl_iterator(const non_const_iterator& other) : m_ptr(other.m_ptr), m_incr(other.m_incr) {} However, this does not cover the `operator==(iterator, const_iterator)`, etc comparisons (the other way around works directly). In glibc++ iterator comparisons are templated allowing arbitrary `__normal_iterator` to be compared, if the underlying container matches. And conversion has to be implemented for other types of iterators as well, like `Reshape<...>::iterator`.
Sure, since I already wrote the unit tests I pushed them: https://bitbucket.org/eigen/eigen/commits/2564b3d97bab242b5ad66bae5b5c914164cd05ff I won't have time to work more on it for a few days.
I have a partial fix for this now: * I fixed the pointer_based_stl_iterator to enable const_iterator = iterator and other paired interactions. * I fixed a couple of test cases that were backward. Other than one of the random initializations that was previously failing, the rest of the stl_iterator test passes. But I'm mulling over one piece that I'm not immediately sure how to solve: * When the dimensionality isn't Nx1 or 1xN, iterator is still defined. That means some checks (e.g. in googletest) are saying, "hey, I find that C::iterator is a thing". This is because their test for usable C::iterator is based on SFINAE and they do something like this: template <typename T> bool IsContainer(int /*i*/, typename T::iterator * /*a*/ = NULL, typename T::iterator * /*b*/ = NULL) { return true; } template <typename T> bool IsContainer(long /*l*/) { return false; } Then later because they detected there were iterators, they have code that uses those iterators. Honestly their check could be better, but I don't they they expected void iterators :-). Ideally, iterators would only be defined when they're usable, but I'm not coming up with any clever ways to make that happen. Basically, I think DenseBase would need two separate definitions, one with iterators (Nx1, 1xN), and one without iterators (NxM).
upgrading gtest should do the job, it is now implemented as: typedef int IsContainer; 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> IsContainer IsContainerTest(int /* dummy */) { return 0; }
Created attachment 890 [details] A patch for StlIterators and stl_iterators test I'm having a few issues getting with hg push in my fork, so just attaching a patch instead.
I guess given gtest's fix maybe it is fine for now to ignore the void iterator thing (I have some commented-out tests in my patch... please ignore those). Thanks for pointing out gtest's fix.
I applied a different patch with less boilerplate code: For pointer-based: https://bitbucket.org/eigen/eigen/commits/0810287d822f/ For indexed-based: https://bitbucket.org/eigen/eigen/commits/f1fd9e95ac1c/
-- GitLab Migration Automatic Message -- This bug has been migrated to gitlab.com's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.com/libeigen/eigen/issues/1619.