[libc++][format] Adds formattable-with concept.

This change has a few additional effects:
- Abstract classes are now formattable.
- Volatile objects are no longer formattable.

Implements
- LWG3631 basic_format_arg(T&&) should use remove_cvref_t<T> throughout
- LWG3925 Concept formattable's definition is incorrect

Reviewed By: #libc, ldionne

Differential Revision: https://reviews.llvm.org/D152092

libcxx/docs/ReleaseNotes.rst

@@ -111,6 +111,9 @@ Deprecations and Removals
 - The classes ``strstreambuf`` , ``istrstream``, ``ostrstream``, and ``strstream`` have been deprecated.
   They have been deprecated in the Standard since C++98, but were never marked as deprecated in libc++.
 
+- LWG3631 ``basic_format_arg(T&&) should use remove_cvref_t<T> throughout`` removed
+  support for ``volatile`` qualified formatters.
+
 Upcoming Deprecations and Removals
 ----------------------------------
 

libcxx/docs/Status/Cxx23Issues.csv

@@ -278,7 +278,7 @@
 "`3867 <https://wg21.link/LWG3867>`__","Should ``std::basic_osyncstream``'s move assignment operator be ``noexcept``?","February 2023","","",""
 "`3441 <https://wg21.link/LWG3441>`__","Misleading note about calls to customization points","February 2023","","",""
 "`3622 <https://wg21.link/LWG3622>`__","Misspecified transitivity of equivalence in §[unord.req.general]","February 2023","","",""
-"`3631 <https://wg21.link/LWG3631>`__","``basic_format_arg(T&&)`` should use ``remove_cvref_t<T>`` throughout","February 2023","|Complete|","15.0",""
+"`3631 <https://wg21.link/LWG3631>`__","``basic_format_arg(T&&)`` should use ``remove_cvref_t<T>`` throughout","February 2023","|Complete|","17.0",""
 "`3645 <https://wg21.link/LWG3645>`__","``resize_and_overwrite`` is overspecified to call its callback with lvalues","February 2023","|Complete|","14.0",""
 "`3655 <https://wg21.link/LWG3655>`__","The ``INVOKE`` operation and union types","February 2023","","",""
 "`3723 <https://wg21.link/LWG3723>`__","``priority_queue::push_range`` needs to ``append_range``","February 2023","","","|ranges|"

libcxx/docs/Status/Cxx2cIssues.csv

@@ -11,7 +11,7 @@
 "`3912 <https://wg21.link/LWG3912>`__","``enumerate_view::iterator::operator-`` should be ``noexcept``","Varna June 2023","","","|ranges|"
 "`3914 <https://wg21.link/LWG3914>`__","Inconsistent template-head of ``ranges::enumerate_view``","Varna June 2023","","","|ranges|"
 "`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|","17.0","|format|"
 "`3927 <https://wg21.link/LWG3927>`__","Unclear preconditions for ``operator[]`` for sequence containers","Varna June 2023","|Nothing To Do|","",""
 "`3935 <https://wg21.link/LWG3935>`__","``template<class X> constexpr complex& operator=(const complex<X>&)`` has no specification","Varna June 2023","|Complete|","Clang 3.4",""
 "`3938 <https://wg21.link/LWG3938>`__","Cannot use ``std::expected`` monadic ops with move-only ``error_type``","Varna June 2023","","",""

libcxx/include/__format/concepts.h

@@ -16,6 +16,7 @@
 #include <__format/format_fwd.h>
 #include <__format/format_parse_context.h>
 #include <__type_traits/is_specialization.h>
+#include <__type_traits/remove_const.h>
 #include <__utility/pair.h>
 #include <tuple>
 
@@ -43,17 +44,21 @@ concept __fmt_char_type =
 template <class _CharT>
 using __fmt_iter_for = _CharT*;
 
+template <class _Tp, class _Context, class _Formatter = typename _Context::template formatter_type<remove_const_t<_Tp>>>
+concept __formattable_with =
+    semiregular<_Formatter> &&
+    requires(_Formatter& __f,
+             const _Formatter& __cf,
+             _Tp&& __t,
+             _Context __fc,
+             basic_format_parse_context<typename _Context::char_type> __pc) {
+      { __f.parse(__pc) } -> same_as<typename decltype(__pc)::iterator>;
+      { __cf.format(__t, __fc) } -> same_as<typename _Context::iterator>;
+    };
+
 template <class _Tp, class _CharT>
 concept __formattable =
