[libcxx-commits] [PATCH] D142808: [libc++][test] Adds more generic test macros.

[Mark de Wever via Phabricator via libcxx-commits]

Mordante marked 5 inline comments as done.
Mordante added inline comments.


================
Comment at: libcxx/test/support/assert_macros.h:38
 
-#if TEST_STD_VER > 17
-
-#  ifndef TEST_HAS_NO_LOCALIZATION
-template <class T>
-concept test_char_streamable = requires(T&& value) { std::stringstream{} << std::forward<T>(value); };
-#  endif
-
-// If possible concatenates message for the assertion function, else returns a
-// default message. Not being able to stream is not considered and error. For
-// example, streaming to std::wcerr doesn't work properly in the CI. Therefore
-// the formatting tests should only stream to std::string_string.
-template <class... Args>
-std::string test_concat_message([[maybe_unused]] Args&&... args) {
-#  ifndef TEST_HAS_NO_LOCALIZATION
-  if constexpr ((test_char_streamable<Args> && ...)) {
-    std::stringstream sstr;
-    ((sstr << std::forward<Args>(args)), ...);
-    return sstr.str();
-  } else
-#  endif
-    return "Message discarded since it can't be streamed to std::cerr.\n";
+void test_log(const char* condition, const char* file, int line, std::string&& message) {
+  test_log(condition, file, line, message.c_str());
----------------
ldionne wrote:
> I don't think you actually need the `string` overload. That would allow you to drop the header dependency.
Good catch, you're correct. I also tested the follow-up patches and they keep working when removing the `std::string` overloads.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142808