[libcamera-devel] [PATCH 01/31] libcamera: Add a C++20-compliant std::span<> implementation

[Kieran Bingham]

Hi Laurent / Jacopo,

On 29/02/2020 16:42, Laurent Pinchart wrote:
> From: Jacopo Mondi <jacopo at jmondi.org>
> 
> C++20 will contain a std::span<> class that represents a contiguous

Will contain ? or contains ...

(Yes, I know it depends on when you wrote that, but it might already be
out-dated)

> sequence of objects. 

How about the following text from the C++20 draft:

"A span is a view over a contiguous sequence of objects, the storage of
which is owned by some other object."

>Its main purpose is to group array pointers and
> size together in APIs.

"It provides a common interface to a contiguous collection of objects
and supports both a static extent where the number of elements is known
and stored as the size, and a dynamic extent where the number of
elements is not known or restricted by the implementation."

(Correct that where necessary or entirely)


> 
> Add a compatible implementation to the utils namespace. This will be
> used to implement array controls.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

I'm not going to admit any kind of real Reviewed-by: on 417 lines of
template magic.

But if this is working, and follows the C++ interfaces, then (with the
commit message improved a little, and possibly dropping span.cpp, and
associated addition in src/libcamera/meson.build which seems pointless)

Acked-by: Kieran Bingham <kieran.bingham at ideasonboard.com>



> ---
>  Documentation/Doxyfile.in     |   4 +-
>  include/libcamera/meson.build |   1 +
>  include/libcamera/span.h      | 417 ++++++++++++++++++++++++++++++++++
>  src/libcamera/meson.build     |   1 +

>  src/libcamera/span.cpp        |  12 +

Do we really need to add an empty span.cpp ?

>  5 files changed, 434 insertions(+), 1 deletion(-)
>  create mode 100644 include/libcamera/span.h
>  create mode 100644 src/libcamera/span.cpp
> 
> diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in
> index beeaf6d3cf48..457e23a086a2 100644
> --- a/Documentation/Doxyfile.in
> +++ b/Documentation/Doxyfile.in
> @@ -840,8 +840,10 @@ RECURSIVE              = YES
>  # Note that relative paths are relative to the directory from which doxygen is
>  # run.
>  
> -EXCLUDE                = @TOP_SRCDIR@/src/libcamera/device_enumerator_sysfs.cpp \
> +EXCLUDE                = @TOP_SRCDIR@/include/libcamera/span.h \
> +			 @TOP_SRCDIR@/src/libcamera/device_enumerator_sysfs.cpp \
>  			 @TOP_SRCDIR@/src/libcamera/device_enumerator_udev.cpp \
> +			 @TOP_SRCDIR@/src/libcamera/span.cpp \

We even exclude this empty file from the doxygen build, so it really
does seem pointless.