-    (semiregular<formatter<remove_cvref_t<_Tp>, _CharT>>) &&
-    requires(formatter<remove_cvref_t<_Tp>, _CharT> __f,
-             const formatter<remove_cvref_t<_Tp>, _CharT> __cf,
-             _Tp __t,
-             basic_format_context<__fmt_iter_for<_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<__fmt_iter_for<_CharT>>;
-    };
+    __formattable_with<remove_reference_t<_Tp>, basic_format_context<__fmt_iter_for<_CharT>, _CharT>>;
 
 #  if _LIBCPP_STD_VER >= 23
 template <class _Tp, class _CharT>

libcxx/include/__format/format_arg.h

@@ -13,6 +13,7 @@
 #include <__assert>
 #include <__concepts/arithmetic.h>
 #include <__config>
+#include <__format/concepts.h>
 #include <__format/format_error.h>
 #include <__format/format_fwd.h>
 #include <__format/format_parse_context.h>
@@ -158,18 +159,14 @@ public:
   /// Contains the implementation for basic_format_arg::handle.
   struct __handle {
     template <class _Tp>
-    _LIBCPP_HIDE_FROM_ABI explicit __handle(_Tp&& __v) noexcept
+    _LIBCPP_HIDE_FROM_ABI explicit __handle(_Tp& __v) noexcept
         : __ptr_(_VSTD::addressof(__v)),
           __format_([](basic_format_parse_context<_CharT>& __parse_ctx, _Context& __ctx, const void* __ptr) {
-            using _Dp = remove_cvref_t<_Tp>;
-            using _Formatter = typename _Context::template formatter_type<_Dp>;
-            constexpr bool __const_formattable =
-                requires { _Formatter().format(std::declval<const _Dp&>(), std::declval<_Context&>()); };
-            using _Qp = conditional_t<__const_formattable, const _Dp, _Dp>;
+            using _Dp = remove_const_t<_Tp>;
+            using _Qp = conditional_t<__formattable_with<const _Dp, _Context>, const _Dp, _Dp>;
+            static_assert(__formattable_with<_Qp, _Context>, "Mandated by [format.arg]/10");
 
-            static_assert(__const_formattable || !is_const_v<remove_reference_t<_Tp>>, "Mandated by [format.arg]/18");
-
-            _Formatter __f;
+            typename _Context::template formatter_type<_Dp> __f;
             __parse_ctx.advance_to(__f.parse(__parse_ctx));
             __ctx.advance_to(__f.format(*const_cast<_Qp*>(static_cast<const _Dp*>(__ptr)), __ctx));
           }) {}
@@ -221,9 +218,7 @@ public:
   _LIBCPP_HIDE_FROM_ABI __basic_format_arg_value(basic_string_view<_CharT> __value) noexcept
       : __string_view_(__value) {}
   _LIBCPP_HIDE_FROM_ABI __basic_format_arg_value(const void* __value) noexcept : __ptr_(__value) {}
-  _LIBCPP_HIDE_FROM_ABI __basic_format_arg_value(__handle __value) noexcept
-      // TODO FMT Investigate why it doesn't work without the forward.
-      : __handle_(std::forward<__handle>(__value)) {}
+  _LIBCPP_HIDE_FROM_ABI __basic_format_arg_value(__handle&& __value) noexcept : __handle_(std::move(__value)) {}
 };
 
 template <class _Context>

libcxx/include/__format/format_arg_store.h

@@ -22,6 +22,7 @@
 #include <__type_traits/conditional.h>
 #include <__type_traits/extent.h>
 #include <__type_traits/is_same.h>
+#include <__type_traits/remove_const.h>
 #include <__utility/forward.h>
 #include <string>
 #include <string_view>
@@ -157,11 +158,18 @@ consteval __arg_t __determine_arg_t() {
   return __arg_t::__none;
 }
 
+// Pseudo constuctor for basic_format_arg
+//
+// Modeled after template<class T> explicit basic_format_arg(T& v) noexcept;
+// [format.arg]/4-6
 template <class _Context, class _Tp>
