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

[Mark de Wever via Phabricator via libcxx-commits]

Mordante marked 2 inline comments as done.
Mordante added a comment.

Thanks for the review!



================
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*>;
+                        };
----------------
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.
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120921