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

[Mark de Wever via Phabricator via libcxx-commits]

Mordante marked 2 inline comments as done.
Mordante 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;
----------------
Quuxplusone wrote:
> 
Again what clang-format makes of the code.


================
Comment at: libcxx/include/__format/buffer.h:219
+  using _Buffer = typename __buffer_selector<_OutIt, _CharT>::type;
+  using _Size = iter_difference_t<_OutIt>;
+
----------------
Quuxplusone wrote:
> 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.
The normal `formatted_size` indeed returns a `size_t`, but `format_to_n` returns
```
  template<class Out> struct format_to_n_result {
    Out out;
    iter_difference_t<Out> size; // This is the formatted_size.
  };
```
So I prefer to keep this the same type as the Standard specified.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110499