>  			 @TOP_SRCDIR@/src/libcamera/include/device_enumerator_sysfs.h \
>  			 @TOP_SRCDIR@/src/libcamera/include/device_enumerator_udev.h \
>  			 @TOP_SRCDIR@/src/libcamera/pipeline/ \
> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
> index f58c02d2cf35..f47c583cbbc0 100644
> --- a/include/libcamera/meson.build
> +++ b/include/libcamera/meson.build
> @@ -14,6 +14,7 @@ libcamera_api = files([
>      'pixelformats.h',
>      'request.h',
>      'signal.h',
> +    'span.h',
>      'stream.h',
>      'timer.h',
>  ])
> diff --git a/include/libcamera/span.h b/include/libcamera/span.h
> new file mode 100644
> index 000000000000..513ddb432405
> --- /dev/null
> +++ b/include/libcamera/span.h
> @@ -0,0 +1,417 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2020, Google Inc.
> + *
> + * span.h - C++20 std::span<> implementation for C++11
> + */
> +
> +#ifndef __LIBCAMERA_SPAN_H__
> +#define __LIBCAMERA_SPAN_H__
> +
> +#include <array>
> +#include <iterator>
> +#include <limits>
> +#include <stddef.h>
> +#include <type_traits>
> +
> +namespace libcamera {
> +
> +static constexpr std::size_t dynamic_extent = std::numeric_limits<std::size_t>::max();
> +
> +template<typename T, std::size_t Extent = dynamic_extent>
> +class Span;
> +
> +namespace details {
> +
> +template<typename U>
> +struct is_array : public std::false_type {
> +};
> +
> +template<typename U, std::size_t N>
> +struct is_array<std::array<U, N>> : public std::true_type {
> +};
> +
> +template<typename U>
> +struct is_span : public std::false_type {
> +};
> +
> +template<typename U, std::size_t Extent>
> +struct is_span<Span<U, Extent>> : public std::true_type {
> +};
> +
> +} /* namespace details */
> +
> +namespace utils {
> +
> +template<typename C>
> +constexpr auto size(const C &c) -> decltype(c.size())
> +{
> +	return c.size();
> +}
> +
> +template<typename C>
> +constexpr auto data(const C &c) -> decltype(c.data())
> +{
> +	return c.data();
> +}
> +
> +template<typename C>
> +constexpr auto data(C &c) -> decltype(c.data())
> +{
> +	return c.data();
> +}
> +
> +template<class T, std::size_t N>
> +constexpr T *data(T (&array)[N]) noexcept
> +{
> +	return array;
> +}
> +
> +template<std::size_t I, typename T>
> +struct tuple_element;
> +
> +template<std::size_t I, typename T, std::size_t N>
> +struct tuple_element<I, Span<T, N>> {
> +	using type = T;
> +};
> +
> +template<typename T>
> +struct tuple_size;
> +
> +template<typename T, std::size_t N>
> +struct tuple_size<Span<T, N>> : public std::integral_constant<std::size_t, N> {
> +};
> +
> +template<typename T>
> +struct tuple_size<Span<T, dynamic_extent>>;
> +
> +} /* namespace utils */
> +
> +template<typename T, std::size_t Extent>
> +class Span
> +{
> +public:
> +	using element_type = T;
> +	using value_type = typename std::remove_cv_t<T>;
> +	using size_type = std::size_t;
> +	using difference_type = std::ptrdiff_t;
> +	using pointer = T *;
> +	using const_pointer = const T *;
> +	using reference = T &;
> +	using const_reference = const T &;
> +	using iterator = pointer;
> +	using const_iterator = const_pointer;
> +	using reverse_iterator = std::reverse_iterator<iterator>;
> +	using const_reverse_iterator = std::reverse_iterator<const_iterator>;
> +
> +	static constexpr std::size_t extent = Extent;
> +
> +	template<bool Dependent = false,
> +		 typename = std::enable_if_t<Dependent || Extent == 0>>
> +	constexpr Span() noexcept
> +		: data_(nullptr)
> +	{
> +	}
> +
> +	constexpr Span(pointer ptr, size_type count)
> +		: data_(ptr)
> +	{
> +	}
> +
> +	constexpr Span(pointer first, pointer last)
> +		: data_(first)
> +	{
> +	}
> +
> +	template<std::size_t N>
> +	constexpr Span(element_type (&arr)[N],
> +		       std::enable_if_t<std::is_convertible<std::remove_pointer_t<decltype(utils::data(arr))> (*)[],
> +							    element_type (*)[]>::value &&
> +					N == Extent,
> +					std::nullptr_t> = nullptr) noexcept
> +		: data_(arr)
> +	{
> +	}
> +
> +	template<std::size_t N>
> +	constexpr Span(std::array<value_type, N> &arr,
> +		       std::enable_if_t<std::is_convertible<std::remove_pointer_t<decltype(utils::data(arr))> (*)[],
> +							    element_type (*)[]>::value &&
> +					N == Extent,
> +					std::nullptr_t> = nullptr) noexcept
> +		: data_(arr.data())
> +	{
> +	}
> +
> +	template<std::size_t N>
> +	constexpr Span(const std::array<value_type, N> &arr,
> +		       std::enable_if_t<std::is_convertible<std::remove_pointer_t<decltype(utils::data(arr))> (*)[],
> +							    element_type (*)[]>::value &&
> +					N == Extent,
> +					std::nullptr_t> = nullptr) noexcept
> +		: data_(arr.data())
> +	{
> +	}
> +
> +	template<class Container>
> +	constexpr Span(Container &cont,
> +		       std::enable_if_t<!details::is_span<Container>::value &&
> +					!details::is_array<Container>::value &&
> +					!std::is_array<Container>::value &&
> +					std::is_convertible<std::remove_pointer_t<decltype(utils::data(cont))> (*)[],
> +							    element_type (*)[]>::value,
> +					std::nullptr_t> = nullptr)
> +		: data_(utils::data(cont))
> +	{
> +	}
> +
> +	template<class Container>
> +	constexpr Span(const Container &cont,
> +		       std::enable_if_t<!details::is_span<Container>::value &&
> +					!details::is_array<Container>::value &&
> +					!std::is_array<Container>::value &&
> +					std::is_convertible<std::remove_pointer_t<decltype(utils::data(cont))> (*)[],
> +							    element_type (*)[]>::value,
> +					std::nullptr_t> = nullptr)
> +		: data_(utils::data(cont))
> +	{
> +		static_assert(utils::size(cont) == Extent, "Size mismatch");
> +	}
> +
> +	template<class U, std::size_t N>
> +	constexpr Span(const Span<U, N> &s,
> +		       std::enable_if_t<std::is_convertible<U (*)[], element_type (*)[]>::value &&
> +					N == Extent,
> +					std::nullptr_t> = nullptr) noexcept
> +		: data_(s.data())
> +	{
> +	}
> +
> +	constexpr Span(const Span &other) noexcept = default;
> +
> +	constexpr Span &operator=(const Span &other) noexcept
> +	{
> +		data_ = other.data_;
> +		return *this;
> +	}
> +
> +	constexpr iterator begin() const { return data(); }
> +	constexpr const_iterator cbegin() const { return begin(); }
> +	constexpr iterator end() const { return data() + size(); }
> +	constexpr const_iterator cend() const { return end(); }
> +	constexpr reverse_iterator rbegin() const { return reverse_iterator(data() + size() - 1); }
> +	constexpr const_reverse_iterator crbegin() const { return rbegin(); }
> +	constexpr reverse_iterator rend() const { return reverse_iterator(data() - 1); }
> +	constexpr const_reverse_iterator crend() const { return rend(); }
> +
> +	constexpr reference front() const { return *data(); }
> +	constexpr reference back() const { return *(data() + size() - 1); }
> +	constexpr reference operator[](size_type idx) const { return data()[idx]; }
> +	constexpr pointer data() const noexcept { return data_; }
> +
> +	constexpr size_type size() const noexcept { return Extent; }
> +	constexpr size_type size_bytes() const noexcept { return size() * sizeof(element_type); }
> +	constexpr bool empty() const noexcept { return size() == 0; }
> +
> +	template<std::size_t Count>
> +	constexpr Span<element_type, Count> first() const
> +	{
> +		static_assert(Count <= Extent, "Count larger than size");
> +		return { data(), Count };
> +	}
> +
> +	constexpr Span<element_type, dynamic_extent> first(std::size_t Count) const
> +	{
> +		return { data(), Count };
> +	}
> +
> +	template<std::size_t Count>
> +	constexpr Span<element_type, Count> last() const
> +	{
> +		static_assert(Count <= Extent, "Count larger than size");
> +		return { data() + size() - Count, Count };
> +	}
> +
> +	constexpr Span<element_type, dynamic_extent> last(std::size_t Count) const
> +	{
> +		return { data() + size() - Count, Count };
> +	}
> +
> +	template<std::size_t Offset, std::size_t Count = dynamic_extent>
> +	constexpr Span<element_type, Count != dynamic_extent ? Count : Extent - Offset> subspan() const
> +	{
> +		static_assert(Offset <= Extent, "Offset larger than size");
> +		static_assert(Count == dynamic_extent || Count + Offset <= Extent,
> +			      "Offset + Count larger than size");
> +		return { data() + Offset, Count == dynamic_extent ? size() - Offset : Count };
> +	}
> +
> +	constexpr Span<element_type, dynamic_extent>
> +	subspan(std::size_t Offset, std::size_t Count = dynamic_extent) const
> +	{
> +		return { data() + Offset, Count == dynamic_extent ? size() - Offset : Count };
> +	}
> +
> +private:
> +	pointer data_;
> +};
> +
> +template<typename T>
> +class Span<T, dynamic_extent>
> +{
> +public:
> +	using element_type = T;
> +	using value_type = typename std::remove_cv_t<T>;
> +	using size_type = std::size_t;
> +	using difference_type = std::ptrdiff_t;
> +	using pointer = T *;
> +	using const_pointer = const T *;
> +	using reference = T &;
> +	using const_reference = const T &;
> +	using iterator = T *;
> +	using const_iterator = const T *;
> +	using reverse_iterator = std::reverse_iterator<iterator>;
> +	using const_reverse_iterator = std::reverse_iterator<const_iterator>;
> +
> +	static constexpr std::size_t extent = dynamic_extent;
> +
> +	constexpr Span() noexcept
> +		: data_(nullptr), size_(0)
> +	{
> +	}
> +
> +	constexpr Span(pointer ptr, size_type count)
> +		: data_(ptr), size_(count)
> +	{
> +	}
> +
> +	constexpr Span(pointer first, pointer last)
> +		: data_(first), size_(last - first)
> +	{
> +	}
> +
> +	template<std::size_t N>
> +	constexpr Span(element_type (&arr)[N],
> +		       std::enable_if_t<std::is_convertible<std::remove_pointer_t<decltype(utils::data(arr))> (*)[],
> +							    element_type (*)[]>::value,
> +					std::nullptr_t> = nullptr) noexcept
> +		: data_(arr), size_(N)
> +	{
> +	}
> +
> +	template<std::size_t N>
> +	constexpr Span(std::array<value_type, N> &arr,
> +		       std::enable_if_t<std::is_convertible<std::remove_pointer_t<decltype(utils::data(arr))> (*)[],
> +							    element_type (*)[]>::value,
> +					std::nullptr_t> = nullptr) noexcept
> +		: data_(utils::data(arr)), size_(N)
> +	{
> +	}
> +
> +	template<std::size_t N>
> +	constexpr Span(const std::array<value_type, N> &arr) noexcept
> +		: data_(utils::data(arr)), size_(N)
> +	{
> +	}
> +
> +	template<class Container>
> +	constexpr Span(Container &cont,
> +		       std::enable_if_t<!details::is_span<Container>::value &&
> +					!details::is_array<Container>::value &&
> +					!std::is_array<Container>::value &&
> +					std::is_convertible<std::remove_pointer_t<decltype(utils::data(cont))> (*)[],
> +							    element_type (*)[]>::value,
> +					std::nullptr_t> = nullptr)
> +		: data_(utils::data(cont)), size_(utils::size(cont))
> +	{
> +	}
> +
> +	template<class Container>
> +	constexpr Span(const Container &cont,
> +		       std::enable_if_t<!details::is_span<Container>::value &&
> +					!details::is_array<Container>::value &&
> +					!std::is_array<Container>::value &&
> +					std::is_convertible<std::remove_pointer_t<decltype(utils::data(cont))> (*)[],
> +							    element_type (*)[]>::value,
> +					std::nullptr_t> = nullptr)
> +		: data_(utils::data(cont)), size_(utils::size(cont))
> +	{
> +	}
> +
> +	template<class U, std::size_t N>
> +	constexpr Span(const Span<U, N> &s,
> +		       std::enable_if_t<std::is_convertible<U (*)[], element_type (*)[]>::value,
> +					std::nullptr_t> = nullptr) noexcept
> +		: data_(s.data()), size_(s.size())
> +	{
> +	}
> +
> +	constexpr Span(const Span &other) noexcept = default;
> +
> +	constexpr Span &operator=(const Span &other) noexcept
> +	{
> +		data_ = other.data_;
> +		size_ = other.size_;
> +		return *this;
> +	}
> +
> +	constexpr iterator begin() const { return data(); }
> +	constexpr const_iterator cbegin() const { return begin(); }
> +	constexpr iterator end() const { return data() + size(); }
> +	constexpr const_iterator cend() const { return end(); }
> +	constexpr reverse_iterator rbegin() const { return reverse_iterator(data() + size() - 1); }
> +	constexpr const_reverse_iterator crbegin() const { return rbegin(); }
> +	constexpr reverse_iterator rend() const { return reverse_iterator(data() - 1); }
> +	constexpr const_reverse_iterator crend() const { return rend(); }
> +
> +	constexpr reference front() const { return *data(); }
> +	constexpr reference back() const { return *(data() + size() - 1); }
> +	constexpr reference operator[](size_type idx) const { return data()[idx]; }
> +	constexpr pointer data() const noexcept { return data_; }
> +
> +	constexpr size_type size() const noexcept { return size_; }
> +	constexpr size_type size_bytes() const noexcept { return size() * sizeof(element_type); }
> +	constexpr bool empty() const noexcept { return size() == 0; }
> +
> +	template<std::size_t Count>
> +	constexpr Span<element_type, Count> first() const
> +	{
> +		return { data(), Count };
> +	}
> +
> +	constexpr Span<element_type, dynamic_extent> first(std::size_t Count) const
> +	{
> +		return { data(), Count };
> +	}
> +
> +	template<std::size_t Count>
> +	constexpr Span<element_type, Count> last() const
> +	{
> +		return { data() + size() - Count, Count };
> +	}
> +
> +	constexpr Span<element_type, dynamic_extent> last(std::size_t Count) const
> +	{
> +		return { data() + size() - Count, Count };
> +	}
> +
> +	template<std::size_t Offset, std::size_t Count = dynamic_extent>
> +	constexpr Span<element_type, Count> subspan() const
> +	{
> +		return { data() + Offset, Count == dynamic_extent ? size() - Offset : Count };
> +	}
> +
> +	constexpr Span<element_type, dynamic_extent>
> +	subspan(std::size_t Offset, std::size_t Count = dynamic_extent) const
> +	{
> +		return { data() + Offset, Count == dynamic_extent ? size() - Offset : Count };
> +	}
> +
> +private:
> +	pointer data_;
> +	size_type size_;
> +};
> +
> +}; /* namespace libcamera */
> +
> +#endif /* __LIBCAMERA_SPAN_H__ */
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index 88658ac563f7..2448f0e96468 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -36,6 +36,7 @@ libcamera_sources = files([
>      'request.cpp',
>      'semaphore.cpp',
>      'signal.cpp',
> +    'span.cpp',
>      'stream.cpp',
>      'thread.cpp',
>      'timer.cpp',
> diff --git a/src/libcamera/span.cpp b/src/libcamera/span.cpp
> new file mode 100644
> index 000000000000..753104765cea
> --- /dev/null
> +++ b/src/libcamera/span.cpp
> @@ -0,0 +1,12 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2020, Google Inc.
> + *
> + * span.h - C++20 std::span<> implementation for C++11

.h? But if this file is dropped then it goes away.


> + */
> +
> +#include <libcamera/span.h>
> +
> +namespace libcamera {
> +
> +} /* namespace libcamera */
> 


Do we actually need span.cpp at all?

-- 
Regards
--
Kieran