[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.

In D120921#3387499 <https://reviews.llvm.org/D120921#3387499>, @ldionne wrote:

> Please ping me on Discord if I should be reviewing this -- my understanding is that it's for a not-yet-approved paper? (if so, you can mark this as "changes planned" until you'd like a review)

Correct the paper isn't accepted yet and the paper targets C++23. However as mentioned in the description I need a formattable concept in C++20. I use the concept in D121514 <https://reviews.llvm.org/D121514>. So I either:

- Write my own concept, which would look slightly different.
- Use P2286 <https://reviews.llvm.org/P2286>'s proposed concept.

Using an existing proposal allows me to reuse this "private" concept in C++23 avoiding code duplication.
So even when P2286 <https://reviews.llvm.org/P2286> will never make it in the Standard this concept is still useful.
(I hope P2286 <https://reviews.llvm.org/P2286> will make it in C++23 since I really like to proposal.)



================
Comment at: libcxx/include/__format/concepts.h:38-44
+template <class _Tp, class _CharT>
+concept __formattable = semiregular<formatter<remove_cvref_t<_Tp>, _CharT>> &&
+    requires(formatter<remove_cvref_t<_Tp>, _CharT> __f, formatter<remove_cvref_t<_Tp>, _CharT> __cf, _Tp __t,
+             basic_format_context<_CharT*, _CharT> __fc, basic_format_parse_context<_CharT> __pc) {
+  { __f.parse(__pc) } -> same_as<typename basic_format_parse_context<_CharT>::iterator>;
+  { __cf.format(__t, __fc) } -> same_as<_CharT*>;
+};
----------------
Quuxplusone wrote:
> Let's add some linebreaks for readability after each requires-parameter:
> ```
> template <class _Tp, class _CharT>
> concept __formattable =
>   semiregular<formatter<remove_cvref_t<_Tp>, _CharT>> &&
>   requires(formatter<remove_cvref_t<_Tp>, _CharT> __f,
>            formatter<remove_cvref_t<_Tp>, _CharT> __cf, // TODO: const
>            _Tp __t,
>            basic_format_context<_CharT*, _CharT> __fc,
>            basic_format_parse_context<_CharT> __pc) {
>     { __f.parse(__pc) } -> same_as<typename basic_format_parse_context<_CharT>::iterator>;
>     { __cf.format(__t, __fc) } -> same_as<_CharT*>;
>   };
> ```
I did part of the formatting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120921