Skip to content

Commit b49679e

Browse files
committed
[libc++][format] Improves diagnostics.
When attempting to format a type that does not have a valid formatter specialization the code is ill-formed and diagnostics are generated. The patch now properly constrains the exposition-only basic_format_arg constructor (LWG3246). This constructor is called by - std::make_format_args. This function has the precondition The type typename Context::template formatter_type<remove_const_t<Ti>> meets the BasicFormatter requirements ([formatter.requirements]) for each Ti in Args. - In libc++ `std::basic_format_string` constructor too. This is not specified by the standard, however the requirements for constructor allow this. Since `basic_format_arg` exposition-only constructor's constraint can't be observed by the user, they can use it to SFINAE their own code. Currently libc++, libstdc++, and MSVC STL consider the code ill-formed. This patch keeps that behaviour. This patch improves the output of these diagnostics: - Reduces the number of errors generated, which reduces the number of lines in the output. - The error gives a more specific diagnotic in multiple cases. For example, the formatter's format function must be const qualified. The diagnostic now mentions this specific issue. Summary of the error messages using the type no_formatter_specialization, which as the name implies has no specialization. std::format (and friends) Before - 163 lines of output with 7 errors - First two diagnostics: - error: static assertion failed due to requirement '__arg != __arg_t::__none': the supplied type is not formattable - note: in instantiation of function template specialization 'std::__format::__create_format_arg<std::format_context, no_formatter_specialization>' requested here After - 25 lines of output with 1 error - First two diagnostics: - error: static assertion failed: The required formatter specialization has not been provided. - note: in instantiation of function template specialization 'std::__format::__diagnose_invalid_formatter<std::format_context, no_formatter_specialization>' requested here std::make_format_args (used by users to call vformat and friends) Before - 70 lines of output with 3 errors - First two diagnostics: - error: static assertion failed due to requirement '__arg != __arg_t::__none': the supplied type is not formattable - note: in instantiation of function template specialization 'std::__format::__create_format_arg<std::format_context, no_formatter_specialization>' requested here After - 25 lines of output with 1 error - First two diagnostics: - error: static assertion failed: The required formatter specialization has not been provided. - note: in instantiation of function template specialization 'std::__format::__diagnose_invalid_formatter<std::format_context, no_formatter_specialization>' requested here Fixes: #100263
1 parent b6bb9dc commit b49679e

File tree

6 files changed

+743
-16
lines changed

6 files changed

+743
-16
lines changed