-_LIBCPP_HIDE_FROM_ABI basic_format_arg<_Context> __create_format_arg(_Tp&& __value) noexcept {
-  constexpr __arg_t __arg = __determine_arg_t<_Context, remove_cvref_t<_Tp>>();
+_LIBCPP_HIDE_FROM_ABI basic_format_arg<_Context> __create_format_arg(_Tp& __value) noexcept {
+  using _Dp               = remove_const_t<_Tp>;
+  constexpr __arg_t __arg = __determine_arg_t<_Context, _Dp>();
   static_assert(__arg != __arg_t::__none, "the supplied type is not formattable");
 
+  static_assert(__formattable_with<_Tp, _Context>);
+
   // Not all types can be used to directly initialize the
   // __basic_format_arg_value.  First handle all types needing adjustment, the
   // final else requires no adjustment.
@@ -179,9 +187,9 @@ _LIBCPP_HIDE_FROM_ABI basic_format_arg<_Context> __create_format_arg(_Tp&& __val
     return basic_format_arg<_Context>{__arg, static_cast<unsigned long long>(__value)};
   else if constexpr (__arg == __arg_t::__string_view)
     // Using std::size on a character array will add the NUL-terminator to the size.
-    if constexpr (is_array_v<remove_cvref_t<_Tp>>)
+    if constexpr (is_array_v<_Dp>)
       return basic_format_arg<_Context>{
-          __arg, basic_string_view<typename _Context::char_type>{__value, extent_v<remove_cvref_t<_Tp>> - 1}};
+          __arg, basic_string_view<typename _Context::char_type>{__value, extent_v<_Dp> - 1}};
     else
       // When the _Traits or _Allocator are different an implicit conversion will
       // fail.
@@ -190,8 +198,7 @@ _LIBCPP_HIDE_FROM_ABI basic_format_arg<_Context> __create_format_arg(_Tp&& __val
   else if constexpr (__arg == __arg_t::__ptr)
     return basic_format_arg<_Context>{__arg, static_cast<const void*>(__value)};
   else if constexpr (__arg == __arg_t::__handle)
-    return basic_format_arg<_Context>{
-        __arg, typename __basic_format_arg_value<_Context>::__handle{_VSTD::forward<_Tp>(__value)}};
+    return basic_format_arg<_Context>{__arg, typename __basic_format_arg_value<_Context>::__handle{__value}};
   else
     return basic_format_arg<_Context>{__arg, __value};
 }

libcxx/include/__format/formatter_string.h

@@ -115,7 +115,8 @@ struct _LIBCPP_TEMPLATE_VIS formatter<_CharT[_Size], _CharT>
   using _Base = __formatter_string<_CharT>;
 
   template <class _FormatContext>
-  _LIBCPP_HIDE_FROM_ABI typename _FormatContext::iterator format(_CharT __str[_Size], _FormatContext& __ctx) const {
+  _LIBCPP_HIDE_FROM_ABI typename _FormatContext::iterator
+  format(const _CharT (&__str)[_Size], _FormatContext& __ctx) const {
     return _Base::format(basic_string_view<_CharT>(__str, _Size), __ctx);
   }
 };

libcxx/test/std/utilities/format/format.formattable/concept.formattable.compile.pass.cpp

@@ -54,7 +54,14 @@
 
 template <class T, class CharT>
 void assert_is_not_formattable() {
-  static_assert(!std::formattable<T, CharT>);
+  // clang-format off
+  static_assert(!std::formattable<      T   , CharT>);
+  static_assert(!std::formattable<      T&  , CharT>);
+  static_assert(!std::formattable<      T&& , CharT>);
+  static_assert(!std::formattable<const T   , CharT>);
+  static_assert(!std::formattable<const T&  , CharT>);
+  static_assert(!std::formattable<const T&& , CharT>);
+  // clang-format on
 }
 
 template <class T, class CharT>
@@ -66,9 +73,16 @@ void assert_is_formattable() {
 #ifndef TEST_HAS_NO_WIDE_CHARACTERS
                 || std::same_as<CharT, wchar_t>
 #endif
-  )
-    static_assert(std::formattable<T, CharT>);
-  else
+  ) {
+    // clang-format off
+    static_assert(std::formattable<      T   , CharT>);
+    static_assert(std::formattable<      T&  , CharT>);
+    static_assert(std::formattable<      T&& , CharT>);
+    static_assert(std::formattable<const T   , CharT>);
+    static_assert(std::formattable<const T&  , CharT>);
+    static_assert(std::formattable<const T&& , CharT>);
+    // clang-format on
+  } else
     assert_is_not_formattable<T, CharT>();
 }
 
@@ -243,6 +257,27 @@ void test_P2286() {
   test_P2286_vector_bool<CharT, std::vector<bool, min_allocator<bool>>>();
 }
 
