[libcxx-commits] [PATCH] D93593: [libc++][format] Add __format_arg_store.

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added a comment.

Thanks for the review.



================
Comment at: libcxx/include/format:290
+// concepts is implemented. To ease the transition to using the not yet
+// implemetned concepts some helper concepts are added.
+// TODO FMT Switch to the real concepts when they are available.
----------------
curdeius wrote:
> Typo: implemented.
Thanks will fix.


================
Comment at: libcxx/include/format:292
+// TODO FMT Switch to the real concepts when they are available.
+#if defined(__cpp_lib_concepts) && __cpp_lib_concepts >= 201806
+template <class _Tp>
----------------
curdeius wrote:
> We should make sure not to define __cpp_lib_concepts before having these concepts implemented then (or better, have all concepts implemented).
> Personally, I'd rather just use below defined concepts and get rid of this ifdef.
> We should make sure not to define __cpp_lib_concepts before having these concepts implemented then (or better, have all concepts implemented).

Agreed.

> Personally, I'd rather just use below defined concepts and get rid of this ifdef.

I've no objection. That also makes it possible to switch to specific concepts once they've landed. The (un)signed_integral is already in review.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93593