[libcxx-commits] [PATCH] D118191: [libcxx] [test] Fix testcases that fail on systems with 16 bit wchar_t

Martin Storsjö via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Tue Jan 25 14:31:16 PST 2022


mstorsjo added a comment.

In D118191#3270794 <https://reviews.llvm.org/D118191#3270794>, @Quuxplusone wrote:

> IMHO it might be better to //add// your new test, but keep the existing test under an `if (sizeof(wchar_t) >= 4)` or `if (WCHAR_MAX > 100003-or-whatever)`, something like that. It feels important to test that on platforms //with// 32-bit wchar_t, we can actually use all 32 bits and aren't silently truncating to 16 bits somewhere internally.
> (That said, what the heck, why is this test //discarding// the value of `.from_bytes` instead of asserting on it.)

I presume the main intent of this test is to test some other aspect (move constructor), not test the conversion itself, and therefore it doesn't really functionally test the conversion.

> It's possible both of my comments are sufficiently addressed by test coverage elsewhere in the test suite, in which case, looks great, ship it!

Not sure offhand, but this as this test only is for the move constructor, I would guess so.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118191



More information about the libcxx-commits mailing list