[libcxx-commits] [PATCH] D152092: [libc++][format] Adds formattable-with concept.

Tue Jun 20 08:55:12 PDT 2023

ldionne accepted this revision.
ldionne added a comment.
This revision is now accepted and ready to land.

LGTM w/ nitpicks.

================
Comment at: libcxx/docs/Status/Cxx2cIssues.csv:14
 "`3915 <https://wg21.link/LWG3915>`__","Redundant paragraph about expression variations","Varna June 2023","","","|ranges|"
-"`3925 <https://wg21.link/LWG3925>`__","Concept ``formattable``'s definition is incorrect","Varna June 2023","|In Progress|","","|format|"
+"`3925 <https://wg21.link/LWG3925>`__","Concept ``formattable``'s definition is incorrect","Varna June 2023","|Complete|","Clang 17.0","|format|"
 "`3927 <https://wg21.link/LWG3927>`__","Unclear preconditions for ``operator[]`` for sequence containers","Varna June 2023","","",""
----------------

================
Comment at: libcxx/test/std/utilities/format/format.formattable/concept.formattable.compile.pass.cpp:55

 template <class T, class CharT>
 void assert_is_not_formattable() {
----------------
Would it make sense to add a test for `!std::formattable<volatile T&, CharT>`. I guess you could do `assert_is_no_formattable<volatile int, CharT>()` -- I think something like that would do the trick. And in the place where you make this call you can say `// https://wg21.link/LWGXXXXXX` to justify it.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152092