cpp-common/bt2: rework `bt2::CommonIterator` (now `bt2::BorrowedObjectIterator`)

We don't need all the STL stuff (and constraints) currently, therefore
this iterator class may be simpler, making operator*() call
ContainerT::operator[]() directly instead of keeping a temporary wrapper
object and having a conditional for each iteration increment.

Making operator->() return a proxy because the iterator instance has no
persistent wrapper instance.

Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: I1bdb254a37533b8524450b22aa7f25a4d8025c2c
Reviewed-on: https://review.lttng.org/c/babeltrace/+/11236
Reviewed-by: Simon Marchi <simon.marchi@efficios.com>

diff --git a/src/Makefile.am b/src/Makefile.am
index 1429883f654cb5bea2f769d17a3f9378a6c8785f..4d6eeb6983965c340302e55638f5a2686e6d2c7b 100644 (file)
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -12,11 +12,11 @@ SUBDIRS += bindings/python/bt2
 endif
 
 noinst_HEADERS = \
+       cpp-common/bt2/borrowed-object-iterator.hpp \
        cpp-common/bt2/borrowed-object-proxy.hpp \
        cpp-common/bt2/borrowed-object.hpp \
        cpp-common/bt2/clock-class.hpp \
        cpp-common/bt2/clock-snapshot.hpp \
-       cpp-common/bt2/common-iterator.hpp \
        cpp-common/bt2/exc.hpp \
        cpp-common/bt2/field-class.hpp \
        cpp-common/bt2/field.hpp \

diff --git a/src/cpp-common/bt2/borrowed-object-iterator.hpp b/src/cpp-common/bt2/borrowed-object-iterator.hpp
new file mode 100644 (file)
index 0000000..46e68c1
--- /dev/null
+++ b/src/cpp-common/bt2/borrowed-object-iterator.hpp
@@ -0,0 +1,92 @@
+/*
+ * Copyright (c) 2022 Francis Deslauriers <francis.deslauriers@efficios.com>
+ * Copyright (c) 2023 Philippe Proulx <pproulx@efficios.com>
+ *
+ * SPDX-License-Identifier: MIT
+ */
+
+#ifndef BABELTRACE_CPP_COMMON_BT2_BORROWED_OBJECT_ITERATOR_HPP
+#define BABELTRACE_CPP_COMMON_BT2_BORROWED_OBJECT_ITERATOR_HPP
+
+#include <cstdint>
+#include <type_traits>
+#include <utility>
+
+#include "common/assert.h"
+
+#include "borrowed-object-proxy.hpp"
+
+namespace bt2 {
+
+/*
+ * An iterator class to iterate an instance of a borrowed object
+ * container of type `ContainerT`.
+ *
+ * `ContainerT` must implement:
+ *
+ *     // Returns the number of contained borrowed objects.
+ *     std::uint64_t length() const noexcept;
+ *
+ *     // Returns the borrowed object at index `i`.
+ *     SomeObject operator[](std::uint64_t i) const noexcept;
+ */
+template <typename ContainerT>
+class BorrowedObjectIterator final
+{
+    friend ContainerT;
+
+public:
+    using Object = typename std::remove_reference<
+        decltype(std::declval<ContainerT>()[std::declval<std::uint64_t>()])>::type;
+
+private:
+    explicit BorrowedObjectIterator(const ContainerT container, const uint64_t idx) :
+        _mContainer {container}, _mIdx {idx}
+    {
+    }
+
+public:
+    BorrowedObjectIterator& operator++() noexcept
+    {
+        ++_mIdx;
+        return *this;
+    }
+
+    BorrowedObjectIterator operator++(int) noexcept
+    {
+        const auto tmp = *this;
+
+        ++(*this);
+        return tmp;
+    }
+
+    bool operator==(const BorrowedObjectIterator& other) const noexcept
+    {
+        BT_ASSERT_DBG(_mContainer.isSame(other._mContainer));
+        return _mIdx == other._mIdx;
+    }
+
+    bool operator!=(const BorrowedObjectIterator& other) const noexcept
+    {
+        return !(*this == other);
+    }
+
+    Object operator*() const noexcept
+    {
+        BT_ASSERT_DBG(_mIdx < _mContainer.length());
+        return _mContainer[_mIdx];
+    }
+
+    BorrowedObjectProxy<Object> operator->() const noexcept
+    {
+        return BorrowedObjectProxy<Object> {(**this).libObjPtr()};
+    }
+
+private:
+    ContainerT _mContainer;
+    std::uint64_t _mIdx;
+};
+
+} /* namespace bt2 */
+
+#endif /* BABELTRACE_CPP_COMMON_BT2_BORROWED_OBJECT_ITERATOR_HPP */

