[libcxx-commits] [PATCH] D110498: [libc++][format][4/6] Improve formatted_size.

[Victor Zverovich via Phabricator via libcxx-commits]

vitaut added a comment.

In general looks good, just a few minor comments inline. However, the discussion in earlier diffs may affect `__formatted_size_buffer` API.



================
Comment at: libcxx/include/__format/buffer.h:272
+
+  _LIBCPP_HIDE_FROM_ABI void flush(_CharT*, size_t __size) {
+    __size_ += __size;
----------------
Why not `const _CharT*`?


================
Comment at: libcxx/include/__format/buffer.h:276
+
+  _LIBCPP_HIDE_FROM_ABI size_t result() && {
+    __output_.flush();
----------------
Why `&&`-qualified?


================
Comment at: libcxx/include/format:579-599
+template <class... _Args>
+_LIBCPP_HIDE_FROM_ABI _LIBCPP_AVAILABILITY_FORMAT size_t
+formatted_size(string_view __fmt, const _Args&... __args) {
+  __format::__formatted_size_buffer<char> __buffer;
+  _VSTD::__vformat_to(__buffer.make_output_iterator(), __fmt,
+                      basic_format_args{_VSTD::make_format_args(__args...)});
+
----------------
I suggest adding `__vformatted_size` and force-inlining `formatted_size` to reduce template bloat.

Also why move this block? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110498