[libcxx-commits] [PATCH] D120921: [libc++][format] Adds a formattable concept.

[Victor Zverovich via Phabricator via libcxx-commits]

vitaut added inline comments.


================
Comment at: libcxx/include/__format/concepts.h:38
+                          { __f.parse(__pc) } -> same_as<typename basic_format_parse_context<_CharT>::iterator>;
+                          { __cf.format(__t, __fc) } -> same_as<_CharT*>;
+                        };
----------------
Mordante wrote:
> vitaut wrote:
> > I think you should use `format_context::iterator`, not `_CharT*`.
> That doesn't works since that only works for the `char` context. I'll change to code to use `fmt-iter-for<charT>` as used in the paper. I prefer to use the `CharT*` since it's the most simple output iterator. This is a valid choice according to the paper.
> 
> ```
> We don’t specify what the iterator type is of format_context or wformat_context, the expectation is that formatters accept any iterator. As such, it is unspecified in the concept which iterator will be checked - simply that it is some output_iterator<charT const&>. Implementations could use format_context::iterator and wformat_context::iterator, or they could have a bespoke minimal iterator dedicated for concept checking.
> ```
Makes sense but maybe clarify it in a comment?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120921