Simplify Iterators
authorRoland Schulz <roland.schulz@intel.com>
Wed, 1 Jul 2020 16:27:30 +0000 (16:27 +0000)
committerPaul Bauer <paul.bauer.q@gmail.com>
Wed, 1 Jul 2020 16:27:30 +0000 (16:27 +0000)
src/gromacs/selection/symrec.h
src/gromacs/simd/simd_memory.h
src/gromacs/topology/mtop_util.cpp
src/gromacs/topology/mtop_util.h
src/gromacs/utility/enumerationhelpers.h

index 5f82126af1c070b92393a93e50c8545a6bf53928..5feeeac876cbaf67ecbf8cedba0babcd2b1bd98e 100644 (file)
@@ -48,6 +48,8 @@
 #include <iterator>
 #include <string>
 
+#include <boost/stl_interfaces/iterator_interface.hpp>
+
 #include "gromacs/utility/classhelpers.h"
 
 #include "selelem.h"
@@ -126,9 +128,9 @@ private:
 
 /*! \internal
  * \brief
- * Input iterator for iterating symbols of a given type.
+ * Forward iterator for iterating symbols of a given type.
  *
- * Behaves as standard C++ input iterator.  To get an iterator, call
+ * Behaves as standard C++ forward iterator.  To get an iterator, call
  * SelectionParserSymbolTable::beginIterator().  Each time the iterator is
  * incremented, it moves to the next symbol of the type given when the iterator
  * was created.  When there are no more symbols, the iterator will equal
@@ -142,20 +144,13 @@ private:
  *
  * \ingroup module_selection
  */
-class SelectionParserSymbolIterator
+class SelectionParserSymbolIterator :
+    public boost::stl_interfaces::iterator_interface<SelectionParserSymbolIterator, std::forward_iterator_tag, const SelectionParserSymbol>
 {
-public:
-    /*! \name Iterator type traits
-     * Satisfies the requirements for STL input iterator.
-     * \{
-     */
-    using iterator_category = std::input_iterator_tag;
-    using value_type        = const SelectionParserSymbol;
-    using difference_type   = std::ptrdiff_t;
-    using pointer           = const SelectionParserSymbol*;
-    using reference         = const SelectionParserSymbol&;
-    //! \}
+    using Base =
+            boost::stl_interfaces::iterator_interface<SelectionParserSymbolIterator, std::forward_iterator_tag, const SelectionParserSymbol>;
 
+public:
     //! Creates an independent copy of an iterator.
     SelectionParserSymbolIterator(const SelectionParserSymbolIterator& other);
     ~SelectionParserSymbolIterator();
@@ -165,21 +160,11 @@ public:
 
     //! Equality comparison for iterators.
     bool operator==(const SelectionParserSymbolIterator& other) const;
-    //! Inequality comparison for iterators.
-    bool operator!=(const SelectionParserSymbolIterator& other) const { return !operator==(other); }
     //! Dereferences the iterator.
     reference operator*() const;
-    //! Dereferences the iterator.
-    pointer operator->() const { return &operator*(); }
     //! Moves the iterator to the next symbol.
     SelectionParserSymbolIterator& operator++();
-    //! Moves the iterator to the next symbol.
-    SelectionParserSymbolIterator operator++(int)
-    {
-        SelectionParserSymbolIterator tmp(*this);
-                                      operator++();
-        return tmp;
-    }
+    using Base::                   operator++;
 
 private:
     class Impl;
index 0244284df95d7618c5217021118c639e960b2a64..aa4a3c8da81ef788c224b1602288f4346516adf7 100644 (file)
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2017,2018,2019, by the GROMACS development team, led by
+ * Copyright (c) 2017,2018,2019,2020, by the GROMACS development team, led by
  * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl,
  * and including many others, as listed in the AUTHORS file in the
  * top-level source directory and at http://www.gromacs.org.
@@ -92,73 +92,30 @@ private:
 };
 
 template<typename T>
