[libc++][docs] Mark LWG3274 as complete

[jloser]

Summary

Mark LWG3274 as complete. The feature test macro __cpp_lib_span was added in
6d2599e4f776d0cd88438cb82a00c4fc25cc3f67.

https://wg21.link/p1024 mentions marking span:::empty() with
[[nodiscard]] which is not done yet. So, do that and add tests.

[• Quuxplusone]

libcxx/docs/Status/Cxx20Issues.csv

186

The feature test macro has indeed been there since forever; but before we mark this "Complete", let's do a pass over the paper that added it
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1024r3.pdf
and see what else might be missing.

• span::empty() should be [[nodiscard]]
• tuple_size should be specialized for spans of non-dynamic extent... nope, this part was removed again in https://cplusplus.github.io/LWG/issue3212 (which is associated with https://wg21.link/P2116 which is already listed as "Complete", so, no action needed on this one)

I think adding [[nodiscard]] to empty() — and writing a test for that — is all that's needed, then.

[jloser]

Mark span::empty as [[nodiscard]] and add tests.

[• Quuxplusone]

LGTM!

[Mordante]

Two small nits, but I'd like to see the CI passing before approving.

libcxx/include/span

333–335

Since the block was manually aligned it would be nice to keep it that way.

libcxx/test/std/containers/views/span.obs/empty.nodiscard.verify.cpp

19

Minor nit can you remove the // and the extra blank line?

[jloser]

libcxx/include/span

333–335

With adding [[nodiscard]] it won't be aligned with size() and size_bytes() member function. So, we can either manually align those, or not align empty() member function. I chose the latter. WDYT?

libcxx/test/std/containers/views/span.obs/empty.nodiscard.verify.cpp

19

Sure, but it's consistent with empty.pass where this was copied from for example. Several other tests have this same pattern.

[jloser]

[NFC] Remove extra blank line in empty.nodiscard.verify.cpp

[jloser]

In D111516#3055330, @Mordante wrote:

Two small nits, but I'd like to see the CI passing before approving.

I think CI is failing due to the unexpectedly pass test due to the copied XFAIL from empty.pass.cpp. Just removed the XFAIL.

[jloser]

Remove XFAIL from test

[• Quuxplusone]

libcxx/include/span

333–335

Yeah, I think @Mordante was asking for

_LIBCPP_INLINE_VISIBILITY constexpr size_type size()           const noexcept { return _Extent; }
_LIBCPP_INLINE_VISIBILITY constexpr size_type size_bytes()     const noexcept { return _Extent * sizeof(element_type); }
[[nodiscard]] _LIBCPP_INLINE_VISIBILITY constexpr bool empty() const noexcept { return _Extent == 0; }

FWIW, I think it's fine either way, and would err on the side of the smaller diff (as you did) myself, but I don't care.

[Mordante]

libcxx/include/span

333–335

Yes that was indeed what I was asking for. It's just a bit odd; on one hand we spend time to make code looking nicely in the original commit, but one the other hand we want to keep our diffs minimal. But it was a suggestion so no blocker.

[jloser]

[NFC] Align member functions near empty() in span

[jloser]

libcxx/include/span

333–335

I just aligned the neighboring member functions. I don't feel strongly either way. Like Arthur said, I like to keep the diff as small as possible, but I hear your argument around making the code look nice.

In any case, CI passed earlier before the whitespace alignment (https://buildkite.com/llvm-project/libcxx-ci/builds/5884). I'll land this later tonight.

[Mordante]

LGTM!

libcxx/docs/Status/Cxx20Issues.csv

 "`3257 <https://wg21.link/LWG3257>`__","Missing feature testing macro update from P0858","Belfast","",""
 "`3256 <https://wg21.link/LWG3256>`__","Feature testing macro for ``constexpr``\  algorithms","Belfast","|Complete|","13.0"
 "`3273 <https://wg21.link/LWG3273>`__","Specify ``weekday_indexed``\  to range of ``[0, 7]``\ ","Belfast","",""
 "`3070 <https://wg21.link/LWG3070>`__","``path::lexically_relative``\  causes surprising results if a filename can also be a  *root-name*","Belfast","",""
 "`3266 <https://wg21.link/LWG3266>`__","``to_chars(bool)``\  should be deleted","Belfast","",""
 "`3272 <https://wg21.link/LWG3272>`__","``%I%p``\  should parse/format ``duration``\  since midnight","Belfast","",""
 "`3259 <https://wg21.link/LWG3259>`__","The definition of *constexpr iterators* should be adjusted","Belfast","",""
 "`3103 <https://wg21.link/LWG3103>`__","Errors in taking subview of ``span``\  should be ill-formed where possible","Belfast","",""
-"`3274 <https://wg21.link/LWG3274>`__","Missing feature test macro for ``<span>``\ ","Belfast","",""
+"`3274 <https://wg21.link/LWG3274>`__","Missing feature test macro for ``<span>``\ ","Belfast","|Complete|","11.0"
 "`3276 <https://wg21.link/LWG3276>`__","Class ``split_view::outer_iterator::value_type``\  should inherit from ``view_interface``\ ","Belfast","",""
 "`3277 <https://wg21.link/LWG3277>`__","Pre-increment on prvalues is not a requirement of ``weakly_incrementable``\ ","Belfast","",""
 "`3149 <https://wg21.link/LWG3149>`__","``DefaultConstructible``\  should require default initialization","Belfast","|Complete|","13.0"
 "","","","",""
 "`1203 <https://wg21.link/LWG1203>`__","More useful rvalue stream insertion","Prague","|Complete|","12.0"
 "`2859 <https://wg21.link/LWG2859>`__","Definition of *reachable* in [ptr.launder] misses pointer arithmetic from pointer-interconvertible object","Prague","",""
 "`3018 <https://wg21.link/LWG3018>`__","``shared_ptr``\  of function type","Prague","",""
 "`3050 <https://wg21.link/LWG3050>`__","Conversion specification problem in ``chrono::duration``\  constructor","Prague","",""

libcxx/include/span

public:
 
     constexpr span<element_type, dynamic_extent> first(size_type count) const;
     constexpr span<element_type, dynamic_extent> last(size_type count) const;
     constexpr span<element_type, dynamic_extent> subspan(size_type offset, size_type count = dynamic_extent) const;
 
     // [span.obs], span observers
     constexpr size_type size() const noexcept;
     constexpr size_type size_bytes() const noexcept;
-    constexpr bool empty() const noexcept;
+    [[nodiscard]] constexpr bool empty() const noexcept;
 
     // [span.elem], span element access
     constexpr reference operator[](size_type idx) const;
     constexpr reference front() const;
     constexpr reference back() const;
     constexpr pointer data() const noexcept;
 
     // [span.iterators], span iterator support
constexpr span<element_type, dynamic_extent>
         _LIBCPP_ASSERT(__offset <= size(), "Offset out of range in span::subspan(offset, count)");
         _LIBCPP_ASSERT(__count  <= size() || __count == dynamic_extent, "Count out of range in span::subspan(offset, count)");
         if (__count == dynamic_extent)
             return {data() + __offset, size() - __offset};
         _LIBCPP_ASSERT(__count <= size() - __offset, "Offset + count out of range in span::subspan(offset, count)");
         return {data() + __offset, __count};
     }
 
