From 56862ee2f64f9229a3dd38ac1b6d02a08eb951a6 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Mon, 11 Dec 2023 15:03:50 -0500 Subject: [PATCH] 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 Change-Id: I1bdb254a37533b8524450b22aa7f25a4d8025c2c Reviewed-on: https://review.lttng.org/c/babeltrace/+/11236 Reviewed-by: Simon Marchi --- src/Makefile.am | 2 +- .../bt2/borrowed-object-iterator.hpp | 92 ++++++++++++++++++ src/cpp-common/bt2/common-iterator.hpp | 94 ------------------- src/cpp-common/bt2/field-class.hpp | 13 ++- src/cpp-common/bt2/field-path.hpp | 5 +- src/cpp-common/bt2/integer-range-set.hpp | 4 +- src/cpp-common/bt2/value.hpp | 4 +- 7 files changed, 105 insertions(+), 109 deletions(-) create mode 100644 src/cpp-common/bt2/borrowed-object-iterator.hpp delete mode 100644 src/cpp-common/bt2/common-iterator.hpp diff --git a/src/Makefile.am b/src/Makefile.am index 1429883f..4d6eeb69 100644 --- 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 index 00000000..46e68c16 --- /dev/null +++ b/src/cpp-common/bt2/borrowed-object-iterator.hpp @@ -0,0 +1,92 @@ +/* + * Copyright (c) 2022 Francis Deslauriers + * Copyright (c) 2023 Philippe Proulx + * + * SPDX-License-Identifier: MIT + */ + +#ifndef BABELTRACE_CPP_COMMON_BT2_BORROWED_OBJECT_ITERATOR_HPP +#define BABELTRACE_CPP_COMMON_BT2_BORROWED_OBJECT_ITERATOR_HPP + +#include +#include +#include + +#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 +class BorrowedObjectIterator final +{ + friend ContainerT; + +public: + using Object = typename std::remove_reference< + decltype(std::declval()[std::declval()])>::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 operator->() const noexcept + { + return BorrowedObjectProxy {(**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 index 99332d65..00000000 --- a/src/cpp-common/bt2/common-iterator.hpp +++ /dev/null @@ -1,94 +0,0 @@ -/* - * Copyright (c) 2022 Francis Deslauriers - * - * SPDX-License-Identifier: MIT - */ - -#ifndef BABELTRACE_CPP_COMMON_BT2_COMMON_ITERATOR_HPP -#define BABELTRACE_CPP_COMMON_BT2_COMMON_ITERATOR_HPP - -#include -#include - -#include "cpp-common/optional.hpp" - -namespace bt2 { - -template -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 _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 a053da3d..5ce29b70 100644 --- 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; + using Iterator = BorrowedObjectIterator; using Mapping = MappingT; explicit CommonEnumerationFieldClass(const _LibObjPtr libObjPtr) noexcept : @@ -1126,12 +1126,11 @@ private: public: using Shared = SharedFieldClass, LibObjT>; + using Iterator = BorrowedObjectIterator>; using Member = internal::DepType; - using Iterator = CommonIterator, Member>; - explicit CommonStructureFieldClass(const _LibObjPtr libObjPtr) noexcept : _ThisCommonFieldClass {libObjPtr} { @@ -2153,12 +2152,11 @@ protected: public: using Shared = SharedFieldClass, LibObjT>; + using Iterator = BorrowedObjectIterator; using Option = internal::DepType; - using Iterator = CommonIterator; - explicit CommonVariantFieldClass(const _LibObjPtr libObjPtr) noexcept : _ThisCommonFieldClass {libObjPtr} { @@ -2473,8 +2471,9 @@ public: using Shared = SharedFieldClass<_ThisCommonVariantWithIntegerSelectorFieldClass, LibObjT>; using Option = OptionT; + using Iterator = - CommonIterator, Option>; + BorrowedObjectIterator>; 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 8f20405d..69f48455 100644 --- 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 { public: using Shared = SharedObject; - - using Iterator = CommonIterator; + using Iterator = BorrowedObjectIterator; enum class Scope { diff --git a/src/cpp-common/bt2/integer-range-set.hpp b/src/cpp-common/bt2/integer-range-set.hpp index be0d1b75..b42a28c4 100644 --- a/src/cpp-common/bt2/integer-range-set.hpp +++ b/src/cpp-common/bt2/integer-range-set.hpp @@ -12,8 +12,8 @@ #include +#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; + using Iterator = BorrowedObjectIterator; 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 c0a60fe2..37020672 100644 --- 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, LibObjT>; - using Iterator = CommonIterator, CommonValue>; + using Iterator = BorrowedObjectIterator>; explicit CommonArrayValue(const _LibObjPtr libObjPtr) noexcept : _ThisCommonValue {libObjPtr} { -- 2.34.1