[libcamera-devel] [PATCH/RFC 1/3] libcamera: utils: Add enumerate view for range-based for loops
Hirokazu Honda
hiroh at chromium.org
Tue Apr 27 04:29:44 CEST 2021
Hi Laurent, thanks for the patch.
On Fri, Apr 23, 2021 at 11:09 AM Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> Range-based for loops are handy and widely preferred in C++, but are
> limited in their ability to replace for loops that require access to a
> loop counter. The enumerate() function solves this problem by wrapping
> the \a iterable in an adapter that, when used as a range-expression,
> will provide iterators whose value_type is a pair of index and value
> reference.
>
> The iterable must support std::begin() and std::end(). This includes all
> containers provided by the standard C++ library, as well as C-style
> arrays.
>
> A typical usage pattern would use structured binding to store the index
> and value in two separate variables:
>
> std::vector<int> values = ...;
>
> for (auto [index, value] : utils::enumerate(values)) {
> ...
> }
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> include/libcamera/internal/utils.h | 86 ++++++++++++++++++++++++++++++
> src/libcamera/utils.cpp | 29 ++++++++++
> test/utils.cpp | 59 ++++++++++++++++++++
> 3 files changed, 174 insertions(+)
>
> diff --git a/include/libcamera/internal/utils.h b/include/libcamera/internal/utils.h
> index d0146b71727d..9372a75889ac 100644
> --- a/include/libcamera/internal/utils.h
> +++ b/include/libcamera/internal/utils.h
> @@ -9,12 +9,14 @@
>
> #include <algorithm>
> #include <chrono>
> +#include <iterator>
> #include <memory>
> #include <ostream>
> #include <sstream>
> #include <string>
> #include <string.h>
> #include <sys/time.h>
> +#include <utility>
> #include <vector>
>
> #ifndef __DOXYGEN__
> @@ -230,6 +232,90 @@ details::reverse_adapter<T> reverse(T &&iterable)
> return { iterable };
> }
>
> +namespace details {
> +
> +template<typename Base>
> +class enumerate_iterator
> +{
I always implemented my own iterator with deriving std::itrator.
Today I learned it is getting deprecated... :0
> +private:
> + using base_reference = typename std::iterator_traits<Base>::reference;
> +
> +public:
> + using difference_type = typename std::iterator_traits<Base>::difference_type;
> + using value_type = std::pair<const difference_type, base_reference>;
> + using pointer = value_type *;
> + using reference = value_type &;
> + using iterator_category = typename std::iterator_traits<Base>::iterator_category;
This class implements only std::forward_iterator_tag. I think this
should be std::forward_iterator_tag.
> +
> + explicit enumerate_iterator(Base iter)
> + : current_(iter), pos_(0)
> + {
> + }
> +
> + enumerate_iterator &operator++()
> + {
> + ++current_;
> + ++pos_;
> + return *this;
> + }
> +
> + bool operator!=(const enumerate_iterator &other) const
> + {
> + return current_ != other.current_;
> + }
> +
> + value_type operator*() const
> + {
> + return { pos_, *current_ };
> + }
Write in one-line?
value_type operator*() const { return { pos_, *current_ }; }
> +
> +private:
> + Base current_;
> + difference_type pos_;
> +};
> +
> +template<typename Base>
> +class enumerate_adapter
> +{
> +public:
> + using iterator = enumerate_iterator<Base>;
> +
> + enumerate_adapter(Base begin, Base end)
> + : begin_(begin), end_(end)
> + {
> + }
> +
> + iterator begin()
> + {
> + return iterator{ begin_ };
> + }
> +
> + iterator end()
> + {
> + return iterator{ end_ };
> + }
> +
begin() and end() should be one-line too.
Can we have begin_ and end_ to be iterator, so that begin() and end()
just returns them, not dynamically construct iterator?
Furthemore, can the variables and the function be constant?
-Hiro
> +private:
> + Base begin_;
> + Base end_;
> +};
> +
> +} /* namespace details */
> +
> +template<typename T>
> +auto enumerate(T &iterable) -> details::enumerate_adapter<decltype(iterable.begin())>
> +{
> + return { std::begin(iterable), std::end(iterable) };
> +}
> +
> +#ifndef __DOXYGEN__
> +template<typename T, size_t N>
> +auto enumerate(T (&iterable)[N]) -> details::enumerate_adapter<T *>
> +{
> + return { std::begin(iterable), std::end(iterable) };
> +}
> +#endif
> +
> } /* namespace utils */
>
> } /* namespace libcamera */
> diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp
> index c4098a74e0ab..ff9a5832b10e 100644
> --- a/src/libcamera/utils.cpp
> +++ b/src/libcamera/utils.cpp
> @@ -472,6 +472,35 @@ std::string libcameraSourcePath()
> * loop, will cause the loop to iterate over the \a iterable in reverse order
> */
>
> +/**
> + * \fn enumerate(T &iterable)
> + * \brief Wrap an iterable to enumerate index and value in a range-based loop
> + * \param[in] iterable The iterable
> + *
> + * Range-based for loops are handy and widely preferred in C++, but are limited
> + * in their ability to replace for loops that require access to a loop counter.
> + * The enumerate() function solves this problem by wrapping the \a iterable in
> + * an adapter that, when used as a range-expression, will provide iterators
> + * whose value_type is a pair of index and value reference.
> + *
> + * The iterable must support std::begin() and std::end(). This includes all
> + * containers provided by the standard C++ library, as well as C-style arrays.
> + *
> + * A typical usage pattern would use structured binding to store the index and
> + * value in two separate variables:
> + *
> + * \code{.cpp}
> + * std::vector<int> values = ...;
> + *
> + * for (auto [index, value] : utils::enumerate(values)) {
> + * ...
> + * }
> + * \endcode
> + *
> + * \return A value of unspecified type that, when used in a range-based for
> + * loop, iterates over an indexed view of the \a iterable
> + */
> +
> } /* namespace utils */
>
> } /* namespace libcamera */
> diff --git a/test/utils.cpp b/test/utils.cpp
> index 08f293898fd9..7e24c71e4775 100644
> --- a/test/utils.cpp
> +++ b/test/utils.cpp
> @@ -12,6 +12,7 @@
> #include <vector>
>
> #include <libcamera/geometry.h>
> +#include <libcamera/span.h>
>
> #include "libcamera/internal/utils.h"
>
> @@ -73,6 +74,60 @@ protected:
> return TestPass;
> }
>
> + int testEnumerate()
> + {
> + std::vector<int> integers{ 1, 2, 3, 4, 5 };
> + int i = 0;
> +
> + for (auto [index, value] : utils::enumerate(integers)) {
> + if (index != i || value != i + 1) {
> + cerr << "utils::enumerate(<vector>) test failed: i=" << i
> + << ", index=" << index << ", value=" << value
> + << std::endl;
> + return TestFail;
> + }
> +
> + /* Verify that we can modify the value. */
> + --value;
> + ++i;
> + }
> +
> + if (integers != std::vector<int>{ 0, 1, 2, 3, 4 }) {
> + cerr << "Failed to modify container in enumerated range loop" << endl;
> + return TestFail;
> + }
> +
> + Span<const int> span{ integers };
> + i = 0;
> +
> + for (auto [index, value] : utils::enumerate(span)) {
> + if (index != i || value != i) {
> + cerr << "utils::enumerate(<span>) test failed: i=" << i
> + << ", index=" << index << ", value=" << value
> + << std::endl;
> + return TestFail;
> + }
> +
> + ++i;
> + }
> +
> + const int array[] = { 0, 2, 4, 6, 8 };
> + i = 0;
> +
> + for (auto [index, value] : utils::enumerate(array)) {
> + if (index != i || value != i * 2) {
> + cerr << "utils::enumerate(<array>) test failed: i=" << i
> + << ", index=" << index << ", value=" << value
> + << std::endl;
> + return TestFail;
> + }
> +
> + ++i;
> + }
> +
> + return TestPass;
> + }
> +
> int run()
> {
> /* utils::hex() test. */
> @@ -177,6 +232,10 @@ protected:
> return TestFail;
> }
>
> + /* utils::enumerate() test. */
> + if (testEnumerate() != TestPass)
> + return TestFail;
> +
> return TestPass;
> }
> };
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
More information about the libcamera-devel
mailing list