[libcxx-commits] [libcxx] [libc++][format] Improves diagnostics. (PR #127234)

[Louis Dionne via libcxx-commits]

================
@@ -205,6 +208,149 @@ _LIBCPP_HIDE_FROM_ABI basic_format_arg<_Context> __create_format_arg(_Tp& __valu
     return basic_format_arg<_Context>{__arg, __value};
 }
 
+// Helper function that issues a diagnostic when __formattable_with<_Tp, _Context> is false.
+//
+// Since it's quite easy to make a mistake writing a formatter specialization
+// this function tries to give a better explanation. This should improve the
+// diagnostics when trying to format type that has no properly specialized
+// formatter.
+template <class _Context, class _Tp>
+[[noreturn]] _LIBCPP_HIDE_FROM_ABI constexpr void __diagnose_invalid_formatter() {
+  using _Formatter = typename _Context::template formatter_type<remove_const_t<_Tp>>;
+  constexpr bool __is_disabled =
+      !is_default_constructible_v<_Formatter> && !is_copy_constructible_v<_Formatter> &&
+      !is_move_constructible_v<_Formatter> && !is_copy_assignable_v<_Formatter> && !is_move_assignable_v<_Formatter>;
+  constexpr bool __is_semiregular = semiregular<_Formatter>;
+
+  constexpr bool __has_parse_function =
+      requires(_Formatter& __f, basic_format_parse_context<typename _Context::char_type> __pc) {
+        { __f.parse(__pc) };
+      };
+  constexpr bool __correct_parse_function_return_type =
+      requires(_Formatter& __f, basic_format_parse_context<typename _Context::char_type> __pc) {
+        { __f.parse(__pc) } -> same_as<typename decltype(__pc)::iterator>;
+      };
+
+  // The reason these static_asserts are placed in an if-constexpr-chain is to
+  // only show one error. For example, when the formatter is not specialized it
+  // would show all static_assert messages. With this chain the compiler only
+  // evaluates one static_assert.
+
+  if constexpr (__is_disabled)
+
+    static_assert(
+#  if defined(_LIBCPP_APPLE_CLANG_VER) && _LIBCPP_APPLE_CLANG_VER < 1600
+        sizeof(_Tp) == 0
+#  else
+        false
+#  endif
+        ,
+        "The required formatter specialization has not been provided.");
+  else if constexpr (!__is_semiregular)
+    static_assert(
+#  if defined(_LIBCPP_APPLE_CLANG_VER) && _LIBCPP_APPLE_CLANG_VER < 1600
+        sizeof(_Tp) == 0
+#  else
+        false
+#  endif
+        ,
+        "The required formatter specialization is not semiregular.");
+
+  else if constexpr (!__has_parse_function)
+    static_assert(
+#  if defined(_LIBCPP_APPLE_CLANG_VER) && _LIBCPP_APPLE_CLANG_VER < 1600
+        sizeof(_Tp) == 0
+#  else
+        false
+#  endif
+        ,
+        "The required formatter specialization does not have a parse function taking the proper arguments.");
+  else if constexpr (!__correct_parse_function_return_type)
+    static_assert(
+#  if defined(_LIBCPP_APPLE_CLANG_VER) && _LIBCPP_APPLE_CLANG_VER < 1600
+        sizeof(_Tp) == 0
+#  else
+        false
+#  endif
----------------
ldionne wrote:

```suggestion
        sizeof(_Tp) == 0
```

IMO the added complexity of using `sizeof(_Tp) == 0` isn't worth carrying a compiler-dependent diff, even if it goes away soon.

https://github.com/llvm/llvm-project/pull/127234