[ PATCH ] C++20 <span>

[Jonathan Wakely]

On 30/08/19 15:22 -0400, JeanHeyd Meneide wrote:
>This patch implements <span> as it currently exists in the C++20 Working Draft.

Nice!


>Notes:
>- __gnu_cxx::__normal_iterator is not fully constexpr, so its not used here

I'd prefer to make __normal_iterator constexpr, and use it.
It needs to be constexpr anyway for string and vector.

>- P1394 might be slated to end up in C++20 per National Body Comments.
>Therefore, an early implementation is left in an
>implementation-defined _GLIBCXX_P1394 define.
>
>2019-08-30  JeanHeyd "ThePhD" Meneide  <phdofthehouse@gmail.com>
>
>        * include/std/span: Implement the entirety of span.
>        * include/bits/range_access.h: Add __adl_* versions of access functions.
>        * testsuite/23_containers/span/everything.cc: constexpr and
>non-constexpr tests.
>        * include/Makefile.in: Add span to install.
>        * include/Makefile.am: Likewise

>+++ b/libstdc++-v3/include/std/span
>@@ -0,0 +1,549 @@
>+// Components for manipulating non-owning sequences of objects -*- C++ -*-
>+
>+// Copyright (C) 2019-2019 Free Software Foundation, Inc.

Just 2019 please, not 2019-2019.

>+// WARNING: they forgot this feature test macro
>+// get on someone's back about it in Belfast!!!

Please use FIXME: instead of WARNING: (for consistency with the rest
of the sources, so people grepping for FIXME: can find this).

The new feature test macro should be in <version> too.

There's no need to qualify ::std::true_type and ::std::size_t when
within namespace std already. There's no ADL for type names, and
normal unqualified lookup will find the right ones (and is easier to
read).

>+      static_assert(
>+        _Count == ::std::dynamic_extent || _Extent == ::std::dynamic_extent || _Count <= _Extent,
>+        "bad span length");

There are a number of lines that are too long, they need to be broken
before 80 columns.

Our static_assert messages should be stated as the positive condition
that is being asserted. So the diagnostic reads like
"assertion failed: thing being asserted"

So "bad span length" makes it look like we asserted the length is bad,
but actually it was good. I prefer to write something saying "X must
be true", e.g. "count must be equal to dynamic_extent, or less than
the span's extent".

Do we need to check _Extent == ::std::dynamic_extent here, give nthat
if it's true then _Count <= _Extent will be true as well?

>+  std::vector<std::int_least32_t> value{ 0 };

Your new testcase uses std::vector without including <vector>.