[libcxx-commits] [PATCH] D103425: [libc++][format] Adds string formatter.

[Victor Zverovich via Phabricator via libcxx-commits]

vitaut accepted this revision.
vitaut added a comment.

Some nits inline, otherwise LGTM.



================
Comment at: libcxx/include/__format/formatter.h:124-126
+ * @note The type of the input @a __first, @a __last can differ from the type
+ * of @a __fill. Integer output uses @c std::to_chars for its conversion. This
+ * function returns a buffer with @c char elements.
----------------
Did you mean the type of values in [`__first`, `__last`) being different from the type of `__fill`? `__first` and `__last` themselves are pointers, not code units.

Also the comment says "This function returns a buffer with @c char elements." while in fact it returns an output iterator.


================
Comment at: libcxx/include/__format/formatter_string.h:101
+    formatter<const _CharT*, _CharT>
+    : public __format_spec::__formatter_string<_CharT> {
+  using _Base = __format_spec::__formatter_string<_CharT>;
----------------
nit: public is redundant here and elsewhere since this is a struct.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103425