diff --git a/src/cpp-common/bt2/common-iterator.hpp b/src/cpp-common/bt2/common-iterator.hpp
deleted file mode 100644 (file)
index 99332d6..0000000
--- a/src/cpp-common/bt2/common-iterator.hpp
+++ /dev/null
@@ -1,94 +0,0 @@
-/*
- * Copyright (c) 2022 Francis Deslauriers <francis.deslauriers@efficios.com>
- *
- * SPDX-License-Identifier: MIT
- */
-
-#ifndef BABELTRACE_CPP_COMMON_BT2_COMMON_ITERATOR_HPP
-#define BABELTRACE_CPP_COMMON_BT2_COMMON_ITERATOR_HPP
-
-#include <cstdint>
-#include <iterator>
-
-#include "cpp-common/optional.hpp"
-
-namespace bt2 {
-
-template <typename ContainerT, typename ElemT>
-class CommonIterator
-{
-    friend ContainerT;
-
-public:
-    using difference_type = std::ptrdiff_t;
-    using value_type = ElemT;
-    using pointer = value_type *;
-    using reference = value_type&;
-    using iterator_category = std::input_iterator_tag;
-
-private:
-    explicit CommonIterator(const ContainerT container, const uint64_t idx) :
-        _mContainer {container}, _mIdx {idx}
-    {
-        this->_updateCurrentValue();
-    }
-
-public:
-    CommonIterator(const CommonIterator&) = default;
-    CommonIterator(CommonIterator&&) = default;
-    CommonIterator& operator=(const CommonIterator&) = default;
-    CommonIterator& operator=(CommonIterator&&) = default;
-
-    CommonIterator& operator++() noexcept
-    {
-        ++_mIdx;
-        this->_updateCurrentValue();
-        return *this;
-    }
-
-    CommonIterator operator++(int) noexcept
-    {
-        const auto tmp = *this;
-
-        ++(*this);
-        return tmp;
-    }
-
-    bool operator==(const CommonIterator& other) const noexcept
-    {
-        return _mIdx == other._mIdx;
-    }
-
-    bool operator!=(const CommonIterator& other) const noexcept
-    {
-        return !(*this == other);
-    }
-
-    reference operator*() const noexcept
-    {
-        return *_mCurrVal;
-    }
-
-    pointer operator->() const noexcept
-    {
-        return &(*_mCurrVal);
-    }
-
-private:
-    void _updateCurrentValue() noexcept
-    {
-        if (_mIdx < _mContainer.length()) {
-            _mCurrVal = _mContainer[_mIdx];
-        } else {
-            _mCurrVal = nonstd::nullopt;
-        }
-    }
-
-    nonstd::optional<value_type> _mCurrVal;
-    ContainerT _mContainer;
-    uint64_t _mIdx;
-};
-
-} /* namespace bt2 */
-
-#endif /* BABELTRACE_CPP_COMMON_BT2_COMMON_ITERATOR_HPP */

diff --git a/src/cpp-common/bt2/field-class.hpp b/src/cpp-common/bt2/field-class.hpp
index a053da3d7081abe34de1455761ecf23ef10fd4da..5ce29b70a4d8c621bf9df48bd57c96f12d362c43 100644 (file)
--- a/src/cpp-common/bt2/field-class.hpp
+++ b/src/cpp-common/bt2/field-class.hpp
@@ -16,8 +16,8 @@
 #include "cpp-common/optional.hpp"
 #include "cpp-common/string_view.hpp"
 
+#include "borrowed-object-iterator.hpp"
 #include "borrowed-object.hpp"
-#include "common-iterator.hpp"
 #include "exc.hpp"
 #include "field-path.hpp"
 #include "integer-range-set.hpp"
@@ -826,7 +826,7 @@ private:
 
 public:
     using Shared = SharedFieldClass<_ThisCommonEnumerationFieldClass, LibObjT>;
-    using Iterator = CommonIterator<CommonEnumerationFieldClass, MappingT>;
+    using Iterator = BorrowedObjectIterator<CommonEnumerationFieldClass>;
     using Mapping = MappingT;
 
     explicit CommonEnumerationFieldClass(const _LibObjPtr libObjPtr) noexcept :
@@ -1126,12 +1126,11 @@ private:
 
 public:
     using Shared = SharedFieldClass<CommonStructureFieldClass<LibObjT>, LibObjT>;
+    using Iterator = BorrowedObjectIterator<CommonStructureFieldClass<LibObjT>>;
 
     using Member =
         internal::DepType<LibObjT, StructureFieldClassMember, ConstStructureFieldClassMember>;
 
