ilist_iterator: Allow conversion between reverse and forward iterators

[MatzeB]

ping

[MatzeB]

I wasn't aware of the subtle shift-by-1 semantics when converting to std::reverse_iterator (must have missed the getReverse() comment). If we go with that semantics I would need extra std::next/std::prev calls in my code anyway which doesn't make it much simpler compared to MyIterator->getReverseIterator(). Going for semantics different from STL is not a good idea IMO.

include/llvm/ADT/ilist_iterator.h

public:
   /// Create from an ilist_node.
   explicit ilist_iterator(node_reference N) : NodePtr(&N) {}
 
   explicit ilist_iterator(pointer NP) : NodePtr(Access::getNodePtr(NP)) {}
   explicit ilist_iterator(reference NR) : NodePtr(Access::getNodePtr(&NR)) {}
   ilist_iterator() : NodePtr(nullptr) {}
 
   // This is templated so that we can allow constructing a const iterator from
-  // a nonconst iterator...
-  template <bool RHSIsConst>
+  // a nonconst iterator and switch between reverse/forward iterators...
+  template <bool RHSIsConst, bool RHSIsReverse>
   ilist_iterator(
-      const ilist_iterator<OptionsT, IsReverse, RHSIsConst> &RHS,
+      const ilist_iterator<OptionsT, RHSIsReverse, RHSIsConst> &RHS,
       typename std::enable_if<IsConst || !RHSIsConst, void *>::type = nullptr)
       : NodePtr(RHS.NodePtr) {}
 
   // This is templated so that we can allow assigning to a const iterator from
   // a nonconst iterator...
   template <bool RHSIsConst>
   typename std::enable_if<IsConst || !RHSIsConst, ilist_iterator &>::type
   operator=(const ilist_iterator<OptionsT, IsReverse, RHSIsConst> &RHS) {

unittests/ADT/SimpleIListTest.cpp

TEST(SimpleIListTest, reverse_iterator) {
   auto ReverseIter = L.rbegin();
   EXPECT_EQ(C.getReverseIterator(), ReverseIter);
   ++ReverseIter;
   EXPECT_EQ(B.getReverseIterator(), ReverseIter);
   ++ReverseIter;
   EXPECT_EQ(A.getReverseIterator(), ReverseIter);
   ++ReverseIter;
   EXPECT_EQ(L.rend(), ReverseIter);
+
+  // Check conversion between reverse/forward iterators.
+  typedef simple_ilist<Node>::reverse_iterator reverse_iterator;
+  typedef simple_ilist<Node>::iterator iterator;
+  EXPECT_EQ(reverse_iterator(L.begin()), std::prev(L.rend()));
+  EXPECT_EQ(reverse_iterator(std::prev(L.end())), L.rbegin());
+  EXPECT_EQ(iterator(L.rbegin()), std::prev(L.end()));
+  EXPECT_EQ(iterator(std::prev(L.rend())), L.begin());
+  for (Node &N : L) {
+    EXPECT_EQ(iterator(reverse_iterator(N.getIterator())),
+              N.getIterator());
+    EXPECT_EQ(reverse_iterator(iterator(N.getReverseIterator())),
+              N.getReverseIterator());
+  }
 }
 
 TEST(SimpleIListTest, eraseAndDispose) {
   simple_ilist<Node> L;
   Node A, C;
   Node *B = new Node;
   L.push_back(A);
   L.push_back(*B);