[libc++] Verify span and string_view are trivially copyable

[jloser]

Summary

Implement P2251 which requires span and basic_string_view to be
trivially copyable. They already are - this just adds tests to bind that
behavior.

[jloser]

Explicitly include <type_traits> in tests

[• Quuxplusone]

libcxx/test/std/containers/views/trivially_copyable.compile.pass.cpp

19–24

(1) s/Trival/Trivial/g
(2) I don't think lines 18–23 are needed at all. We can trust that span doesn't do anything stupidly depending on the type-the-pointer-points-to.

libcxx/test/std/strings/string.view/trivially_copyable.compile.pass.cpp

10

This surely needs some // UNSUPPORTED: c++03, c++11, c++14, but buildkite will tell you for sure.
...Actually, it looks like libc++ supports string_view as an extension all the way back to C++03? is that true? So you'll want to use static_assert(std::is_trivially_copyable<...>::value, ""); below.

[jloser]

Address review feedback

libcxx/test/std/containers/views/trivially_copyable.compile.pass.cpp

19–24

Removed them - I agree it was a bit far. For example, I thought about adding an evil custom char traits class to use as a template in the other test for string_view, but I thought it was a bit overkill.

libcxx/test/std/strings/string.view/trivially_copyable.compile.pass.cpp

10

Seems to be the case that libc++ supports string_view all the way back to C++03 oddly enough. Just fixed up the static_asserts to play nicely with older modes.

[Mordante]

In general looks good, just want to discuss the supported versions of the tests before approving.

libcxx/test/std/containers/views/trivially_copyable.compile.pass.cpp

9

A bit pedantic, but this is only required in C++23 not in C++20. Can you add a comment. (P2251 was not retroactively applied.)

libcxx/test/std/strings/string.view/trivially_copyable.compile.pass.cpp

10

libc++ originally "back-ported" several features to older standards. This gives us occasionally additional maintenance work. Nowadays we no longer back-port features.

10

I wonder whether we should disable the test on older versions. This test will fail on MSVC STL in C++11 mode. Looking at cppreference.com I expect other implementations to be trivially copyable from the start.

[jloser]

libcxx/test/std/containers/views/trivially_copyable.compile.pass.cpp

9

I can add a comment about it just working or mark it as UNSUPPORTED: c++20. Thanks for pointing out it doesn't retroactively apply to other standards modes. I don't feel strongly.

@ldionne @Quuxplusone - do either of you have strong opinions?

libcxx/test/std/strings/string.view/trivially_copyable.compile.pass.cpp

10

I'd be OK marking this test as unsupported for all standards except C++23.

[ldionne]

We need to mark this paper as implemented in the status page. The Status pages haven't been updated for yesterday's meeting yet - let me do that now and then you can rebase on top.

libcxx/test/std/containers/views/trivially_copyable.compile.pass.cpp

9

I would do

// UNSUPPORTED: c++03, c++11, c++14, c++17

// P2251 is supported by libc++ even in C++20 mode.
// UNSUPPORTED: !stdlib=libc++ && c++20

libcxx/test/std/strings/string.view/trivially_copyable.compile.pass.cpp

9

Can you add a comment explaining what this test is testing? Include a link to P2251.

10

Here's what I suggest:

// P2251 was voted into C++23, but libc++ guarantees triviality in all Standard modes,
// so we enable the test in all Standard modes for libc++.
// UNSUPPORTED: !stdlib=libc++ && (c++03 || c++11 || c++14 || c++17 || c++20)

Not the prettiest UNSUPPORTED annotation I've seen, but it does the job.

[jloser]

In D111197#3046008, @ldionne wrote:

We need to mark this paper as implemented in the status page. The Status pages haven't been updated for yesterday's meeting yet - let me do that now and then you can rebase on top.

@ldionne note https://reviews.llvm.org/D111166 is in flight. Let's have that land first and then I'll rebase with it and mark P2251 as complete in this PR.

[ldionne]

In D111197#3046016, @jloser wrote:

In D111197#3046008, @ldionne wrote:

We need to mark this paper as implemented in the status page. The Status pages haven't been updated for yesterday's meeting yet - let me do that now and then you can rebase on top.

@ldionne note https://reviews.llvm.org/D111166 is in flight. Let's have that land first and then I'll rebase with it and mark P2251 as complete in this PR.

I just saw that! Thanks :)

[• Quuxplusone]

libcxx/test/std/strings/string.view/trivially_copyable.compile.pass.cpp

10

If we really care about !stdlib=libc++ cases that hypothetically might not have backported this fix, then shouldn't we also start caring about !stdlib=libc++ cases that don't support string_view in C++11 or C++14 at all?
I think we should just be simple and aggressive with both of these tests, and then if someone actually turns up a third-party library that fails these tests, then we can PR how to disable them.

[jloser]

Address review feedback

[jloser]

Guard against char8_t not existing

[• Quuxplusone]

libcxx/test/std/strings/string.view/trivially_copyable.compile.pass.cpp

21

#ifndef _LIBCPP_HAS_NO_CHAR8_T plz

[DavidSpickett]

FYI, gdb_pretty_printer_test.sh.cpp failures in CI should go away once you rebase.

[Mordante]

LGTM modulo my last comment.

libcxx/test/std/strings/string.view/trivially_copyable.compile.pass.cpp

21

