[NFC][libc++][format] Improves diagnostics.

[Mordante]

Summary

While implementing the tests for LWG3720 I noticed the std::format
errors for non-formattable types are not user friendly (and thus hard to
write a .verify test too).

The issue stems from using a deleted function for invalid types. By
using a function that returns an invalid value the diagnostics become a
lot better. Before this change the existing "invalid value"
static_assert could never trigger. Now it can be triggered by user
code, therefore a diagnostic message has been added.

Before this change using a non-formattable type resulted in list of
error messages along the line of

.../include/c++/v1/__format/format_arg_store.h:167:29: error: call to deleted function '__determine_arg_t'
  constexpr __arg_t __arg = __determine_arg_t<_Context, remove_cvref_t<_Tp>>();
                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
.../include/c++/v1/__format/format_arg_store.h:210:54: note: in instantiation of function template specialization 'std::__format::__create_format_arg<std::format_context, char16_t &>' requested here
        basic_format_arg<_Context> __arg = __format::__create_format_arg<_Context>(__args);
                                                     ^
.../include/c++/v1/__format/format_arg_store.h:246:19: note: in instantiation of function template specialization 'std::__format::__create_packed_storage<std::format_context, int &, char16_t &>' requested here
        __format::__create_packed_storage(__storage.__types_, __storage.__values_, __args...);
                  ^
.../include/c++/v1/__format/format_functions.h:73:10: note: in instantiation of member function 'std::__format_arg_store<std::format_context, int &, char16_t &>::__format_arg_store' requested here
  return _VSTD::__format_arg_store<_Context, _Args...>(__args...);
         ^
.../include/c++/v1/__config:664:17: note: expanded from macro '_VSTD'
#  define _VSTD std
                ^
.../libcxx/test/std/utilities/format/format.string/format.string.std/lwg3720_arg_id_width_precision_allowed_types.pass.cpp:46:50: note: in instantiation of function template specialization 'std::make_format_args<std::format_context, int &, char16_t &>' requested here
    TEST_IGNORE_NODISCARD std::vformat(fmt, std::make_format_args<context_t<CharT>>(args...));
                                                 ^
.../libcxx/test/std/utilities/format/format.string/format.string.std/lwg3720_arg_id_width_precision_allowed_types.pass.cpp:69:3: note: in instantiation of function template specialization 'test_exception<char, int, char16_t>' requested here
  test_exception(SV("{:{}}"), 42, u'0');
  ^
.../libcxx/test/std/utilities/format/format.string/format.string.std/lwg3720_arg_id_width_precision_allowed_types.pass.cpp:97:3: note: in instantiation of function template specialization 'test<char>' requested here
  test<char>();
  ^
.../include/c++/v1/__format/format_arg_store.h:154:19: note: candidate function [with _Context = std::format_context, _Tp = char16_t] has been explicitly deleted
consteval __arg_t __determine_arg_t()
                  ^
.../include/c++/v1/__format/format_arg_store.h:148:19: note: candidate function [with _Context = std::format_context, _Tp = char16_t]
consteval __arg_t __determine_arg_t() {

<more errors omitted>

.../include/c++/v1/__format/format_arg_store.h:185:22: note: initializer of '__arg' is not a constant expression
.../include/c++/v1/__format/format_arg_store.h:167:21: note: declared here
  constexpr __arg_t __arg = __determine_arg_t<_Context, remove_cvref_t<_Tp>>();
                    ^
.../build/include/c++/v1/__format/format_arg_store.h:194:73: error: member reference base type 'char16_t' is not a structure or union
          __arg, basic_string_view<typename _Context::char_type>{__value.data(), __value.size()}};
                                                                 ~~~~~~~^~~~~
11 errors generated.

After the change using the same non-formmatable type gives the following
diagnostics

.../include/c++/v1/__format/format_arg_store.h:168:3: error: static assertion failed due to requirement '__arg != __arg_t::__none': the supplied type is not formattable
  static_assert(__arg != __arg_t::__none, "the supplied type is not formattable");
  ^             ~~~~~~~~~~~~~~~~~~~~~~~~
