[libcxx-commits] [PATCH] D90222: [1/N] [libcxx] Implement c++2a char8_t input/output of std::filesystem::path

[Marek Kurdej via Phabricator via libcxx-commits]

curdeius accepted this revision.
curdeius added a comment.

LGTM. But wait for an approval from a higher instance :).



================
Comment at: libcxx/test/support/filesystem_test_helper.h:433
 
-#define MKSTR(Str) {Str, TEST_CONCAT(L, Str), TEST_CONCAT(u, Str), TEST_CONCAT(U, Str)}
+#define MKSTR(Str) {Str, TEST_CONCAT(L, Str), TEST_CONCAT(u8, Str), TEST_CONCAT(u, Str), TEST_CONCAT(U, Str)}
 
----------------
mstorsjo wrote:
> curdeius wrote:
> > Shouldn't you guard it on `__cpp_char8_t` instead of using `dummy` below?
> Yeah, it's possible, with something like this:
> ```
> #if TEST_STD_VER > 17 && defined(__cpp_char8_t)
> #define CHAR8_ONLY(x) x,
> #else
> #define CHAR8_ONLY
> #endif
> #define MKSTR(Str) {... CHAR8_ONLY(TEST_CONCAT(u8, Str)) TEST_CONCAT(u, Str), ...
> ```
> The trailing comma is a bit tricky; it has to be included as part of the expansion of `CHAR8_ONLY()`, but this way it seems like it would work.
> 
> If you prefer that form, I can change it that way.
I thought of just doing:
```
#if TEST_STD_VER... 
#define MKSTR(...u8...)
#else 
#define MKSTR(...)
#endif
```
But that's OK as it is for me. Macros are tricky (or evil).


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

https://reviews.llvm.org/D90222