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  <phdoftheho...@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>.


Reply via email to