-class SimdIterator
+class SimdIterator :
+    public boost::stl_interfaces::iterator_interface<SimdIterator<T>, std::random_access_iterator_tag, T, SimdReference<T>>
 {
-public:
-    //! Type for representing size of the container.
-    using size_type = size_t;
-    //! Type for representing difference between two container indices.
-    using difference_type = std::ptrdiff_t;
-    //! Type of values stored in the container.
-    using value_type = T;
-    //! Pointer to a container element.
-    using pointer = typename SimdTraits<T>::type*;
-    //! Reference to a container element.
-    using reference = internal::SimdReference<T>;
+    using Base =
+            boost::stl_interfaces::iterator_interface<SimdIterator<T>, std::random_access_iterator_tag, T, SimdReference<T>>;
+    // pointer is T*
+    using DataPointer = typename SimdTraits<T>::type*;
 
-    explicit SimdIterator(pointer p = 0) : p_(p)
+public:
+    explicit SimdIterator(DataPointer p = 0) : p_(p)
     {
-        GMX_ASSERT((reinterpret_cast<size_type>(p) / sizeof(*p)) % simdWidth == 0,
+        GMX_ASSERT((reinterpret_cast<size_t>(p) / sizeof(*p)) % simdWidth == 0,
                    "Trying to create aligned iterator for non aligned address.");
     }
-    SimdIterator& operator++()
-    {
-        p_ += simdWidth;
-        return *this;
-    }
-    SimdIterator operator++(int)
-    {
-        SimdIterator retval = *this;
-        ++(*this);
-        return retval;
-    }
-    SimdIterator& operator--()
-    {
-        p_ -= simdWidth;
-        return *this;
-    }
-    SimdIterator operator--(int)
-    {
-        SimdIterator retval = *this;
-        --(*this);
-        return retval;
-    }
-    SimdIterator& operator+=(difference_type d)
+    SimdIterator& operator+=(typename Base::difference_type d)
     {
         p_ += simdWidth * d;
         return *this;
     }
-    SimdIterator& operator-=(difference_type d)
-    {
-        p_ -= simdWidth * d;
-        return *this;
-    }
-    SimdIterator operator+(difference_type d) { return SimdIterator(p_ + simdWidth * d); }
-    SimdIterator operator-(difference_type d) { return SimdIterator(p_ - simdWidth * d); }
-
-    difference_type operator-(SimdIterator o) { return (p_ - o.p_) / simdWidth; }
-
-    bool operator==(SimdIterator other) const { return p_ == other.p_; }
-    bool operator!=(SimdIterator other) const { return p_ != other.p_; }
-    bool operator<(SimdIterator other) const { return p_ < other.p_; }
-    bool operator>(SimdIterator other) const { return p_ > other.p_; }
-    bool operator<=(SimdIterator other) const { return p_ <= other.p_; }
-    bool operator>=(SimdIterator other) const { return p_ >= other.p_; }
-
-    reference operator*() const { return reference(p_); }
+    typename Base::difference_type operator-(SimdIterator o) { return (p_ - o.p_) / simdWidth; }
+    typename Base::reference       operator*() const { return typename Base::reference(p_); }
 
 private:
-    pointer              p_;
+    DataPointer          p_;
     static constexpr int simdWidth = SimdTraits<T>::width;
 };
 
index 444406ae30ff55244fe9c908ce6a882db5739b6e..0c2006def870cacd50ec5c18ce0f75b3826db334 100644 (file)
@@ -231,23 +231,11 @@ AtomIterator& AtomIterator::operator++()
     return *this;
 }
 
-AtomIterator AtomIterator::operator++(int)
-{
-    AtomIterator temp = *this;
-    ++(*this);
-    return temp;
-}
-
 bool AtomIterator::operator==(const AtomIterator& o) const
 {
     return mtop_ == o.mtop_ && globalAtomNumber_ == o.globalAtomNumber_;
 }
 
-bool AtomIterator::operator!=(const AtomIterator& o) const
-{
-    return !(*this == o);
-}
-
 const t_atom& AtomProxy::atom() const
 {
     return it_->atoms_->atom[it_->localAtomNumber_];
index c01d2d1d6a01305d22b6f78a5ba6c1d04508a41c..e027bfef7c138a2c2056227c0885297c1d39fa59 100644 (file)
@@ -43,6 +43,8 @@
 #include <array>
 #include <vector>
 
+#include <boost/stl_interfaces/iterator_interface.hpp>
+
 #include "gromacs/topology/topology.h"
 #include "gromacs/utility/basedefinitions.h"
 
@@ -104,43 +106,28 @@ private:
     const AtomIterator* it_;
 };
 
-//! Wrapper around proxy object to implement operator->
-template<typename T>
-class ProxyPtr
-{
-public:
-    //! Construct with proxy object.
-    ProxyPtr(T t) : t_(t) {}
-    //! Member of pointer operator.
-    T* operator->() { return &t_; }
-
-private:
-    T t_;
-};
-
 /*! \brief
  * Object that allows looping over all atoms in an mtop.
  */
