ENH: consistent reverse iterator definitions for UList and FixedList

- consistent with C++ STL conventions, the reverse iterators should
  use operator++ to transit the list from rbegin() to rend().

  The previous implementation used raw pointers, which meant that they
  had the opposite behaviour: operator-- to transit from rbegin() to
  rend().

  The updated version only has operator++ defined, thus the compiler
  should catch any possible instances where people were using the old
  (incorrect) versions.

- updated forAllReverseIters() and forAllConstReverseIters() macros to
  be consistent with new implementation and with C++ STL conventions.

applications/test/FixedList/Test-FixedList.C

@@ -38,6 +38,7 @@ See also
 #include "List.H"
 #include "IPstream.H"
 #include "OPstream.H"
+#include <numeric>
 
 using namespace Foam;
 
@@ -48,6 +49,63 @@ int main(int argc, char *argv[])
 {
     argList args(argc, argv);
 
+    if (false)
+    {
+        FixedList<string, 1> ident;
+
+        auto iter = ident.begin();
+
+        Info << iter->size() << endl;
+
+        auto riter = ident.rbegin();
+        Info << riter->size() << endl;
+
+        auto iter2 = ident.rbegin();
+
+        iter2 = iter;
+    }
+
+    {
+        FixedList<label, 15> ident;
+        std::iota(ident.begin(), ident.end(), 0);
+
+        // auto iter = ident.begin();
+        //
+        // iter += 5;
+        // Info << *iter << "< " << endl;
+        // iter -= 2;
+        // Info << *iter << "< " << endl;
+
+        // Don't yet bother with making reverse iterators random access
+        // auto riter = ident.crbegin();
+
+        // riter += 5;
+        // Info << *riter << "< " << endl;
+        // riter += 2;
+        // Info << *riter << "< " << endl;
+
+        Info<<"Ident:";
+        forAllConstIters(ident, iter)
+        {
+            Info<<" " << *iter;
+        }
+        Info<< nl;
+
+        Info<<"reverse:";
+        forAllReverseIters(ident, iter)
+        {
+            Info<<" " << *iter;
+        }
+        Info<< nl;
+
+        Info<<"const reverse:";
+        forAllConstReverseIters(ident, iter)
+        {
+            Info<<" " << *iter;
+        }
+        Info<< nl;
+    }
+
     {
         FixedList<label, 4> list1{1, 2, 3, 4};
 

applications/test/List/Test-List.C

@@ -47,6 +47,7 @@ See also
 #include "SubList.H"
 
 #include <list>
+#include <numeric>
 
 using namespace Foam;
 
@@ -79,6 +80,33 @@ int main(int argc, char *argv[])
 
     #include "setRootCase.H"
 
+    {
+        List<label> ident(15);
+        std::iota(ident.begin(), ident.end(), 0);
+
+        Info<<"Ident:";
+        forAllConstIters(ident, iter)
+        {
+            Info<<" " << *iter;
+        }
+        Info<< nl;
+
+        Info<<"reverse:";
+        forAllReverseIters(ident, iter)
+        {
+            Info<<" " << *iter;
+        }
+        Info<< nl;
+
+        Info<<"const reverse:";
+        forAllConstReverseIters(ident, iter)
+        {
+            Info<<" " << *iter;
+        }
+        Info<< nl;
+    }
+
+
     if (false)
     {
         labelList intlist(IStringStream("(0 1 2)")());
@@ -109,15 +137,15 @@ int main(int argc, char *argv[])
     forAllConstIters(list2, iter) { Info<< " " << *iter; }
     Info<< endl;
 
-    Info<< "forAllReverseConstIters(list2): ";
+    Info<< "forAllConstReverseIters(list2): ";
-    forAllReverseConstIters(list2, iter) { Info<< " " << *iter; }
+    forAllConstReverseIters(list2, iter) { Info<< " " << *iter; }
     Info<< endl;
 
     Info<< "forAllConstIters(list2): ";
     forAllIters(list2, iter) { *iter *= 2; Info<< " " << *iter; }
     Info<< endl;
 
-    Info<< "forAllReverseConstIters(list2): ";
+    Info<< "forAllReverseIters(list2): ";
     forAllReverseIters(list2, iter) { *iter *= 0.5; Info<< " " << *iter; }
     Info<< endl;
 

src/OpenFOAM/containers/Lists/FixedList/FixedList.H

@@ -338,10 +338,86 @@ public:
         inline const_iterator end() const;
 
 
-    // STL reverse_iterator
+    // Reverse iterators
 
-        //- Reverse iterator for reverse traversal of FixedList
+        //- Generic const/non-const reverse iterator
-        typedef T* reverse_iterator;
+        template<bool Const>
+        class reverse_iterator_base
+        {
+        public:
+            //- The const/non-const type for entries
+            typedef typename std::conditional
+                <Const, const T, T>::type value_type;
+
+            //- A pointer to a const/non-const entry
+            typedef value_type* pointer;
+
+            //- A reference to a const/non-const entry
+            typedef value_type& reference;
+
+
+        private:
+
+            //- The element pointer
+            pointer ptr_;
+
+        public:
+
+            //- Construct null or from list element pointer
+            inline reverse_iterator_base(pointer ptr = nullptr)
+            :
+                ptr_(ptr)
+            {}
+
+            //- Copy construct
+            inline reverse_iterator_base(const reverse_iterator_base& iter)
+            :
+                ptr_(iter.ptr_)
+            {}
+
+
+            //- Reverse increment
+            inline void operator++()
+            {
+                --ptr_;
+            }
+
+            //- Reverse increment
+            inline reverse_iterator_base operator++(int)
+            {
+                reverse_iterator_base old(*this);
+                --ptr_;
+                return old;
+            }
+
+            //- Dereference iterator
+            reference operator*() const
+            {
+                return *ptr_;
+            }
+
+            //- Dereference iterator
+            pointer operator->() const
+            {
+                return ptr_;
+            }
+
+            //- Equality
+            bool operator==(const reverse_iterator_base& iter) const
+            {
+                return ptr_ == iter.ptr_;
+            }
+
+            //- inequality
+            bool operator!=(const reverse_iterator_base& iter) const
+            {
+                return ptr_ != iter.ptr_;
+            }
+        };
+
+
+        //- STL reverse_iterator
+        typedef reverse_iterator_base<false> reverse_iterator;
 
         //- Return reverse_iterator to begin reverse traversing the FixedList
         inline reverse_iterator rbegin();
@@ -350,10 +426,8 @@ public:
         inline reverse_iterator rend();
 
 
-    // STL const_reverse_iterator
+        //- STL const reverse iterator
-
+        typedef reverse_iterator_base<true> const_reverse_iterator;
-        //- Reverse iterator for reverse traversal of constant FixedList
-        typedef const T* const_reverse_iterator;
 
         //- Return const_reverse_iterator to begin reverse traversing FixedList
         inline const_reverse_iterator crbegin() const;

src/OpenFOAM/containers/Lists/FixedList/FixedListI.H

@@ -465,50 +465,50 @@ Foam::FixedList<T, Size>::cend() const
 
 
 template<class T, unsigned Size>
-inline typename Foam::FixedList<T, Size>::iterator
+inline typename Foam::FixedList<T, Size>::reverse_iterator
 Foam::FixedList<T, Size>::rbegin()
 {
-    return &v_[Size-1];
+    return reverse_iterator(&v_[Size-1]);
 }
 
 
 template<class T, unsigned Size>
-inline typename Foam::FixedList<T, Size>::const_iterator
+inline typename Foam::FixedList<T, Size>::const_reverse_iterator
 Foam::FixedList<T, Size>::rbegin() const
 {
-    return &v_[Size-1];
+    return const_reverse_iterator(&v_[Size-1]);
 }
 
 
 template<class T, unsigned Size>
-inline typename Foam::FixedList<T, Size>::const_iterator
+inline typename Foam::FixedList<T, Size>::const_reverse_iterator
 Foam::FixedList<T, Size>::crbegin() const
 {
-    return &v_[Size-1];
+    return const_reverse_iterator(&v_[Size-1]);
 }
 
 
 template<class T, unsigned Size>
-inline typename Foam::FixedList<T, Size>::iterator
+inline typename Foam::FixedList<T, Size>::reverse_iterator
 Foam::FixedList<T, Size>::rend()
 {
-    return &v_[-1];
+    return reverse_iterator(&v_[-1]);
 }
 
 
 template<class T, unsigned Size>
-inline typename Foam::FixedList<T, Size>::const_iterator
+inline typename Foam::FixedList<T, Size>::const_reverse_iterator
 Foam::FixedList<T, Size>::rend() const
 {
-    return &v_[-1];
+    return const_reverse_iterator(&v_[-1]);
 }
 
 
 template<class T, unsigned Size>
-inline typename Foam::FixedList<T, Size>::const_iterator
+inline typename Foam::FixedList<T, Size>::const_reverse_iterator
 Foam::FixedList<T, Size>::crend() const
 {
-    return &v_[-1];
+    return const_reverse_iterator(&v_[-1]);
 }
 
 

src/OpenFOAM/containers/Lists/UList/UList.H

@@ -383,11 +383,94 @@ public:
         //- Return const_iterator to end traversing the constant UList
         inline const_iterator end() const;
 
+    // Reverse iterators
 
-    // STL reverse_iterator
+        //- Generic const/non-const reverse iterator
+        template<bool Const>
+        class reverse_iterator_base
+        {
+        public:
+            //- The const/non-const type for entries
+            typedef typename std::conditional
+                <Const, const T, T>::type value_type;
 
-        //- Reverse iterator for reverse traversal of UList
+            //- A pointer to a const/non-const entry
-        typedef T* reverse_iterator;
+            typedef value_type* pointer;
+
+            //- A reference to a const/non-const entry
+            typedef value_type& reference;
+
+
+        private:
+
+            //- The element pointer
+            pointer ptr_;
+
+
+        public:
+
+            //- Construct null or from list element pointer
+            inline reverse_iterator_base(pointer ptr = nullptr)
+            :
+                ptr_(ptr)
+            {}
+
+
+            //- Copy construct
+            inline reverse_iterator_base(const reverse_iterator_base& iter)
+            :
+                ptr_(iter.ptr_)
+            {}
+
+
+            //- Reverse increment
+            inline void operator++()
+            {
+                --ptr_;
+            }
+
+            //- Reverse increment
+            inline reverse_iterator_base operator++(int)
+            {
+                reverse_iterator_base old(*this);
+                --ptr_;
+                return old;
+            }
+
+            //- Reverse increase
+            inline void operator+=(int n)
+            {
+                ptr_ -= n;
+            }
+
+            //- Dereference iterator
+            reference operator*() const
+            {
+                return *ptr_;
+            }
+
+            //- Dereference iterator
+            pointer operator->() const
+            {
+                return ptr_;
+            }
+
+            //- Equality
+            bool operator==(const reverse_iterator_base& iter) const
+            {
+                return ptr_ == iter.ptr_;
+            }
+
+            //- inequality
+            bool operator!=(const reverse_iterator_base& iter) const
+            {
+                return ptr_ != iter.ptr_;
+            }
+        };
+
+
+        //- STL reverse_iterator
+        typedef reverse_iterator_base<false> reverse_iterator;
 
         //- Return reverse_iterator to begin reverse traversing the UList
         inline reverse_iterator rbegin();
@@ -396,10 +479,8 @@ public:
         inline reverse_iterator rend();
 
 
-    // STL const_reverse_iterator
+        //- STL const reverse iterator
-
+        typedef reverse_iterator_base<true> const_reverse_iterator;
-        //- Reverse iterator for reverse traversal of constant UList
-        typedef const T* const_reverse_iterator;
 
         //- Return const_reverse_iterator to begin reverse traversing the UList
         inline const_reverse_iterator crbegin() const;

src/OpenFOAM/containers/Lists/UList/UListI.H

@@ -286,45 +286,45 @@ Foam::UList<T>::cend() const
 }
 
 template<class T>
-inline typename Foam::UList<T>::iterator
+inline typename Foam::UList<T>::reverse_iterator
 Foam::UList<T>::rbegin()
 {
-    return &v_[size_-1];
+    return reverse_iterator(&v_[size_-1]);
 }
 
 template<class T>
-inline typename Foam::UList<T>::const_iterator
+inline typename Foam::UList<T>::const_reverse_iterator
 Foam::UList<T>::rbegin() const
 {
-    return &v_[size_-1];
+    return const_reverse_iterator(&v_[size_-1]);
 }
 
 template<class T>
-inline typename Foam::UList<T>::const_iterator
+inline typename Foam::UList<T>::const_reverse_iterator
 Foam::UList<T>::crbegin() const
 {
-    return &v_[size_-1];
+    return const_reverse_iterator(&v_[size_-1]);
 }
 
 template<class T>
-inline typename Foam::UList<T>::iterator
+inline typename Foam::UList<T>::reverse_iterator
 Foam::UList<T>::rend()
 {
-    return &v_[-1];
+    return reverse_iterator(&v_[-1]);
 }
 
 template<class T>
-inline typename Foam::UList<T>::const_iterator
+inline typename Foam::UList<T>::const_reverse_iterator
 Foam::UList<T>::rend() const
 {
-    return &v_[-1];
+    return const_reverse_iterator(&v_[-1]);
 }
 
 template<class T>
-inline typename Foam::UList<T>::const_iterator
+inline typename Foam::UList<T>::const_reverse_iterator
 Foam::UList<T>::crend() const
 {
-    return &v_[-1];
+    return const_reverse_iterator(&v_[-1]);
 }
 
 

src/OpenFOAM/include/stdFoam.H

@@ -207,13 +207,13 @@ constexpr auto crend(const C& c) -> decltype(c.rend())
 //      statements;
 //  }
 //  \endcode
-//  \sa forAllReverseConstIters
+//  \sa forAllConstReverseIters
 #define forAllReverseIters(container,iter)                                     \
     for                                                                        \
     (                                                                          \
         auto iter = stdFoam::rbegin(container);                                \
         iter != stdFoam::rend(container);                                      \
-        --iter                                                                 \
+        ++iter                                                                 \
     )
 
 
@@ -226,12 +226,12 @@ constexpr auto crend(const C& c) -> decltype(c.rend())
 //  }
 //  \endcode
 //  \sa forAllReverseIters
-#define forAllReverseConstIters(container,iter)                                \
+#define forAllConstReverseIters(container,iter)                                \
     for                                                                        \
     (                                                                          \
         auto iter = stdFoam::crbegin(container);                               \
         iter != stdFoam::crend(container);                                     \
-        --iter                                                                 \
+        ++iter                                                                 \
     )