Actually I think we can remove the #ifdef since we only support compilers providing char8_t. In D110868 I've started to make the support unconditional, but that was put on hold due to the Buildkite issues. (For now I'll wait until the @ldionne's wchar_t changes have landed.) If you want to add an #ifdef please use @Quuxplusone's suggestion.

[jloser]

Use _LIBCPP_HAS_NO_CHAR8_T

[jloser]

libcxx/test/std/strings/string.view/trivially_copyable.compile.pass.cpp

21

The issue is that clang only supports char8_t itself in C++20 or later, but this test is run in C++03 or later, so the #ifdef is needed. Switched to using #ifndef _LIBCPP_HAS_NO_CHAR8_T.

[Mordante]

libcxx/test/std/strings/string.view/trivially_copyable.compile.pass.cpp

21

Of course this is the string_view not the span. In that case I think it would be slightly better to guard against C++20, but I probably will update when I continue with D110868.

[jloser]

libcxx/test/std/strings/string.view/trivially_copyable.compile.pass.cpp

10

I like the idea of this being more consistent with our other tests in libcxx/test of having these tests work for other standard library implementations. And if one of them fails, then we can mark it as XFAIL or UNSUPPORTED.

Thoughts @ldionne?

[ldionne]

libcxx/test/std/strings/string.view/trivially_copyable.compile.pass.cpp

10

Okay, I agree. So I guess we can just remove the UNSUPPORTED line entirely then.

[ldionne]

LGTM once you update on top of D111166!

[jloser]

Rebase and mark P2251 complete in Cxx2bPapers.csv

If BuildKite is still happy, I'll land this as-is.

[jloser]

Run the tests on all vendors for all relevant standards. Let's see if any fail.

[jloser]

libcxx/test/std/strings/string.view/trivially_copyable.compile.pass.cpp

10

Yep, running these tests with all vendors now. Let's see if any fail and go from there now. Thanks @Quuxplusone.

[jloser]

The string_view test passes on all vendors. CI is green. Everyone still OK if I land this as-is then?

[jloser]

ping @Quuxplusone @Mordante

libcxx/docs/Status/Cxx2bPapers.csv

 "`P0288 <https://wg21.link/P0288>`__","LWG","``any_invocable``","October 2021","",""
 "`P0798 <https://wg21.link/P0798>`__","LWG","Monadic operations for ``std::optional``","October 2021","",""
 "`P0849 <https://wg21.link/P0849>`__","LWG","``auto(x)``: ``DECAY_COPY`` in the language","October 2021","",""
 "`P1072 <https://wg21.link/P1072>`__","LWG","``basic_string::resize_and_overwrite``","October 2021","",""
 "`P1147 <https://wg21.link/P1147>`__","LWG","Printing ``volatile`` Pointers","October 2021","",""
 "`P1272 <https://wg21.link/P1272>`__","LWG","Byteswapping for fun&&nuf","October 2021","",""
 "`P1675 <https://wg21.link/P1675>`__","LWG","``rethrow_exception`` must be allowed to copy","October 2021","",""
 "`P2077 <https://wg21.link/P2077>`__","LWG","Heterogeneous erasure overloads for associative containers","October 2021","",""
-"`P2251 <https://wg21.link/P2251>`__","LWG","Require ``span`` & ``basic_string_view`` to be Trivially Copyable","October 2021","",""
+"`P2251 <https://wg21.link/P2251>`__","LWG","Require ``span`` & ``basic_string_view`` to be Trivially Copyable","October 2021","|Complete|","14.0"
 "`P2301 <https://wg21.link/P2301>`__","LWG","Add a ``pmr`` alias for ``std::stacktrace``","October 2021","",""
 "`P2321 <https://wg21.link/P2321>`__","LWG","``zip``","October 2021","",""
 "`P2340 <https://wg21.link/P2340>`__","LWG","Clarifying the status of the 'C headers'","October 2021","",""
 "`P2393 <https://wg21.link/P2393>`__","LWG","Cleaning up ``integer``-class types","October 2021","",""
 "`P2401 <https://wg21.link/P2401>`__","LWG","Add a conditional ``noexcept`` specification to ``std::exchange``","October 2021","",""
 "","","","","",""

libcxx/test/std/containers/views/trivially_copyable.compile.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
+
+// P2251 was voted into C++23, but is supported even in C++20 mode by all vendors.
+
+// <span>
+
+#include <span>
+#include <type_traits>
+
+static_assert(std::is_trivially_copyable_v<std::span<int>>);
+static_assert(std::is_trivially_copyable_v<std::span<int, 3>>);

libcxx/test/std/strings/string.view/trivially_copyable.compile.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
+//
+//===----------------------------------------------------------------------===//
+
+// P2251 was voted into C++23, but is supported even in C++17 mode by all vendors.
+
+// <string_view>
+
+#include <string_view>
+#include <type_traits>
+
+static_assert(std::is_trivially_copyable<std::basic_string_view<char> >::value, "");
+static_assert(std::is_trivially_copyable<std::basic_string_view<wchar_t> >::value, "");
+#ifndef _LIBCPP_HAS_NO_CHAR8_T
+static_assert(std::is_trivially_copyable<std::basic_string_view<char8_t> >::value, "");
+#endif
+static_assert(std::is_trivially_copyable<std::basic_string_view<char16_t> >::value, "");
+static_assert(std::is_trivially_copyable<std::basic_string_view<char32_t> >::value, "");