[libcxx-commits] [PATCH] D146398: [libcxx] Fix using std::wcout/wcin on Windows with streams configured in wide mode
Martin Storsjö via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Sat Apr 8 09:13:50 PDT 2023
mstorsjo added a comment.
In D146398#4253025 <https://reviews.llvm.org/D146398#4253025>, @Mordante wrote:
> Is it possible to add tests for these changes?
I’d love to, but it’s not very straightforward.. We already lack coverage of the conversion aspects of `std::cout`/`std::wcout` - the most complex thing we test is to print `1234` to the output streams and check that it ends up printing the right thing with a bash script. To test the relevant aspects here, we’d need to print unicode charts, and I’m not sure how well that integrates with bash here (or how well llvm-lit manages to pass the unicode string to validate to the script).
================
Comment at: libcxx/include/__std_stream:87
+#if defined(_LIBCPP_WIN32API)
+ if constexpr (sizeof(char_type) == sizeof(wchar_t)) {
+ __encoding_ = 1;
----------------
Mordante wrote:
> `if constexpr` fails before C++17, AFAIK this code should be working in all language modi.
Yeah, we can’t use it in public headers, but as far as I could see, this header isn’t ever accessed by any public header - it should only be included by the libcxx sources.
I think it might be fine to leave out the constexpr if you want it out of principle though, I don’t think there was a functional need for it.
================
Comment at: libcxx/include/__std_stream:124
}
+#if defined(_LIBCPP_WIN32API)
+ char_type __extbuf[__limit];
----------------
Mordante wrote:
> Is this `ifdef` really needed? Will it fail on Linux when using `char_type`?
Yes; the type of `__extbuf` needs to be the right one for the conversion from the external charset (utf8 in a char array on other systems, but read directly as wchars on windows) into `char_type` - see the `__cv_->in` call below on line 138/164.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146398/new/
https://reviews.llvm.org/D146398
More information about the libcxx-commits
mailing list