[libcxx-commits] [PATCH] D110499: [libc++][format][5/6] Improve format_to_n.

Louis Dionne via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Thu Apr 7 12:56:45 PDT 2022

ldionne added inline comments.

Comment at: libcxx/include/__format/buffer.h:290
+template <class _OutIt, __formatter::__char_type _CharT, bool>
+requires(output_iterator<_OutIt, const _CharT&>) struct _LIBCPP_TEMPLATE_VIS
+    __format_to_n_buffer_base {
This formatting strikes me as really strange. I'd expect something like:

template <class _OutIt, __formatter::__char_type _CharT, bool>
  requires output_iterator<_OutIt, const _CharT&>
struct _LIBCPP_TEMPLATE_VIS __format_to_n_buffer_base {

The current formatting for this declaration is somewhat difficult to read IMO.

Comment at: libcxx/include/format:372
+                                _VSTD::__format_context_create(__buffer.make_output_iterator(), __args));
+  return _VSTD::move(__buffer).result();
You probably want to use `std::` instead of `_VSTD::`

Comment at: libcxx/include/format:519
 template <output_iterator<const wchar_t&> _OutIt, class... _Args>
-format_to_n(_OutIt __out_it, iter_difference_t<_OutIt> __n, locale __loc,
-            wstring_view __fmt, const _Args&... __args) {
-  // TODO FMT Improve PoC: using std::string is inefficient.
-  wstring __str = _VSTD::vformat(_VSTD::move(__loc), __fmt,
-                                 _VSTD::make_wformat_args(__args...));
-  iter_difference_t<_OutIt> __s = __str.size();
-  iter_difference_t<_OutIt> __m =
-      _VSTD::clamp(__n, iter_difference_t<_OutIt>(0), __s);
-  __out_it = _VSTD::copy_n(__str.begin(), __m, _VSTD::move(__out_it));
-  return {_VSTD::move(__out_it), __s};
+format_to_n(_OutIt __out_it, iter_difference_t<_OutIt> __n, locale __loc, wstring_view __fmt, const _Args&... __args) {
I would rather not use `_LIBCPP_ALWAYS_INLINE` I would let the compiler make the right call here -- `_LIBCPP_ALWAYS_INLINE` has a negative impact on various optimizations and on the debugging experience, so we should steer clear of it unless we have a specific reason to. A function being a one-liner isn't a sufficient reason -- the optimizer should be able to figure out whether it is worth actually generating a function call (most likely not).

  rG LLVM Github Monorepo



More information about the libcxx-commits mailing list