+// Tests volatile quified objects are no longer formattable.
+template <class CharT>
+void test_LWG3631() {
+  assert_is_not_formattable<volatile CharT, CharT>();
+
+  assert_is_not_formattable<volatile bool, CharT>();
+
+  assert_is_not_formattable<volatile signed int, CharT>();
+  assert_is_not_formattable<volatile unsigned int, CharT>();
+
+  assert_is_not_formattable<volatile std::chrono::microseconds, CharT>();
+  assert_is_not_formattable<volatile std::chrono::sys_time<std::chrono::microseconds>, CharT>();
+  assert_is_not_formattable<volatile std::chrono::day, CharT>();
+
+  assert_is_not_formattable<std::array<volatile int, 42>, CharT>();
+
+  assert_is_not_formattable<std::pair<volatile int, int>, CharT>();
+  assert_is_not_formattable<std::pair<int, volatile int>, CharT>();
+  assert_is_not_formattable<std::pair<volatile int, volatile int>, CharT>();
+}
+
 class c {
   void f();
   void fc() const;
@@ -335,12 +370,40 @@ void test_disabled() {
   assert_is_not_formattable<std::variant<c>, CharT>();
 }
 
+struct abstract {
+  virtual ~abstract() = 0;
+};
+
+template <class CharT>
+  requires std::same_as<CharT, char>
+#ifndef TEST_HAS_NO_WIDE_CHARACTERS
+        || std::same_as<CharT, wchar_t>
+#endif
+struct std::formatter<abstract, CharT> {
+  template <class ParseContext>
+  constexpr typename ParseContext::iterator parse(ParseContext& parse_ctx) {
+    return parse_ctx.begin();
+  }
+
+  template <class FormatContext>
+  typename FormatContext::iterator format(const abstract&, FormatContext& ctx) const {
+    return ctx.out();
+  }
+};
+
+template <class CharT>
+void test_abstract_class() {
+  assert_is_formattable<abstract, CharT>();
+}
+
 template <class CharT>
 void test() {
   test_P0645<CharT>();
   test_P1361<CharT>();
   test_P1636<CharT>();
   test_P2286<CharT>();
+  test_LWG3631<CharT>();
+  test_abstract_class<CharT>();
   test_disabled<CharT>();
 }
 

libcxx/test/std/utilities/format/format.tuple/format.functions.tests.h

@@ -351,23 +351,17 @@ void run_tests(TestFunction check, ExceptionTest check_exception) {
   test_escaping<CharT>(check, std::make_pair(CharT('*'), STR("")));
   test_escaping<CharT>(check, std::make_tuple(CharT('*'), STR("")));
 
-  // Test cvref-qualified types.
+  // Test const ref-qualified types.
   // clang-format off
-  check(SV("(42)"), SV("{}"), std::tuple<               int  >{42});
-  check(SV("(42)"), SV("{}"), std::tuple<const          int  >{42});
-  check(SV("(42)"), SV("{}"), std::tuple<      volatile int  >{42});
-  check(SV("(42)"), SV("{}"), std::tuple<const volatile int  >{42});
+  check(SV("(42)"), SV("{}"), std::tuple<      int  >{42});
+  check(SV("(42)"), SV("{}"), std::tuple<const int  >{42});
 
   int answer = 42;
-  check(SV("(42)"), SV("{}"), std::tuple<               int& >{answer});
-  check(SV("(42)"), SV("{}"), std::tuple<const          int& >{answer});
-  check(SV("(42)"), SV("{}"), std::tuple<      volatile int& >{answer});
-  check(SV("(42)"), SV("{}"), std::tuple<const volatile int& >{answer});
+  check(SV("(42)"), SV("{}"), std::tuple<      int& >{answer});
+  check(SV("(42)"), SV("{}"), std::tuple<const int& >{answer});
 
-  check(SV("(42)"), SV("{}"), std::tuple<               int&&>{42});
-  check(SV("(42)"), SV("{}"), std::tuple<const          int&&>{42});
-  check(SV("(42)"), SV("{}"), std::tuple<      volatile int&&>{42});
-  check(SV("(42)"), SV("{}"), std::tuple<const volatile int&&>{42});
+  check(SV("(42)"), SV("{}"), std::tuple<      int&&>{42});
+  check(SV("(42)"), SV("{}"), std::tuple<const int&&>{42});
   // clang-format on
 }