From 38a014f7201d8841a4e50527f873fbc17c8343fc Mon Sep 17 00:00:00 2001
From: Mateusz Jakub Fila <mateusz.jakub.fila@cern.ch>
Date: Tue, 11 Jun 2024 23:14:49 +0200
Subject: [PATCH] make dereference const by not updating local object,
 iterators are std::input_iterators

---
 doc/collections_as_container.md          |  4 ++--
 python/templates/macros/iterator.jinja2  |  8 ++++----
 tests/unittests/std_interoperability.cpp | 12 ++++++------
 3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/doc/collections_as_container.md b/doc/collections_as_container.md
index 461c89403..338fb24aa 100644
--- a/doc/collections_as_container.md
+++ b/doc/collections_as_container.md
@@ -80,12 +80,12 @@ In the following tables a convention from `Collection` is used: `iterator` stand
 
 | Concept | `iterator` | `const_iterator` |
 |---------|------------------------|------------------------------|
-| `std::indirectly_readable` | ❌ no | ❌ no |
+| `std::indirectly_readable` | ✔️ yes / ✔️ yes |
 | `std::indirectly_writable` | ❌ no | ❌ no |
 | `std::weakly_incrementable` | ✔️ yes | ✔️ yes |
 | `std::incrementable` | ✔️ yes | ✔️ yes |
 | `std::input_or_output_iterator` | ✔️ yes | ✔️ yes |
-| `std::input_iterator` | ❌ no | ❌ no |
+| `std::input_iterator` | ✔️ yes / ✔️ yes |
 | `std::output_iterator` | ❌ no | ❌ no |
 | `std::forward_iterator` | ❌ no | ❌ no |
 | `std::bidirectional_iterator` | ❌ no | ❌ no |
diff --git a/python/templates/macros/iterator.jinja2 b/python/templates/macros/iterator.jinja2
index 441ffb187..645c198e0 100644
--- a/python/templates/macros/iterator.jinja2
+++ b/python/templates/macros/iterator.jinja2
@@ -8,6 +8,7 @@ public:
   using reference = {{ prefix }}{{ class.bare_type }};
   using pointer = {{ prefix }}{{ class.bare_type }}*;
   using iterator_category = std::input_iterator_tag;
+  using iterator_concept = std::input_iterator_tag;
 
   {{ iterator_type }}(size_t index, const {{ class.bare_type }}ObjPointerContainer* collection) : m_index(index), m_object({{ ptr_init }}), m_collection(collection) {}
   {{ iterator_type }}() = default;
@@ -26,7 +27,7 @@ public:
     return m_index ==  x.m_index;
   }
 
-  reference operator*();
+  reference operator*() const;
   pointer operator->();
   {{ iterator_type }}& operator++();
   {{ iterator_type }} operator++(int);
@@ -43,9 +44,8 @@ private:
 {% macro iterator_definitions(class, prefix='') %}
 {% with iterator_type = class.bare_type + prefix + 'CollectionIterator' %}
 {% set ptr_type = 'podio::utils::MaybeSharedPtr<' + class.bare_type +'Obj>' %}
-{{ iterator_type }}::reference {{ iterator_type }}::operator*() {
-  m_object.m_obj = {{ ptr_type }}((*m_collection)[m_index]);
-  return m_object;
+{{ iterator_type }}::reference {{ iterator_type }}::operator*() const {
+  return reference{ {{ ptr_type }}((*m_collection)[m_index]) };
 }
 
 {{ iterator_type }}::pointer {{ iterator_type }}::operator->() {
diff --git a/tests/unittests/std_interoperability.cpp b/tests/unittests/std_interoperability.cpp
index 3acfc0619..2a72952ea 100644
--- a/tests/unittests/std_interoperability.cpp
+++ b/tests/unittests/std_interoperability.cpp
@@ -481,15 +481,15 @@ TEST_CASE("Collection and iterator concepts") {
   SECTION("input_iterator") {
     // indirectly_readable
     // iterator
-    DOCUMENTED_STATIC_FAILURE(std::indirectly_readable<iterator>);
+    STATIC_REQUIRE(std::indirectly_readable<iterator>);
     // const_iterator
-    DOCUMENTED_STATIC_FAILURE(std::indirectly_readable<const_iterator>);
+    STATIC_REQUIRE(std::indirectly_readable<const_iterator>);
 
     // input_iterator
     // iterator
-    DOCUMENTED_STATIC_FAILURE(std::input_iterator<iterator>);
+    STATIC_REQUIRE(std::input_iterator<iterator>);
     // const_iterator
-    DOCUMENTED_STATIC_FAILURE(std::input_iterator<const_iterator>);
+    STATIC_REQUIRE(std::input_iterator<const_iterator>);
   }
 
   SECTION("output_iterator") {
@@ -1093,7 +1093,7 @@ TEST_CASE("Collection and std iterator adaptors", "[collection][container][adapt
     STATIC_REQUIRE(traits::has_iterator_category_v<std::iterator_traits<iterator>>);
     STATIC_REQUIRE(std::is_base_of_v<std::input_iterator_tag, std::iterator_traits<iterator>::iterator_category>);
 #if (__cplusplus >= 202002L)
-    DOCUMENTED_STATIC_FAILURE(std::input_iterator<iterator>);
+    STATIC_REQUIRE(std::input_iterator<iterator>);
 #endif
     STATIC_REQUIRE(std::is_same_v<iterator::reference, std::move_iterator<iterator>::reference>);
     // const_iterator
@@ -1101,7 +1101,7 @@ TEST_CASE("Collection and std iterator adaptors", "[collection][container][adapt
     STATIC_REQUIRE(traits::has_iterator_category_v<std::iterator_traits<const_iterator>>);
     STATIC_REQUIRE(std::is_base_of_v<std::input_iterator_tag, std::iterator_traits<const_iterator>::iterator_category>);
 #if (__cplusplus >= 202002L)
-    DOCUMENTED_STATIC_FAILURE(std::input_iterator<const_iterator>);
+    STATIC_REQUIRE(std::input_iterator<const_iterator>);
 #endif
     STATIC_REQUIRE(std::is_same_v<const_iterator::reference, std::move_iterator<const_iterator>::reference>);
   }