[libcxx-commits] [PATCH] D130643: [libc++][format] Exposes basic-format-string

[Louis Dionne via Phabricator via libcxx-commits]

ldionne accepted this revision.
ldionne added inline comments.
This revision is now accepted and ready to land.


================
Comment at: libcxx/docs/UsingLibcxx.rst:448
+
+Extensions ``<format>``
+~~~~~~~~~~~~~~~~~~~~~~~
----------------



================
Comment at: libcxx/docs/UsingLibcxx.rst:451-455
+The exposition only type ``basic-format-string`` and its typedefs
+``format-string`` and ``wformat-string`` became ``basic_format_string``,
+``format_string``, and ``wformat_string`` in C++23. As an extension libc++
+makes these types available in C++20. (The exposition only types were never
+shipped.)
----------------
IMO it's not necessary to mention that we never shipped the previous names, since they were exposition-only. Also, it's mostly relevant for us libc++ developers to understand this change, but not for users who will consume this documentation.


================
Comment at: libcxx/include/format:28
   template<class charT, class... Args>
-    struct basic-format-string;                       // exposition only
+    struct basic_format_string {                                // since C++23, before an exposition only type
+    private:
----------------
Here and elsewhere


================
Comment at: libcxx/test/std/utilities/format/format.fmt.string/ctor.verify.cpp:9-11
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+// UNSUPPORTED: !stdlib=libc++ && c++20
+// UNSUPPORTED: libcpp-has-no-incomplete-format
----------------
Can you please set this line apart and add a comment like this? Applies elsewhere too.


================
Comment at: libcxx/utils/generate_feature_test_macro_components.py:313-314
+    # P2508, P2286, and P2419 were accepted in the same plenary and modify this
+    # feature-test macro. I expect to see an LWG issue soon. For now keep the
+    # value as is.
+    # TODO FMT Set P2508's feature-test macro.
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130643