[libcxx-commits] [PATCH] D129195: [libc++] Implement P1423R3 (char8_t backward compatibility remediation)

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added inline comments.


================
Comment at: libcxx/include/ostream:1103
+
+#ifndef _LIBCPP_HAS_NO_WIDE_CHARACTERS
+template <class _Traits>
----------------
Mordante wrote:
> Next time for reviewing it would be easier to use the same order as the paper.
I first put it in the same order as in the paper, but that would have meant 3x the amount preprocessor directives, which would be a lot less readable in the long term IMO.


================
Comment at: libcxx/test/std/input.output/iostream.format/output.streams/ostream/deleted_output_functions.verify.cpp:21-22
+#ifndef TEST_HAS_NO_WIDE_CHARACTERS
+  s << wchar_t();                      // expected-error {{overload resolution selected deleted operator '<<'}}
+  s << std::declval<const wchar_t*>(); // expected-error {{overload resolution selected deleted operator '<<'}}
+#endif
----------------
Mordante wrote:
> For clarity I would prefer and similar changes to other tests.
I think explicitly naming the types makes it a lot clearer. I have to think a lot harder to know what `u8`, `u`, `U` and `L` mean (I actually had to look these up).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129195