-    _LIBCPP_INLINE_VISIBILITY constexpr size_type size()       const noexcept { return _Extent; }
-    _LIBCPP_INLINE_VISIBILITY constexpr size_type size_bytes() const noexcept { return _Extent * sizeof(element_type); }
-    _LIBCPP_INLINE_VISIBILITY constexpr bool empty()           const noexcept { return _Extent == 0; }
+    _LIBCPP_INLINE_VISIBILITY constexpr size_type size()           const noexcept { return _Extent; }
+    _LIBCPP_INLINE_VISIBILITY constexpr size_type size_bytes()     const noexcept { return _Extent * sizeof(element_type); }
+    [[nodiscard]] _LIBCPP_INLINE_VISIBILITY constexpr bool empty() const noexcept { return _Extent == 0; }
 
     _LIBCPP_INLINE_VISIBILITY constexpr reference operator[](size_type __idx) const noexcept
     {
         _LIBCPP_ASSERT(__idx < size(), "span<T,N>[] index out of bounds");
         return __data[__idx];
     }
 
     _LIBCPP_INLINE_VISIBILITY constexpr reference front() const noexcept
subspan(size_type __offset, size_type __count = dynamic_extent) const noexcept
         _LIBCPP_ASSERT(__offset <= size(), "Offset out of range in span::subspan(offset, count)");
         _LIBCPP_ASSERT(__count  <= size() || __count == dynamic_extent, "count out of range in span::subspan(offset, count)");
         if (__count == dynamic_extent)
             return {data() + __offset, size() - __offset};
         _LIBCPP_ASSERT(__count <= size() - __offset, "Offset + count out of range in span::subspan(offset, count)");
         return {data() + __offset, __count};
     }
 
-    _LIBCPP_INLINE_VISIBILITY constexpr size_type size()       const noexcept { return __size; }
-    _LIBCPP_INLINE_VISIBILITY constexpr size_type size_bytes() const noexcept { return __size * sizeof(element_type); }
-    _LIBCPP_INLINE_VISIBILITY constexpr bool empty()           const noexcept { return __size == 0; }
+    _LIBCPP_INLINE_VISIBILITY constexpr size_type size()           const noexcept { return __size; }
+    _LIBCPP_INLINE_VISIBILITY constexpr size_type size_bytes()     const noexcept { return __size * sizeof(element_type); }
+    [[nodiscard]] _LIBCPP_INLINE_VISIBILITY constexpr bool empty() const noexcept { return __size == 0; }
 
     _LIBCPP_INLINE_VISIBILITY constexpr reference operator[](size_type __idx) const noexcept
     {
         _LIBCPP_ASSERT(__idx < size(), "span<T>[] index out of bounds");
         return __data[__idx];
     }
 
     _LIBCPP_INLINE_VISIBILITY constexpr reference front() const noexcept

libcxx/test/std/containers/views/span.obs/empty.nodiscard.verify.cpp

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+// UNSUPPORTED: libcpp-has-no-incomplete-ranges
+
+// <span>
+
+// [[nodiscard]] constexpr bool empty() const noexcept;
+
+#include <span>
+
+void test() {
+  std::span<int> s1;
+  s1.empty(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+
+  int arr[] = {1, 2, 3};
+  std::span<int, 3> s2{arr};
+  s2.empty(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+}

libcxx/test/std/containers/views/span.obs/empty.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 // UNSUPPORTED: c++03, c++11, c++14, c++17
 // UNSUPPORTED: libcpp-has-no-incomplete-ranges
 
 // AppleClang 12.0.0 doesn't fully support ranges/concepts
 // XFAIL: apple-clang-12.0.0
 
 // <span>
 
-// constexpr bool empty() const noexcept;
+// [[nodiscard]] constexpr bool empty() const noexcept;
 //
 
 
 #include <span>
 #include <cassert>
 #include <string>
 
 #include "test_macros.h"