[libcxx-commits] [PATCH] D119329: [libc++] Prepare string.modifiers tests for constexpr

[Louis Dionne via Phabricator via libcxx-commits]

ldionne requested changes to this revision.
ldionne added a comment.
This revision now requires changes to proceed.

Looks pretty good, with some comments.



================
Comment at: libcxx/test/std/strings/basic.string/string.modifiers/string_insert/size_T_size_size.pass.cpp:1833
+template <class S, class SV>
+void test() {
+  test0<S, SV>();
----------------
Instead, can you do

```
template <class S, class V>
_LIBCPP_CONSTEXPR_AFTER_CXX17 void tests() {
  test0<S, SV>();
  // ...
  test31<S, SV>();
}

bool check_all() {
  tests<std::string, std::string_view>();
#if TEST_STD_VER >= 11
  tests<std::basic_string<char, std::char_traits<char>, min_allocator<char>>,
        std::basic_string_view<char, std::char_traits<char>>>();
#endif
  return true;
}

int main() {
  check_all();
#if TEST_STD_VER > 17
  static_assert(check_all());
#endif
}
```

You can also tweak `test31` to be templated like the other ones, I think you only need to use the `SV()` macro and friends in it.


================
Comment at: libcxx/test/std/strings/basic.string/string.modifiers/string_insert/size_string_size_size.pass.cpp:1861
 
 int main(int, char**)
 {
----------------
Same comment as above.


================
Comment at: libcxx/test/std/strings/basic.string/string.modifiers/string_replace/iter_iter_iter_iter.pass.cpp:1049
+template <class S>
+void test() {
+  test0<S>();
----------------
Same comment as above.


================
Comment at: libcxx/test/std/strings/basic.string/string.modifiers/string_replace/iter_iter_pointer_size.pass.cpp:983
+template <class S>
+void test() {
+  test0<S>();
----------------
Same comment as above.


================
Comment at: libcxx/test/std/strings/basic.string/string.modifiers/string_replace/size_size_T_size_size.pass.cpp:6020
+template <class S, class SV>
+bool test() {
+  test0<S, SV>();
----------------
Same comment as above.


================
Comment at: libcxx/test/std/strings/basic.string/string.modifiers/string_replace/size_size_pointer_size.pass.cpp:1324
+
+bool test() {
+  test0<S>();
----------------
Here too.


================
Comment at: libcxx/test/std/strings/basic.string/string.modifiers/string_replace/size_size_string_size_size.pass.cpp:5863
+template <class S>
+bool test() {
+  test0<S>();
----------------
Here too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119329