-class AtomIterator
+class AtomIterator :
+    public boost::stl_interfaces::proxy_iterator_interface<AtomIterator, std::forward_iterator_tag, t_atom, AtomProxy>
 {
+    using Base =
+            boost::stl_interfaces::proxy_iterator_interface<AtomIterator, std::forward_iterator_tag, t_atom, AtomProxy>;
+
 public:
     //! Construct from topology and optionalally a global atom number.
     explicit AtomIterator(const gmx_mtop_t& mtop, int globalAtomNumber = 0);
 
     //! Prefix increment.
     AtomIterator& operator++();
-    //! Postfix increment.
-    AtomIterator operator++(int);
+    using Base::  operator++;
 
     //! Equality comparison.
     bool operator==(const AtomIterator& o) const;
-    //! Non-equal comparison.
-    bool operator!=(const AtomIterator& o) const;
 
     //! Dereference operator. Returns proxy.
     AtomProxy operator*() const { return { this }; }
-    //! Member of pointer operator.
-    ProxyPtr<AtomProxy> operator->() const { return { this }; }
 
 private:
     //! Global topology.
index e500ecb0aff698f27fa6c0a9d5888b068d2898ce..e9119e27ddfc5e4caf6e45c688bc88d5b59ea309 100644 (file)
 #include <iterator>
 #include <type_traits>
 
+#if __has_include(<boost/stl_interfaces/iterator_interface.hpp>)
+#    include <boost/stl_interfaces/iterator_interface.hpp>
+#else // fallback for installed headers
+#    include <gromacs/external/boost/stl_interfaces/iterator_interface.hpp>
+#endif
+
 #include "gromacs/utility/gmxassert.h"
 
 namespace gmx
@@ -107,78 +113,38 @@ namespace gmx
  * \tparam  Step       Step increment.
  */
 template<typename EnumType, EnumType Last = EnumType::Count, unsigned int Step = 1>
-class EnumerationIterator final
+class EnumerationIterator final :
+    public boost::stl_interfaces::iterator_interface<EnumerationIterator<EnumType, Last, Step>, std::random_access_iterator_tag, EnumType>
 {
 public:
     //! Convenience alias
     using IntegerType = std::underlying_type_t<EnumType>;
 
-    /*! \name Iterator type traits
-     * Satisfies the requirements for STL forward iterator.
-     * \{
-     */
-    using iterator_category = std::forward_iterator_tag;
-    using value_type        = EnumType;
-    using difference_type   = std::ptrdiff_t;
-    using pointer           = EnumType*;
-    using reference         = EnumType&;
-    //! \}
-
     constexpr EnumerationIterator() noexcept : m_current{ 0 } // Assumes 0 is the first constant
     {
     }
-    //! Copy constructor
-    constexpr EnumerationIterator(const EnumType index) noexcept :
+    //! Conversion constructor
+    explicit constexpr EnumerationIterator(const EnumType index) noexcept :
         m_current(static_cast<IntegerType>(index))
     {
     }
-    //! Pre-increment operator
-    EnumerationIterator operator++()
+    //! Addition-assignment operator
+    constexpr EnumerationIterator& operator+=(std::ptrdiff_t i) noexcept
     {
-        m_current += Step;
+        m_current += Step * i;
         return *this;
     }
-    //! Post-increment operator
-    EnumerationIterator operator++(int)
-    {
-        EnumerationIterator old_val{ *this };
-        m_current += Step;
-        return old_val;
-    }
     //! Dereference operator
-    EnumType operator*() const
+    constexpr EnumType operator*() const noexcept
     {
         GMX_ASSERT(m_current < static_cast<IntegerType>(Last), "dereferencing out of range");
         return static_cast<EnumType>(m_current);
     }
-
-    /*!@{*/
-    //! Comparision operators
-    bool operator==(const EnumerationIterator other) const noexcept
-    {
-        return m_current == other.m_current;
-    }
-    bool operator!=(const EnumerationIterator other) const noexcept
+    //! Difference operator
+    constexpr std::ptrdiff_t operator-(const EnumerationIterator other) const noexcept
     {
-        return m_current != other.m_current;
+        return (m_current - other.m_current) / Step;
     }
-    bool operator<(const EnumerationIterator other) const noexcept
-    {
-        return m_current < other.m_current;
-    }
-    bool operator>(const EnumerationIterator other) const noexcept
-    {
-        return m_current > other.m_current;
-    }
-    bool operator<=(const EnumerationIterator other) const noexcept
-    {
-        return m_current <= other.m_current;
-    }
-    bool operator>=(const EnumerationIterator other) const noexcept
-    {
-        return m_current >= other.m_current;
-    }
-    /*!@}*/
 
 private:
     IntegerType m_current;