[libcxx-commits] [PATCH] D120135: [libc++] Overhaul std::quoted; fix its relationship to character traits.

[Louis Dionne via Phabricator via libcxx-commits]

ldionne requested changes to this revision.
ldionne added a comment.
This revision now requires changes to proceed.
Herald added a project: All.

Is there any way you can land the style changes (whitespace and other consistency stuff) as a separate NFC commit, and re-upload this one so we can take a look?



================
Comment at: libcxx/include/iomanip:596-597
 
+    _LIBCPP_HIDE_FROM_ABI
+    explicit __quoted_proxy(basic_string<_CharT, _Traits, _Allocator>& __s, _CharT __d, _CharT __e)
+        : __string_(__s), __delim_(__d), __escape_(__e) {}
----------------
Quuxplusone wrote:
> I've added `_LIBCPP_HIDE_FROM_ABI` to this ctor. That's correct, right? @ldionne
Yes, that looks correct to me.


================
Comment at: libcxx/test/std/input.output/iostream.format/quoted.manip/quoted_traits.compile.pass.cpp:9
+
+// UNSUPPORTED: c++03, c++11
+
----------------
We already have `libcxx/test/std/input.output/iostream.format/quoted.manip/quoted.pass.cpp`. Can you please add a comment explaining what this is actually testing, since it's obviously different from `libcxx/test/std/input.output/iostream.format/quoted.manip/quoted.pass.cpp`.


================
Comment at: libcxx/test/std/input.output/iostream.format/quoted.manip/quoted_traits.compile.pass.cpp:145
+
+#ifndef TEST_HAS_NO_WCHAR_T
+  std::wstring_view ws = L"hello";
----------------
I think that's what you mean -- same above.


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

https://reviews.llvm.org/D120135