give up on increment/decrement: requires friend class

Signed-off-by: Martijn Govers <[email protected]>

Author: mgovers

power_grid_model_c/power_grid_model/include/power_grid_model/common/grouped_index_vector.hpp

@@ -177,6 +177,8 @@ class SparseGroupedIdxVector {
 class DenseGroupedIdxVector {
   private:
     class GroupIterator : public IteratorFacade {
+        friend class IteratorFacade; // to expose increment and decrement
+
       public:
         using iterator = GroupIterator;
         using const_iterator = std::add_const_t<GroupIterator>;

power_grid_model_c/power_grid_model/include/power_grid_model/common/iterator_facade.hpp

@@ -46,15 +46,15 @@ class IteratorFacade {
     }
 
     template <typename Self> constexpr std::add_lvalue_reference_t<Self> operator++(this Self& self) {
-        if constexpr (requires { self.increment(); }) {
+        if constexpr (requires { self.increment(); }) { // NOTE: IteratorFacade should be a friend class
             self.increment();
         } else {
             return (self += 1);
         }
         return self;
     }
     template <typename Self> constexpr std::add_lvalue_reference_t<Self> operator--(this Self& self) {
-        if constexpr (requires { self.decrement(); }) {
+        if constexpr (requires { self.decrement(); }) { // NOTE: IteratorFacade should be a friend class
             self.decrement();
         } else {
             return (self += -1);

tests/cpp_unit_tests/test_iterator_facade.cpp

@@ -7,14 +7,33 @@
 
 #include <doctest/doctest.h>
 
+#include <optional>
+
 namespace {
 using power_grid_model::Idx;
 using power_grid_model::IdxRange;
 using power_grid_model::IdxVector;
 using power_grid_model::IteratorFacade;
 using power_grid_model::detail::iterator_facadeable_c;
 
-template <typename UnderlyingType> class BaseTestIterator : public IteratorFacade {
+enum class IteratorFacadeableCalls : Idx {
+    none = 0,
+    dereference = 1,
+    advance = 2,
+    distance_to = 3,
+    increment = 4,
+    decrement = 5,
+};
+
+struct without_increment_decrement_t {};
+struct with_increment_decrement_t {};
+
+template <typename T>
+concept advance_type_c = std::same_as<T, without_increment_decrement_t> || std::same_as<T, with_increment_decrement_t>;
+
+template <advance_type_c advance_type_t, typename UnderlyingType> class BaseTestIterator : public IteratorFacade {
+    friend class IteratorFacade;
+
   public:
     using underlying = UnderlyingType;
 
@@ -23,38 +42,84 @@ template <typename UnderlyingType> class BaseTestIterator : public IteratorFacad
     using pointer = underlying::pointer;
     using reference = underlying::reference;
 
+    static constexpr auto increment_style = std::same_as<advance_type_t, with_increment_decrement_t>
+                                                ? IteratorFacadeableCalls::increment
+                                                : IteratorFacadeableCalls::advance;
+    static constexpr auto decrement_style = std::same_as<advance_type_t, with_increment_decrement_t>
+                                                ? IteratorFacadeableCalls::decrement
+                                                : IteratorFacadeableCalls::advance;
+
+    mutable std::optional<IteratorFacadeableCalls> last_call;
+
     constexpr BaseTestIterator(std::remove_cvref_t<underlying> it) : IteratorFacade{*this}, it_{std::move(it)} {}
 
-    constexpr auto operator*() -> reference { return *it_; }
-    constexpr auto operator*() const -> std::add_lvalue_reference_t<std::add_const_t<value_type>> { return *it_; }
+    constexpr auto operator*() -> reference {
+        last_call = IteratorFacadeableCalls::dereference;
+        return *it_;
+    }
+    constexpr auto operator*() const -> std::add_lvalue_reference_t<std::add_const_t<value_type>> {
+        last_call = IteratorFacadeableCalls::dereference;
+        return *it_;
+    }
 
     friend constexpr auto operator<=>(BaseTestIterator const& first,
                                       BaseTestIterator const& second) -> std::strong_ordering {
+        first.last_call = IteratorFacadeableCalls::distance_to;
         return *first.it_ <=> *second.it_;
     }
 
-    constexpr auto operator+=(difference_type n) -> std::add_lvalue_reference_t<BaseTestIterator> {
+    constexpr decltype(auto) operator+=(difference_type n) {
+        last_call = IteratorFacadeableCalls::advance;
         it_ += n;
         return *this;
     }
     friend constexpr auto operator-(BaseTestIterator const& first, BaseTestIterator const& second) -> difference_type {
+        first.last_call = IteratorFacadeableCalls::distance_to;
         return first.it_ - second.it_;
     }
 
   private:
     underlying it_;
+
+    constexpr void increment()
+        requires std::same_as<advance_type_t, with_increment_decrement_t>
+    {
+        last_call = IteratorFacadeableCalls::increment;
+        ++it_;
+    }
+    constexpr void decrement()
+        requires std::same_as<advance_type_t, with_increment_decrement_t>
+    {
+        last_call = IteratorFacadeableCalls::decrement;
+        --it_;
+    }
 };
 
-using TestIdxVectorIterator = BaseTestIterator<typename IdxVector::iterator>;
-using TestIdxVectorConstIterator = BaseTestIterator<typename IdxVector::const_iterator>;
+using TestIdxVectorIterator = BaseTestIterator<without_increment_decrement_t, typename IdxVector::iterator>;
+using TestIdxVectorConstIterator = BaseTestIterator<without_increment_decrement_t, typename IdxVector::const_iterator>;
+using TestIdxVectorIteratorWithIncDec = BaseTestIterator<with_increment_decrement_t, typename IdxVector::iterator>;
+using TestIdxVectorConstIteratorWithIncDec =
+    BaseTestIterator<with_increment_decrement_t, typename IdxVector::const_iterator>;
 
 static_assert(iterator_facadeable_c<TestIdxVectorIterator>);
 static_assert(iterator_facadeable_c<TestIdxVectorConstIterator>);
+static_assert(iterator_facadeable_c<TestIdxVectorIteratorWithIncDec>);
+static_assert(iterator_facadeable_c<TestIdxVectorConstIteratorWithIncDec>);
+
 static_assert(std::constructible_from<IteratorFacade, TestIdxVectorIterator>);
 static_assert(std::constructible_from<IteratorFacade, TestIdxVectorConstIterator>);
+static_assert(std::constructible_from<IteratorFacade, TestIdxVectorIteratorWithIncDec>);
+static_assert(std::constructible_from<IteratorFacade, TestIdxVectorConstIteratorWithIncDec>);
+
 } // namespace
 
-TEST_CASE_TEMPLATE("Test IteratorFacade", T, TestIdxVectorIterator, TestIdxVectorConstIterator) {
+TYPE_TO_STRING_AS("TestIdxVectorIterator", TestIdxVectorIterator);
+TYPE_TO_STRING_AS("TestIdxVectorConstIterator", TestIdxVectorConstIterator);
+TYPE_TO_STRING_AS("TestIdxVectorIteratorWithIncDec", TestIdxVectorIteratorWithIncDec);
+TYPE_TO_STRING_AS("TestIdxVectorConstIteratorWithIncDec", TestIdxVectorConstIteratorWithIncDec);
+
+TEST_CASE_TEMPLATE("Test IteratorFacade", T, TestIdxVectorIterator, TestIdxVectorConstIterator,
+                   TestIdxVectorIteratorWithIncDec, TestIdxVectorConstIteratorWithIncDec) {
     using TestIterator = T;
 
     auto vec = IdxRange{40} | std::ranges::to<IdxVector>();
@@ -63,10 +128,10 @@ TEST_CASE_TEMPLATE("Test IteratorFacade", T, TestIdxVectorIterator, TestIdxVecto
         auto it = TestIterator{vec.begin() + 5};
         CHECK(*it == 5);
         CHECK(*(++it) == 6);
-        CHECK(*it++ == 6);
+        CHECK(*(it++) == 6);
         CHECK(*it == 7);
         CHECK(*(--it) == 6);
-        CHECK(*it-- == 6);
+        CHECK(*(it--) == 6);
         CHECK(*it == 5);
         it += 3;
         CHECK(*it == 8);
@@ -103,4 +168,28 @@ TEST_CASE_TEMPLATE("Test IteratorFacade", T, TestIdxVectorIterator, TestIdxVecto
             CHECK(vec[5] == 42);
         }
     }
+    SUBCASE("Incremented/Decremented calls") {
+        auto it = TestIterator{vec.begin() + 5};
+        REQUIRE(!it.last_call.has_value());
+
+        ++it;
+        REQUIRE(it.last_call.has_value());
+        CHECK(it.last_call.value() == TestIterator::increment_style);
+        it.last_call.reset();
+
+        it++;
+        REQUIRE(it.last_call.has_value());
+        CHECK(it.last_call.value() == TestIterator::increment_style);
+        it.last_call.reset();
+
+        --it;
+        REQUIRE(it.last_call.has_value());
+        CHECK(it.last_call.value() == TestIterator::decrement_style);
+        it.last_call.reset();
+
+        it--;
+        REQUIRE(it.last_call.has_value());
+        CHECK(it.last_call.value() == TestIterator::decrement_style);
+        it.last_call.reset();
+    }
 }