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

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added inline comments.


================
Comment at: libcxx/include/__format/buffer.h:216-217
+template <class _OutIt, __formatter::__char_type _CharT>
+requires(output_iterator<_OutIt, const _CharT&>) class _LIBCPP_TEMPLATE_VIS
+    __format_to_n_buffer {
+  using _Buffer = typename __buffer_selector<_OutIt, _CharT>::type;
----------------



================
Comment at: libcxx/include/__format/buffer.h:219
+  using _Buffer = typename __buffer_selector<_OutIt, _CharT>::type;
+  using _Size = iter_difference_t<_OutIt>;
+
----------------
Given that you continue incrementing `__size_` even after hitting the end of `__buffer_`, I think it's a bit dangerous to use any type here except `size_t`. I recommend `s/_Size/size_t/g`.
I don't see any particular usefulness in continuing to store `__n_` as `_Size`, either. `formatted_size()` is specced to return `size_t`, not `iter_difference_t<anything>`, so I think `size_t` throughout would make plenty of sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110499