[libcxx-commits] [PATCH] D146398: [libcxx] Fix using std::wcout/wcin on Windows with streams configured in wide mode

[Tom Honermann via Phabricator via libcxx-commits]

tahonermann requested changes to this revision.
tahonermann added a comment.
This revision now requires changes to proceed.

This doesn't seem like the right approach to me.

First, the code changes aren't actually sensitive to the mode of the file handle so the calls to `_setmode()` in the tests presumably have no behavioral impact (other than they may change the behavior of functions called by the changed code).

Second, but related to the first point above, the code changes are isolated to the `__stdinbuf` and `__stdoutbuf` classes which are only used to wrap `stdin`, `stdout`, and `stderr`; thus the changes won't impact other file handles for which `_setmode()` has been called. I don't think libc++ currently exposes the `FILE*` pointers held in `basic_filebuf` instances, but future changes may require doing so. See https://wg21.link/p1759 for example.

It probably is necessary to `#ifdef` the code to enable Windows-specific behavior. However, I don't think the `sizeof()` checks are what should be used for conditional behavior. Rather, I think the file handle mode should determine whether, e.g., `getwc()` or `getc()` is called. If it really is necessary to conditionalize some of the code for `char_type`, then I think that should be done with `is_same_v<char_type, wchar_t>` rather than using `sizeof` as a proxy for the type.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146398