[libcxx-commits] [PATCH] D122534: [NFC][libc++][format] Prepare unit tests.

[Mark de Wever via Phabricator via libcxx-commits]

Mordante marked an inline comment as done.
Mordante added a comment.

Thanks for the review!



================
Comment at: libcxx/test/std/utilities/format/format.functions/formatted_size.pass.cpp:27
 #include "test_macros.h"
 #include "format_tests.h"
 
----------------
ldionne wrote:
> I would prefer if the tests could include what they use -- here we're using `string_literal` but we haven't included `string_literal.h`. I think we should not rely on the transitive include via `format_tests.h`.
In this case the test really requires `format_tests.h` to be included. But I agree it's good practice not to depend on transitive includes, so I'll fix it.


================
Comment at: libcxx/test/support/string_literal.h:47-50
+  char data_[N + 1];
+#  ifndef TEST_HAS_NO_WIDE_CHARACTERS
+  wchar_t wdata_[N + 1];
+#  endif
----------------
ldionne wrote:
> Should those be private?
No that doesn't compile. I tried that, hence the comment at comment on line 27. I haven't looked at the exact rules of `consteval`, but I have a strong suspicion the class needs to be an aggregate.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122534