[ PATCH ] C++20 <span>
Jonathan Wakely
jwakely@redhat.com
Thu Sep 5 13:52:00 GMT 2019
On 05/09/19 12:27 +0100, Jonathan Wakely wrote:
>On 04/09/19 18:47 -0400, JeanHeyd Meneide wrote:
>> Thank you for the thorough review!
>>
>>On Tue, Sep 3, 2019 at 9:31 AM Jonathan Wakely <jwakely@redhat.com> wrote:
>>>It looks like__adl_begin and __adl_end should be guarded by #ifdef
>>>_GLIBCXX_P1394 because they're not needed otherwise.
>>
>> I'm absolutely going to need it for the bit data structures (and
>>in tuning up other parts of the library). It will also be important
>>for things that need to behave nicely when working with things needing
>>[range.access] internally in libstdc++,
>>while we wait for the cmcstl2 implementation. I did gate
>>__adl_to_address behind p1394, though, because that is definitely
>>specific to that paper (and its friend, p1474).
>
>OK, then it's fine as is, thanks.
>
>>+/** @file span
>>+ * This is a Standard C++ Library header.
>>+ */
>>+
>>+//
>>+// P0122 span library
>>+// Contributed by ThePhD
>>+//
>>+
>>+#ifndef _GLIBCXX_SPAN
>>+#define _GLIBCXX_SPAN 1
>>+
>>+#pragma GCC system_header
>>+
>>+#if __cplusplus > 201703L
>>+
>>+#include <cstddef>
>
>This header isn't needed. <bits/c++config.h> defines std::size_t and
>std::ptrdiff_t, and every libstdc++ header should include
><bits/c++config.h> already. Not including <cstddef> is a minor
>optimisation for compile time, because we don't need to open <cstddef>
>and <stddef.h>.
>
>>+ template<typename _Tp, size_t _Num>
>>+ inline constexpr bool
>>+ __is_std_array_v<_GLIBCXX_STD_C::array<_Tp, _Num>> = true;
>>+
>>+#ifdef _GLIBCXX_DEBUG
>>+ template<typename _Tp, size_t _Num>
>>+ inline constexpr bool
>>+ __is_std_array_v<std::__debug::array<_Tp, _Num>> = true;
>>+#endif // debug/array
>>+
>>+ template<size_t _Extent>
>>+ class __extent_storage
>>+ {
>>+ public:
>>+
>>+ constexpr
>>+ __extent_storage() noexcept = default;
>
>Something else I forgot to mention (which I'll add to the coding
>conventions documentation) is that we use TABs for indentation at the
>start of lines. Tabstop should be set to 8, so anything indented by 8
>chars or more should use TABs. Anything indented by fewer than 8 chars
>just uses spaces.
>
>>+
>>+ constexpr
>>+ __extent_storage(size_t) noexcept
>>+ {}
>>+
>>+ static constexpr size_t
>>+ _M_extent() noexcept
>>+ { return _Extent; }
>>+ };
>>+
>>+ template<>
>>+ class __extent_storage<static_cast<size_t>(-1)>
>>+ {
>>+ public:
>>+
>>+ constexpr
>>+ __extent_storage() noexcept : _M_extent_value(0){};
>>+
>>+ constexpr
>>+ __extent_storage(size_t __extent) noexcept
>>+ : _M_extent_value(__extent)
>>+ {}
>>+
>>+ constexpr size_t
>>+ _M_extent() const noexcept
>>+ { return this->_M_extent_value; }
>>+
>>+ private:
>>+ size_t _M_extent_value;
>>+ };
>>+
>>+ } // namespace __detail
>>+
>>+ template<typename _Type, size_t _Extent = dynamic_extent>
>>+ class span : private __detail::__extent_storage<_Extent>
>>+ {
>>+ public:
>>+ // member types
>>+ using value_type = remove_cv_t<_Type>;
>>+ using element_type = _Type;
>>+ using index_type = size_t;
>>+ using reference = element_type&;
>>+ using const_reference = const element_type&;
>>+ using pointer = _Type*;
>>+ using const_pointer = const _Type*;
>>+ using iterator = __gnu_cxx::__normal_iterator<pointer, span>;
>>+ using const_iterator = __gnu_cxx::__normal_iterator<const_pointer, span>;
>>+ using reverse_iterator = ::std::reverse_iterator<iterator>;
>>+ using const_reverse_iterator = ::std::reverse_iterator<const_iterator>;
>>+ using difference_type = ptrdiff_t;
>>+ // Official wording has no size_type -- why??
>>+ // using size_type = size_t;
>>+
>>+ // member constants
>>+ static inline constexpr size_t extent = _Extent;
>>+
>>+ private:
>>+ using __base_t = __detail::__extent_storage<extent>;
>>+
>>+ public:
>>+ // constructors
>>+ template <typename _Dummy = _Type, enable_if_t<is_same_v<_Dummy, _Type> && (_Extent == dynamic_extent || _Extent == 0)>* = nullptr>
>
>This line is too long, it needs to be split before 80 columns.
>
>>+ constexpr span() noexcept : __base_t(0), _M_ptr(nullptr)
>>+ {}
>>+
>>+ constexpr
>>+ span(const span&) noexcept = default;
>>+
>>+ template<size_t _ArrayExtent,
>>+ enable_if_t<
>>+ (_Extent == dynamic_extent || _ArrayExtent == _Extent) &&
>
>N.B. line breaks should come before logical operators like && e.g.
>
> foo
> && bar
>
>Not:
>
> foo &&
> bar
>
>The rationale for this is to make it easy to see the operator. You can
>look at the start of the line (at a predictable, easily findable
>position) instead of having to look at the right hand side to see if
>it's a && or || operator.
>
>
>>+ template<typename _Container,
>>+ enable_if_t<
>>+ (_Extent == dynamic_extent) &&
>>+ !is_same_v<remove_cvref_t<_Container>, span> &&
>>+ !__detail::__is_std_array_v<remove_cvref_t<_Container>> &&
>>+ !is_array_v<remove_cvref_t<_Container>> &&
>
>Here's a good example where the && operators are all at different
>columns, but if you put line breaks before the operator it looks like:
>
> (_Extent == dynamic_extent)
> && !is_same_v<remove_cvref_t<_Container>, span>
> && !__detail::__is_std_array_v<remove_cvref_t<_Container>>
> && !is_array_v<remove_cvref_t<_Container>>
> && ...
>
>This is easier to see that it's a series of AND conditions with a
>quick glance.
>
>As discussed on IRC the new XFAIL tests need some adjustment. The
>attached patch changes the whitespace as discussed above, and fixes
>the tests.
>
>Our convention for XFAIL tests is to call them foo_neg.cc (not
>fail.foo.cc like libc++ uses).
>
>The attached patch is what I've tested and committed to trunk - thanks
>very much!
>
>I'll make some additional fixes (e.g. LWG 3103) and update the docs
>later today.
Here's the follow-up patch.
Tested x86_64-linux, committed to trunk.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch.txt
Type: text/x-patch
Size: 43109 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/libstdc++/attachments/20190905/4dd1813f/attachment.bin>
More information about the Libstdc++
mailing list