From afb8d60c49229832dbce2a13532bccc67d0a003a Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Wed, 18 Dec 2024 16:58:00 +0100 Subject: [PATCH] refactor: Make Span an alias of std::span This uses a macro, which can be a bit more brittle than an alias template. However, class template argument deduction for alias templates is only implemented in clang-19. --- doc/developer-notes.md | 4 +- src/span.h | 174 +--------------------------------------- src/test/span_tests.cpp | 7 -- 3 files changed, 3 insertions(+), 182 deletions(-) diff --git a/doc/developer-notes.md b/doc/developer-notes.md index 63cc6c91db0..cfa3ae7f16e 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -856,14 +856,14 @@ class A - *Rationale*: Easier to understand what is happening, thus easier to spot mistakes, even for those that are not language lawyers. -- Use `Span` as function argument when it can operate on any range-like container. +- Use `std::span` as function argument when it can operate on any range-like container. - *Rationale*: Compared to `Foo(const vector&)` this avoids the need for a (potentially expensive) conversion to vector if the caller happens to have the input stored in another type of container. However, be aware of the pitfalls documented in [span.h](../src/span.h). ```cpp -void Foo(Span data); +void Foo(std::span data); std::vector vec{1,2,3}; Foo(vec); diff --git a/src/span.h b/src/span.h index 2c984aa7ca4..829738ca605 100644 --- a/src/span.h +++ b/src/span.h @@ -11,31 +11,7 @@ #include #include -#ifdef DEBUG -#define CONSTEXPR_IF_NOT_DEBUG -#define ASSERT_IF_DEBUG(x) assert((x)) -#else -#define CONSTEXPR_IF_NOT_DEBUG constexpr -#define ASSERT_IF_DEBUG(x) -#endif - -#if defined(__clang__) -#if __has_attribute(lifetimebound) -#define SPAN_ATTR_LIFETIMEBOUND [[clang::lifetimebound]] -#else -#define SPAN_ATTR_LIFETIMEBOUND -#endif -#else -#define SPAN_ATTR_LIFETIMEBOUND -#endif - /** A Span is an object that can refer to a contiguous sequence of objects. - * - * This file implements a subset of C++20's std::span. It can be considered - * temporary compatibility code until C++20 and is designed to be a - * self-contained abstraction without depending on other project files. For this - * reason, Clang lifetimebound is defined here instead of including - * , which also defines it. * * Things to be aware of when writing code that deals with Spans: * @@ -93,155 +69,7 @@ * result will be present in that variable after the call. Passing a temporary * is useless in that context. */ -template -class Span -{ - C* m_data; - std::size_t m_size{0}; - - template - struct is_Span_int : public std::false_type {}; - template - struct is_Span_int> : public std::true_type {}; - template - struct is_Span : public is_Span_int::type>{}; - - -public: - constexpr Span() noexcept : m_data(nullptr) {} - - /** Construct a span from a begin pointer and a size. - * - * This implements a subset of the iterator-based std::span constructor in C++20, - * which is hard to implement without std::address_of. - */ - template ::value, int>::type = 0> - constexpr Span(T* begin, std::size_t size) noexcept : m_data(begin), m_size(size) {} - - /** Construct a span from a begin and end pointer. - * - * This implements a subset of the iterator-based std::span constructor in C++20, - * which is hard to implement without std::address_of. - */ - template ::value, int>::type = 0> - CONSTEXPR_IF_NOT_DEBUG Span(T* begin, T* end) noexcept : m_data(begin), m_size(end - begin) - { - ASSERT_IF_DEBUG(end >= begin); - } - - /** Implicit conversion of spans between compatible types. - * - * Specifically, if a pointer to an array of type O can be implicitly converted to a pointer to an array of type - * C, then permit implicit conversion of Span to Span. This matches the behavior of the corresponding - * C++20 std::span constructor. - * - * For example this means that a Span can be converted into a Span. - */ - template ::value, int>::type = 0> - constexpr Span(const Span& other) noexcept : m_data(other.m_data), m_size(other.m_size) {} - - /** Default copy constructor. */ - constexpr Span(const Span&) noexcept = default; - - /** Default assignment operator. */ - Span& operator=(const Span& other) noexcept = default; - - /** Construct a Span from an array. This matches the corresponding C++20 std::span constructor. */ - template - constexpr Span(C (&a)[N]) noexcept : m_data(a), m_size(N) {} - - /** Construct a Span for objects with .data() and .size() (std::string, std::array, std::vector, ...). - * - * This implements a subset of the functionality provided by the C++20 std::span range-based constructor. - * - * To prevent surprises, only Spans for constant value types are supported when passing in temporaries. - * Note that this restriction does not exist when converting arrays or other Spans (see above). - */ - template - constexpr Span(V& other SPAN_ATTR_LIFETIMEBOUND, - typename std::enable_if::value && - std::is_convertible().data())>::type (*)[], C (*)[]>::value && - std::is_convertible().size()), std::size_t>::value, std::nullptr_t>::type = nullptr) - : m_data(other.data()), m_size(other.size()){} - - template - constexpr Span(const V& other SPAN_ATTR_LIFETIMEBOUND, - typename std::enable_if::value && - std::is_convertible().data())>::type (*)[], C (*)[]>::value && - std::is_convertible().size()), std::size_t>::value, std::nullptr_t>::type = nullptr) - : m_data(other.data()), m_size(other.size()){} - - constexpr C* data() const noexcept { return m_data; } - constexpr C* begin() const noexcept { return m_data; } - constexpr C* end() const noexcept { return m_data + m_size; } - CONSTEXPR_IF_NOT_DEBUG C& front() const noexcept - { - ASSERT_IF_DEBUG(size() > 0); - return m_data[0]; - } - CONSTEXPR_IF_NOT_DEBUG C& back() const noexcept - { - ASSERT_IF_DEBUG(size() > 0); - return m_data[m_size - 1]; - } - constexpr std::size_t size() const noexcept { return m_size; } - constexpr std::size_t size_bytes() const noexcept { return sizeof(C) * m_size; } - constexpr bool empty() const noexcept { return size() == 0; } - CONSTEXPR_IF_NOT_DEBUG C& operator[](std::size_t pos) const noexcept - { - ASSERT_IF_DEBUG(size() > pos); - return m_data[pos]; - } - CONSTEXPR_IF_NOT_DEBUG Span subspan(std::size_t offset) const noexcept - { - ASSERT_IF_DEBUG(size() >= offset); - return Span(m_data + offset, m_size - offset); - } - CONSTEXPR_IF_NOT_DEBUG Span subspan(std::size_t offset, std::size_t count) const noexcept - { - ASSERT_IF_DEBUG(size() >= offset + count); - return Span(m_data + offset, count); - } - CONSTEXPR_IF_NOT_DEBUG Span first(std::size_t count) const noexcept - { - ASSERT_IF_DEBUG(size() >= count); - return Span(m_data, count); - } - CONSTEXPR_IF_NOT_DEBUG Span last(std::size_t count) const noexcept - { - ASSERT_IF_DEBUG(size() >= count); - return Span(m_data + m_size - count, count); - } - - template friend class Span; -}; - -// Return result of calling .data() method on type T. This is used to be able to -// write template deduction guides for the single-parameter Span constructor -// below that will work if the value that is passed has a .data() method, and if -// the data method does not return a void pointer. -// -// It is important to check for the void type specifically below, so the -// deduction guides can be used in SFINAE contexts to check whether objects can -// be converted to spans. If the deduction guides did not explicitly check for -// void, and an object was passed that returned void* from data (like -// std::vector), the template deduction would succeed, but the Span -// object instantiation would fail, resulting in a hard error, rather than a -// SFINAE error. -// https://stackoverflow.com/questions/68759148/sfinae-to-detect-the-explicitness-of-a-ctad-deduction-guide -// https://stackoverflow.com/questions/16568986/what-happens-when-you-call-data-on-a-stdvectorbool -template -using DataResult = std::remove_pointer_t().data())>; - -// Deduction guides for Span -// For the pointer/size based and iterator based constructor: -template Span(T*, EndOrSize) -> Span; -// For the array constructor: -template Span(T (&)[N]) -> Span; -// For the temporaries/rvalue references constructor, only supporting const output. -template Span(T&&) -> Span && !std::is_void_v>, const DataResult>>; -// For (lvalue) references, supporting mutable output. -template Span(T&) -> Span>, DataResult>>; +#define Span std::span /** Pop the last element off a span, and return a reference to that element. */ template diff --git a/src/test/span_tests.cpp b/src/test/span_tests.cpp index 5a0348e77cb..6cb0d006f95 100644 --- a/src/test/span_tests.cpp +++ b/src/test/span_tests.cpp @@ -47,13 +47,6 @@ BOOST_AUTO_TEST_SUITE(span_tests) // don't work. This makes it is possible to use the Span constructor in a SFINAE // contexts like in the Spannable function above to detect whether types are or // aren't compatible with Spans at compile time. -// -// Previously there was a bug where writing a SFINAE check for vector was -// not possible, because in libstdc++ vector has a data() member -// returning void, and the Span template guide ignored the data() return value, -// so the template substitution would succeed, but the constructor would fail, -// resulting in a fatal compile error, rather than a SFINAE error that could be -// handled. BOOST_AUTO_TEST_CASE(span_constructor_sfinae) { BOOST_CHECK(Spannable(std::vector{}));