.../include/c++/v1/__format/format_arg_store.h:210:54: note: in instantiation of function template specialization 'std::__format::__create_format_arg<std::format_context, char16_t &>' requested here
        basic_format_arg<_Context> __arg = __format::__create_format_arg<_Context>(__args);
                                                     ^
.../include/c++/v1/__format/format_arg_store.h:246:19: note: in instantiation of function template specialization 'std::__format::__create_packed_storage<std::format_context, int &, char16_t &>' requested here
        __format::__create_packed_storage(__storage.__types_, __storage.__values_, __args...);
                  ^
.../include/c++/v1/__format/format_functions.h:73:10: note: in instantiation of member function 'std::__format_arg_store<std::format_context, int &, char16_t &>::__format_arg_store' requested here
  return _VSTD::__format_arg_store<_Context, _Args...>(__args...);
         ^
.../include/c++/v1/__config:664:17: note: expanded from macro '_VSTD'
#  define _VSTD std
                ^
.../libcxx/test/std/utilities/format/format.string/format.string.std/lwg3720_arg_id_width_precision_allowed_types.pass.cpp:46:50: note: in instantiation of function template specialization 'std::make_format_args<std::format_context, int &, char16_t &>' requested here
    TEST_IGNORE_NODISCARD std::vformat(fmt, std::make_format_args<context_t<CharT>>(args...));
                                                 ^
.../libcxx/test/std/utilities/format/format.string/format.string.std/lwg3720_arg_id_width_precision_allowed_types.pass.cpp:69:3: note: in instantiation of function template specialization 'test_exception<char, int, char16_t>' requested here
  test_exception(SV("{:{}}"), 42, u'0');
  ^
.../libcxx/test/std/utilities/format/format.string/format.string.std/lwg3720_arg_id_width_precision_allowed_types.pass.cpp:97:3: note: in instantiation of function template specialization 'test<char>' requested here
  test<char>();
  ^
.../include/c++/v1/__format/format_arg_store.h:168:23: note: expression evaluates to '0 != 0'
  static_assert(__arg != __arg_t::__none, "the supplied type is not formattable");
                ~~~~~~^~~~~~~~~~~~~~~~~~
1 error generated.

[ldionne]

That's a really nice catch! Could we perhaps add a libc++ specific .verify.cpp test to lock in this behavior?

libcxx/include/__format/format_arg_store.h

151–155

IMO this comment is not necessary in its current form, this is a job for the commit message instead.

[Mordante]

libcxx/include/__format/format_arg_store.h

151–155

I generally like to leave this kind of comments to indicate the current way is a conscious design decision. I usually only look at commit messages after encountering a bug/issue. I feel the commit message can be more verbose than the comments in the code.

In this case I don't feel too strongly since changing it will cause CI failures after D144326 lands.

libcxx/include/__format/format_arg_store.h

 consteval __arg_t __determine_arg_t() {
   return __arg_t::__ptr;
 }
 
 // Handle
 //
 // Note this version can't be constrained avoiding ambiguous overloads.
 // That means it can be instantiated by disabled formatters. To solve this, a
-// constrained version for not formattable formatters is added. That overload
-// is marked as deleted to fail creating a storage type for disabled formatters.
+// constrained version for not formattable formatters is added.
 template <class _Context, class _Tp>
 consteval __arg_t __determine_arg_t() {
   return __arg_t::__handle;
 }
 
+// The overload for not formattable types.
+//
+// Originially this overload was marked as deleted, this resulted in 11 error
+// messages when a non-formattable type was used. Returning the "invalid type"
+// value, reduces this to 1 error message.
 template <class _Context, class _Tp>
   requires(!__formattable<_Tp, typename _Context::char_type>)
-consteval __arg_t __determine_arg_t() = delete;
+consteval __arg_t __determine_arg_t() {
+  return __arg_t::__none;
+}
 
 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>>();
-  static_assert(__arg != __arg_t::__none);
+  static_assert(__arg != __arg_t::__none, "the supplied type is not formattable");
 
   // 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.
   if constexpr (__arg == __arg_t::__char_type)
     // On some platforms initializing a wchar_t from a char is a narrowing
     // conversion.
     return basic_format_arg<_Context>{__arg, static_cast<typename _Context::char_type>(__value)};