[libcxx-commits] [PATCH] D96385: [libc++] [P1032] Finish constexpr-ifying <iterator>, <string_view>, <tuple>, <utility>.

[Zoe Carver via Phabricator via libcxx-commits]

zoecarver added a comment.

A few small comments now. More later (probably).



================
Comment at: libcxx/test/std/strings/string.view/string.view.ops/copy.pass.cpp:59
 template<typename CharT>
 void test ( const CharT *s ) {
     typedef std::basic_string_view<CharT> string_view_t;
----------------
Too bad we can't test this in a constexpr context 😔


================
Comment at: libcxx/test/std/strings/string.view/string.view.ops/copy.pass.cpp:83
+template<typename CharT>
+TEST_CONSTEXPR_CXX20 bool test_constexpr_copy(const CharT *abcde, const CharT *g, const CharT *bcdjk)
+{
----------------
Nit: might make sense to either name these `a`, `g`, and `b` or all `abcde`, `ghijk`, `bcdjk`.


================
Comment at: libcxx/test/std/utilities/tuple/tuple.tuple/tuple.assign/copy.pass.cpp:99
+    {
+        typedef std::tuple<int, char, std::string> T;
+        const T t0(2, 'a', "some text");
----------------
Mind adding a comment "Cannot be constexpr because of std::string" or something like that?


================
Comment at: libcxx/test/std/utilities/tuple/tuple.tuple/tuple.assign/move.pass.cpp:129
     }
     {
         // The move should decay to a copy.
----------------
Why not put this all in `test`?


================
Comment at: libcxx/test/std/utilities/tuple/tuple.tuple/tuple.creation/make_tuple.pass.cpp:44
-    }
-#if TEST_STD_VER > 11
-    {
----------------
We're losing test coverage of constexpr between 11 and 20. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96385/new/

https://reviews.llvm.org/D96385