[libcxx-commits] [PATCH] D118717: [libc++][format[[nfc] Use string_view in tests.

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone accepted this revision as: Quuxplusone.
Quuxplusone added inline comments.


================
Comment at: libcxx/test/std/utilities/format/format.functions/format.locale.pass.cpp:40-41
 
-auto test_exception =
-    []<class CharT, class... Args>(std::string_view what, std::basic_string<CharT> fmt, const Args&... args) {
+auto test_exception = []<class CharT, class... Args>(std::string_view what, std::basic_string_view<CharT> fmt,
+                                                     const Args&... args) {
 #ifndef TEST_HAS_NO_EXCEPTIONS
----------------
FWIW, you could use `const auto&... args` to avoid such a long line (and to remove one C++20ism, although obviously removing the //other// template parameter would require more surgery).


================
Comment at: libcxx/test/std/utilities/format/format.functions/format_to.pass.cpp:69
     assert(false);
   } catch (std::format_error& e) {
     LIBCPP_ASSERT(e.what() == what);
----------------
Pre-existing nit presumably throughout: "Catch by const reference" is a good habit.
https://quuxplusone.github.io/blog/2018/09/16/data-race-when-catch-by-nonconst-reference/


================
Comment at: libcxx/test/std/utilities/format/format.functions/vformat_to.locale.pass.cpp:33
 
-auto test = []<class CharT, class... Args>(std::basic_string<CharT> expected, std::basic_string<CharT> fmt,
+auto test = []<class CharT, class... Args>(std::basic_string<CharT> expected, std::basic_string_view<CharT> fmt,
                                            const Args&... args) {
----------------
Why not `std::basic_string_view<CharT> expected` as well? (Ditto throughout.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118717