libcxx/include/__format/format_arg.h

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ class __basic_format_arg_value {
219219
__format_([](basic_format_parse_context<_CharT>& __parse_ctx, _Context& __ctx, const void* __ptr) {
220220
using _Dp = remove_const_t<_Tp>;
221221
using _Qp = conditional_t<__formattable_with<const _Dp, _Context>, const _Dp, _Dp>;
222-
static_assert(__formattable_with<_Qp, _Context>, "Mandated by [format.arg]/10");
222+
static_assert(__formattable_with<_Qp, _Context>, "Mandated by [format.arg]/12");
223223

224224
typename _Context::template formatter_type<_Dp> __f;
225225
__parse_ctx.advance_to(__f.parse(__parse_ctx));
@@ -334,21 +334,16 @@ class _LIBCPP_TEMPLATE_VIS _LIBCPP_NO_SPECIALIZATIONS basic_format_arg {
334334
private:
335335
using char_type = typename _Context::char_type;
336336

337-
// TODO FMT Implement constrain [format.arg]/4
338-
// Constraints: The template specialization
339-
// typename Context::template formatter_type<T>
340-
// meets the Formatter requirements ([formatter.requirements]). The extent
341-
// to which an implementation determines that the specialization meets the
342-
// Formatter requirements is unspecified, except that as a minimum the
343-
// expression
344-
// typename Context::template formatter_type<T>()
345-
// .format(declval<const T&>(), declval<Context&>())
346-
// shall be well-formed when treated as an unevaluated operand.
347-
348337
public:
349338
__basic_format_arg_value<_Context> __value_;
350339
__format::__arg_t __type_;
351340

341+
// This constructor is used for the exposition only constructor.
342+
// Per [format.arg]/4
343+
// template<class T> explicit basic_format_arg(T& v) noexcept;
344+
// Constraints: T satisfies formattable-with<Context>.
345+
//
346+
// This constraint is implemented in __create_format_arg
352347
_LIBCPP_HIDE_FROM_ABI explicit basic_format_arg(__format::__arg_t __type,
353348
__basic_format_arg_value<_Context> __value) noexcept
354349
: __value_(__value), __type_(__type) {}

libcxx/include/__format/format_arg_store.h

Lines changed: 94 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,15 @@
1616

1717
#include <__concepts/arithmetic.h>
1818
#include <__concepts/same_as.h>
19+
#include <__concepts/semiregular.h>
1920
#include <__config>
2021
#include <__format/concepts.h>
2122
#include <__format/format_arg.h>
23+
#include <__format/format_parse_context.h>
2224
#include <__type_traits/conditional.h>
2325
#include <__type_traits/extent.h>
26+
#include <__type_traits/is_assignable.h>
27+
#include <__type_traits/is_constructible.h>
2428
#include <__type_traits/remove_const.h>
2529
#include <cstdint>
2630
#include <string>
@@ -161,12 +165,11 @@ consteval __arg_t __determine_arg_t() {
161165
//
162166
// Modeled after template<class T> explicit basic_format_arg(T& v) noexcept;
163167
// [format.arg]/4-6
164-
template <class _Context, class _Tp>
168+
template <class _Context, __formattable_with<_Context> _Tp>
165169
_LIBCPP_HIDE_FROM_ABI basic_format_arg<_Context> __create_format_arg(_Tp& __value) noexcept {
166170
using _Dp = remove_const_t<_Tp>;
167171
constexpr __arg_t __arg = __format::__determine_arg_t<_Context, _Dp>();
168172
static_assert(__arg != __arg_t::__none, "the supplied type is not formattable");
169-
static_assert(__formattable_with<_Tp, _Context>);
170173

171174
// Not all types can be used to directly initialize the
172175
// __basic_format_arg_value. First handle all types needing adjustment, the
@@ -205,6 +208,95 @@ _LIBCPP_HIDE_FROM_ABI basic_format_arg<_Context> __create_format_arg(_Tp& __valu
205208
return basic_format_arg<_Context>{__arg, __value};
206209
}
207210

211+
// Helper function that issues a diagnostic when __formattable_with<_Tp, _Context> is false.
212+
//
213+
// Since it's quite easy to make a mistake writing a formatter specialization
214+
// this function tries to give a better explanation. This should improve the
215+
// diagnostics when trying to format type that has no properly specialized
216+
// formatter.
217+
template <class _Context, class _Tp>
218+
[[noreturn]] _LIBCPP_HIDE_FROM_ABI constexpr void __diagnose_invalid_formatter() {
219+
using _Formatter = typename _Context::template formatter_type<remove_const_t<_Tp>>;
220+
constexpr bool __is_disabled =
221+
!is_default_constructible_v<_Formatter> && !is_copy_constructible_v<_Formatter> &&
222+
!is_move_constructible_v<_Formatter> && !is_copy_assignable_v<_Formatter> && !is_move_assignable_v<_Formatter>;
223+
constexpr bool __is_semiregular = semiregular<_Formatter>;
224+
225+
constexpr bool __has_parse_function =
226+
requires(_Formatter& __f, basic_format_parse_context<typename _Context::char_type> __pc) {
227+
{ __f.parse(__pc) };
228+
};
229+
constexpr bool __correct_parse_function_return_type =
230+
requires(_Formatter& __f, basic_format_parse_context<typename _Context::char_type> __pc) {
231+
{ __f.parse(__pc) } -> same_as<typename decltype(__pc)::iterator>;
232+
};
233+
234+
// The reason these static_asserts are placed in an if-constexpr-chain is to
235+
// only show one error. For example, when the formatter is not specialized it
236+
// would show all static_assert messages. With this chain the compiler only
237+
// evaluates one static_assert.
238+
239+
if constexpr (__is_disabled)
240+
static_assert(false, "The required formatter specialization has not been provided.");
241+
else if constexpr (!__is_semiregular)
242+
static_assert(false, "The required formatter specialization is not semiregular.");
243+
244+
else if constexpr (!__has_parse_function)
245+
static_assert(
246+
false, "The required formatter specialization does not have a parse function taking the proper arguments.");
247+
else if constexpr (!__correct_parse_function_return_type)
248+
static_assert(false, "The required formatter specialization's parse function does not return the required type.");
249+
250+
else {
251+
// During constant evaluation this function is called, but the format
252+
// member function has not been evaluated. This means these functions
253+
// can't be evaluated at that time.
254+
//
255+
// Note this else branch should never been taken during constant
256+
// eveluation, the static_asserts in one of the branches above should
257+
// trigger.
258+
constexpr bool __has_format_function = requires(_Formatter& __f, _Tp&& __t, _Context __fc) {
259+
{ __f.format(__t, __fc) };
260+
};
261+
constexpr bool __is_format_function_const_qualified = requires(const _Formatter& __cf, _Tp&& __t, _Context __fc) {
262+
{ __cf.format(__t, __fc) };
263+
};
264+
constexpr bool __correct_format_function_return_type = requires(_Formatter& __f, _Tp&& __t, _Context __fc) {
265+
{ __f.format(__t, __fc)->template same_as<typename _Context::iterator> };
266+
};
267+
268+
if constexpr (!__has_format_function)
269+
static_assert(
270+
false, "The required formatter specialization does not have a format function taking the proper arguments.");
271+
else if constexpr (!__is_format_function_const_qualified)
272+
static_assert(false, "The required formatter specialization's format function is not const qualified.");
273+
else if constexpr (!__correct_format_function_return_type)
274+
static_assert(
275+
false, "The required formatter specialization's format function does not return the required type.");
276+
277+
else
278+
// This should not happen; it makes sure the code is ill-formed.
279+
static_assert(false, "The required formatter specialization is not formattable with its context.");
280+
}
281+
}
282+
283+
// The Psuedo constructor is constrained per [format.arg]/4.
284+
// The only way for a user to call __create_format_arg is from std::make_format_args.
285+
// Per [format.arg.store]/3
286+
// template<class Context = format_context, class... Args>
287+
// format-arg-store<Context, Args...> make_format_args(Args&... fmt_args);
288+
//
289+
// Preconditions: The type typename Context::template formatter_type<remove_const_t<Ti>>
290+
// meets the BasicFormatter requirements ([formatter.requirements]) for each Ti in Args.
291+
//
292+
// This function is only called when the precondition is violated.
293+
// Without this function the compilation would fail since the overload is
294+
// SFINAE'ed away. This function is used to provide better diagnostics.
295+
template <class _Context, class _Tp>
296+
_LIBCPP_HIDE_FROM_ABI basic_format_arg<_Context> __create_format_arg(_Tp&) noexcept {
297+
__format::__diagnose_invalid_formatter<_Context, _Tp>();
298+
}
299+
208300
template <class _Context, class... _Args>
209301
_LIBCPP_HIDE_FROM_ABI void
210302
__create_packed_storage(uint64_t& __types, __basic_format_arg_value<_Context>* __values, _Args&... __args) noexcept {

libcxx/include/__format/format_context.h

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,9 +65,13 @@ __format_context_create(_OutIt __out_it, basic_format_args<basic_format_context<
6565
}
6666
# endif
6767

68-
using format_context = basic_format_context<back_insert_iterator<__format::__output_buffer<char>>, char>;
68+
template <class _CharT>
69+
using __format_context _LIBCPP_NODEBUG =
70+
basic_format_context<back_insert_iterator<__format::__output_buffer<_CharT>>, _CharT>;
71+
72+
using format_context = __format_context<char>;
6973
# if _LIBCPP_HAS_WIDE_CHARACTERS
70-
using wformat_context = basic_format_context< back_insert_iterator<__format::__output_buffer<wchar_t>>, wchar_t>;
74+
using wformat_context = __format_context<wchar_t>;
7175
# endif
7276

7377
template <class _OutIt, class _CharT>

libcxx/include/__format/format_functions.h

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include <__algorithm/clamp.h>
1414
#include <__concepts/convertible_to.h>
1515
#include <__concepts/same_as.h>
16+
#include <__concepts/semiregular.h>
1617
#include <__config>
1718
#include <__format/buffer.h>
1819
#include <__format/format_arg.h>
@@ -78,6 +79,27 @@ template <class... _Args>
7879

7980
namespace __format {
8081

82+
/// A "watered-down" __formattable_with for usage during constant evaluation.
83+
///
84+
/// The member function "format" can have a decuded return type. When
85+
/// constraining the __compile_time_handle this leads to a nested concept
86+
/// eveluation that is always false. Instead ignore the "format" member function.
87+
///
88+
/// In theory the same could happen to the "parse" member function. However
89+
/// thisfunction does not is less likely to have odd cased, and this function
90+
/// is "required" to be called during constant evaluation. So if this
91+
/// function's return type needs to be deduced things will probably not work.
92+
///
93+
/// See https://github.com/llvm/llvm-project/issues/81590 and
94+
/// test/std/utilities/format/format.functions/bug_81590.compile.pass.cpp for
95+
/// an example of a problematic "format" member function.
96+
template <class _Tp, class _Context, class _Formatter = typename _Context::template formatter_type<remove_const_t<_Tp>>>
97+
concept __formattable_with_constexpr =
98+
semiregular<_Formatter> &&
99+
requires(_Formatter& __f, _Tp&& __t, basic_format_parse_context<typename _Context::char_type> __pc) {
100+
{ __f.parse(__pc) } -> same_as<typename decltype(__pc)::iterator>;
101+
};
102+
81103
/// Helper class parse and handle argument.
82104
///
83105
/// When parsing a handle which is not enabled the code is ill-formed.
@@ -91,13 +113,22 @@ class _LIBCPP_TEMPLATE_VIS __compile_time_handle {
91113
}
92114

93115
template <class _Tp>
116+
requires __formattable_with_constexpr<_Tp, __format_context<_CharT>>
94117
_LIBCPP_HIDE_FROM_ABI constexpr void __enable() {
95118
__parse_ = [](basic_format_parse_context<_CharT>& __ctx) {
96119
formatter<_Tp, _CharT> __f;
97120
__ctx.advance_to(__f.parse(__ctx));
98121
};
99122
}
100123

124+
template <class _Tp>
125+
_LIBCPP_HIDE_FROM_ABI constexpr void __enable() {
126+
// Avoids the throw set in the constructor.
127+
// Otherwise two diagnostics will be issued.
128+
__parse_ = [](basic_format_parse_context<_CharT>&) {};
129+
__format::__diagnose_invalid_formatter<__format_context<_CharT>, _Tp>();
130+
}
131+
101132
// Before calling __parse the proper handler needs to be set with __enable.
102133
// The default handler isn't a core constant expression.
103134
_LIBCPP_HIDE_FROM_ABI constexpr __compile_time_handle()

0 commit comments

Comments
 (0)