[libcxx-commits] [PATCH] D112361: [libc++][format] Buffer changes proof-of-concept

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added inline comments.


================
Comment at: libcxx/include/__format/buffer.h:70
+
+  _LIBCPP_HIDE_FROM_ABI void push_back(_CharT __c) {
+    __ptr_[__size_++] = __c;
----------------
vitaut wrote:
> You might also want to provide an API for appending multiple chars at a time (e.g. a `string_view`) which can be implemented more efficiently than appending individual characters.
I already considered that, but I prefer to do that in a separate commit.
I'll add a TODO FMT in the source to make that intention clear.


================
Comment at: libcxx/include/__format/buffer.h:80-81
+  _LIBCPP_HIDE_FROM_ABI void flush() {
+    __flush_(__ptr_, __size_, __obj_);
+    __size_ = 0;
   }
----------------
vitaut wrote:
> Wouldn't this overwrite the data for direct storage?
No the flush of the direct storage only adjusts the output iterator. The output iterator needs to be updated when the the direct storage uses a `__wrap_iter<_CharT*>` instead of a `_CharT*`.


================
Comment at: libcxx/include/__format/buffer.h:97
+template <__formatter::__char_type _CharT>
+class _LIBCPP_TEMPLATE_VIS __interal_storage {
+public:
----------------
vitaut wrote:
> Did you mean `internal_storage`? (missing "n")
Thanks for catching this typo.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112361