refs #24322. Understand std::advance behaviour

Framework/Geometry/inc/MantidGeometry/Instrument/InfoIteratorBase.h

@@ -17,6 +17,10 @@ namespace Geometry {
 
 Base to allow users of the Info objects (DetectorInfo etc) access to data
 via a random access iterator.
+
+Note that the reference type (InfoItem<T>) causes the iterator to be treated as
+std::input_iterator for the purposes of many std algorithms such as
+std::advance. See https://en.cppreference.com/w/cpp/iterator/advance for example
 */
 template <typename T, template <typename> class InfoItem>
 class InfoIteratorBase
@@ -51,6 +55,10 @@ private:
                              m_item.m_index + static_cast<size_t>(delta));
   }
 
+  bool equal(const InfoIteratorBase<T, InfoItem> &other) const {
+    return getIndex() == other.getIndex();
+  }
+
   void increment() {
     if (m_item.m_index < m_totalSize) {
       ++m_item.m_index;
@@ -67,10 +75,6 @@ private:
 
   void setIndex(const size_t index) { m_item.m_index = index; }
 
-  bool equal(const InfoIteratorBase<T, InfoItem> &other) const {
-    return getIndex() == other.getIndex();
-  }
-
   InfoItem<T> dereference() const { return m_item; }
 
   uint64_t distance_to(const InfoIteratorBase<T, InfoItem> &other) const {

Framework/Geometry/test/DetectorInfoIteratorTest.h

@@ -14,9 +14,10 @@
 #include "MantidGeometry/Instrument/DetectorInfoIterator.h"
 #include "MantidGeometry/Instrument/InstrumentVisitor.h"
 #include "MantidTestHelpers/ComponentCreationHelper.h"
-#include <iterator>
-
 #include <cxxtest/TestSuite.h>
+#include <iterator>
+#include <type_traits>
+#include <typeinfo>
 
 using namespace ComponentCreationHelper;
 using namespace Mantid::Geometry;
@@ -147,6 +148,35 @@ public:
     TS_ASSERT(iter == detectorInfo->cbegin());
   }
 
+  void test_iterator_catagory() {
+    // Characterisation tests
+    using ItTag =
+        typename std::iterator_traits<DetectorInfoConstIt>::iterator_category;
+    using InputItTag = std::input_iterator_tag;
+    using BidirectionalItTag = std::bidirectional_iterator_tag;
+    const static bool inputit = std::is_convertible<ItTag, InputItTag>::value;
+    const static bool bidirectionalit =
+        std::is_convertible<ItTag, BidirectionalItTag>::value;
+    TSM_ASSERT("Iterator expected to be treated as input_iterator", inputit);
+    // Assert below. Iterator not bidirectional. This is why decrement via
+    // std::advance is not supported. Iterator reference must be true reference
+    // to support this.
+    TSM_ASSERT(
+        "Iterator expected not to be treated as legacy bidirectional iterator",
+        !bidirectionalit);
+
+    // see https://en.cppreference.com/w/cpp/iterator/advance
+    // Demonstrate internal switched behaviour in std::advance
+    auto detectorInfo = create_detector_info_object();
+    auto it = detectorInfo->cbegin();
+    TS_ASSERT_EQUALS(it->index(), 0);
+    std::advance(it, 2);
+    TS_ASSERT_EQUALS(it->index(), 2);
+    std::advance(it, -2);
+    TSM_ASSERT_EQUALS("Was not decremented. Not zero. For reasons above",
+                      it->index(), 2);
+  }
+
   void test_iterator_advance_and_positions() {
     // Get the DetectorInfo object
     auto detectorInfo = create_detector_info_object();
@@ -162,12 +192,12 @@ public:
 
     // Go backwards 2 places
     xValue = 15.0;
-    std::advance(iter, -2);
+    iter -= 2;
     TS_ASSERT_EQUALS(iter->position().X(), xValue)
 
     // Go to the start
-    std::advance(iter, -4);
-    TS_ASSERT(iter == detectorInfo->cbegin());
+    iter -= 4;
+    TS_ASSERT_EQUALS(iter, detectorInfo->cbegin());
   }
 
   void test_copy_iterator_and_positions() {