[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 added a comment.

Thank you for the detailed response, @mstorsjo. I understand the motivation for the changes much better now.

> Actually, as far as I know, there's no way to inspect the current mode set on a file descriptor - so even if we wanted to, we can't. (And other C++ libraries such as MS STL don't inspect the mode either.)

There is a way to do so, but requires use of core CRT internal headers (`corecrt_internal_lowio.h`) and interfaces (`_textmode`) that don't have stability guarantees. So, I agree, we can't (or at least shouldn't) inspect the file mode.

> Secondly, as a detail, that spec draft seems to expose the deepest native handle which seems to be meant to be a HANDLE on Windows

Ah, yes, that is correct.

Microsoft's documentation for _setmode() <https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/setmode?view=msvc-170> states:

> Unicode mode is for wide print functions (for example, `wprintf`) and is not supported for narrow print functions. Use of a narrow print function on a Unicode mode stream triggers an assert.

That makes it clear that the standard library must restrict itself to use of the wide print functions with streams with those file modes. But since we can't check the file mode, that means programmers that (incorrectly) use `std::cout` following a call to `_setmode()` to put the stream in Unicode mode will experience an error that appears to put the blame on the standard library. Perhaps it is worth documenting these interactions.

Perhaps we should be using the wide printing functions for `wchar_t` streams on all platforms. `wchar_t` is seldom used on platforms other than Windows; do `std::wcin`, `std::wcout`, and `std::wcerr` actually work in a reasonable way on non-Windows platforms today?



================
Comment at: libcxx/src/std_stream.h:128-149
+#if defined(_LIBCPP_WIN32API)
+    char_type __extbuf[__limit];
+#else
     char __extbuf[__limit];
+#endif
     int __nread = _VSTD::max(1, __encoding_);
     for (int __i = 0; __i < __nread; ++__i)
----------------
I think it may be possible to do this in a cleaner way by factoring out the calls to `getc()` and `getwc()` doing something like this:
Add some helper methods:
  static bool __do_getc(FILE* __fp, char* __pbuf) {
      int __c = getc(__fp);
      if (__c == EOF)
          return false
       *__pbuf = static_cast<char>(__c);
      return true;
  }
  #ifndef _LIBCPP_HAS_NO_WIDE_CHARACTERS
  static bool __do_getc(FILE* __fp, wchar_t *pbuf) {
      wint_t __c = getwc(__file_);
      if (__c == WEOF)
          return false;
      *__pbuf = static_cast<wchar_t>(__c);
      return true;
  }
  #endif

Change the above code to:

  char_type __extbuf[__limit];
  int __nread = _VSTD::max(1, __encoding_);
  for (int __i = 0; __i < __nread; ++__i)
  {
      if (!do_getc(__file_, &extbuf[__i]))
          return traits_type::eof();
  }

Do similarly for `ungetc`/`ungetwc` and `fwrite`/`fputwc` further below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146398