-    using Iterator = CommonIterator<CommonStructureFieldClass<LibObjT>, Member>;
-
     explicit CommonStructureFieldClass(const _LibObjPtr libObjPtr) noexcept :
         _ThisCommonFieldClass {libObjPtr}
     {
@@ -2153,12 +2152,11 @@ protected:
 
 public:
     using Shared = SharedFieldClass<CommonVariantFieldClass<LibObjT>, LibObjT>;
+    using Iterator = BorrowedObjectIterator<CommonVariantFieldClass>;
 
     using Option =
         internal::DepType<LibObjT, VariantFieldClassOption, ConstVariantFieldClassOption>;
 
-    using Iterator = CommonIterator<CommonVariantFieldClass, Option>;
-
     explicit CommonVariantFieldClass(const _LibObjPtr libObjPtr) noexcept :
         _ThisCommonFieldClass {libObjPtr}
     {
@@ -2473,8 +2471,9 @@ public:
     using Shared = SharedFieldClass<_ThisCommonVariantWithIntegerSelectorFieldClass, LibObjT>;
 
     using Option = OptionT;
+
     using Iterator =
-        CommonIterator<CommonVariantWithIntegerSelectorFieldClass<LibObjT, Option>, Option>;
+        BorrowedObjectIterator<CommonVariantWithIntegerSelectorFieldClass<LibObjT, Option>>;
 
     explicit CommonVariantWithIntegerSelectorFieldClass(const _LibObjPtr libObjPtr) noexcept :
         _ThisCommonVariantWithSelectorFieldClass {libObjPtr}

diff --git a/src/cpp-common/bt2/field-path.hpp b/src/cpp-common/bt2/field-path.hpp
index 8f20405d324778ce94f80b47de1fab0936389af3..69f48455b2c325ec3e91258ee7c6aafe12e7c8ea 100644 (file)
--- a/src/cpp-common/bt2/field-path.hpp
+++ b/src/cpp-common/bt2/field-path.hpp
@@ -13,8 +13,8 @@
 
 #include "common/assert.h"
 
+#include "borrowed-object-iterator.hpp"
 #include "borrowed-object.hpp"
-#include "common-iterator.hpp"
 #include "shared-object.hpp"
 
 namespace bt2 {
@@ -128,8 +128,7 @@ class ConstFieldPath final : public BorrowedObject<const bt_field_path>
 {
 public:
     using Shared = SharedObject<ConstFieldPath, const bt_field_path, internal::FieldPathRefFuncs>;
-
-    using Iterator = CommonIterator<ConstFieldPath, ConstFieldPathItem>;
+    using Iterator = BorrowedObjectIterator<ConstFieldPath>;
 
     enum class Scope
     {

diff --git a/src/cpp-common/bt2/integer-range-set.hpp b/src/cpp-common/bt2/integer-range-set.hpp
index be0d1b7584035a3c136ad5a4f9b852d27607c789..b42a28c4ac21c60b40226f792d6c8a378f5769af 100644 (file)
--- a/src/cpp-common/bt2/integer-range-set.hpp
+++ b/src/cpp-common/bt2/integer-range-set.hpp
@@ -12,8 +12,8 @@
 
 #include <babeltrace2/babeltrace.h>
 
+#include "borrowed-object-iterator.hpp"
 #include "borrowed-object.hpp"
-#include "common-iterator.hpp"
 #include "exc.hpp"
 #include "integer-range.hpp"
 #include "internal/utils.hpp"
@@ -150,7 +150,7 @@ public:
         ConstUnsignedIntegerRange, ConstSignedIntegerRange>::type;
 
     using Value = typename Range::Value;
-    using Iterator = CommonIterator<CommonIntegerRangeSet, Range>;
+    using Iterator = BorrowedObjectIterator<CommonIntegerRangeSet>;
 
     explicit CommonIntegerRangeSet(const _LibObjPtr libObjPtr) noexcept :
         _ThisBorrowedObject {libObjPtr}

diff --git a/src/cpp-common/bt2/value.hpp b/src/cpp-common/bt2/value.hpp
index c0a60fe25c538eeda194fd14074d11993d028309..370206725762acefd7e10ddef350823b3cb1d23f 100644 (file)
--- a/src/cpp-common/bt2/value.hpp
+++ b/src/cpp-common/bt2/value.hpp
@@ -18,8 +18,8 @@
 #include "cpp-common/optional.hpp"
 #include "cpp-common/string_view.hpp"
 
+#include "borrowed-object-iterator.hpp"
 #include "borrowed-object.hpp"
-#include "common-iterator.hpp"
 #include "exc.hpp"
 #include "internal/utils.hpp"
 #include "shared-object.hpp"
@@ -777,7 +777,7 @@ private:
 
 public:
     using Shared = SharedValue<CommonArrayValue<LibObjT>, LibObjT>;
-    using Iterator = CommonIterator<CommonArrayValue<LibObjT>, CommonValue<LibObjT>>;
+    using Iterator = BorrowedObjectIterator<CommonArrayValue<LibObjT>>;
 
     explicit CommonArrayValue(const _LibObjPtr libObjPtr) noexcept : _ThisCommonValue {libObjPtr}
     {