Skip to content

Commit cd5f575

Browse files
committed
Fixes breakage with odd format functions.
1 parent da08841 commit cd5f575

File tree

3 files changed

+273
-29
lines changed

3 files changed

+273
-29
lines changed

libcxx/include/__format/format_arg_store.h

Lines changed: 34 additions & 20 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>
@@ -227,16 +231,6 @@ template <class _Context, class _Tp>
227231
{ __f.parse(__pc) } -> same_as<typename decltype(__pc)::iterator>;
228232
};
229233

230-
constexpr bool __has_format_function = requires(_Formatter& __f, _Tp&& __t, _Context __fc) {
231-
{ __f.format(__t, __fc) };
232-
};
233-
constexpr bool __is_format_function_const_qualified = requires(const _Formatter& __cf, _Tp&& __t, _Context __fc) {
234-
{ __cf.format(__t, __fc) };
235-
};
236-
constexpr bool __correct_format_function_return_type = requires(_Formatter& __f, _Tp&& __t, _Context __fc) {
237-
{ __f.format(__t, __fc)->same_as<typename _Context::iterator> };
238-
};
239-
240234
// The reason these static_asserts are placed in an if-constexpr-chain is to
241235
// only show one error. For example, when the formatter is not specialized it
242236
// would show all static_assert messages. With this chain the compiler only
@@ -253,17 +247,37 @@ template <class _Context, class _Tp>
253247
else if constexpr (!__correct_parse_function_return_type)
254248
static_assert(false, "The required formatter specialization's parse function does not return the required type.");
255249

256-
else if constexpr (!__has_format_function)
257-
static_assert(
258-
false, "The required formatter specialization does not have a format function taking the proper arguments.");
259-
else if constexpr (!__is_format_function_const_qualified)
260-
static_assert(false, "The required formatter specialization's format function is not const qualified.");
261-
else if constexpr (!__correct_format_function_return_type)
262-
static_assert(false, "The required formatter specialization's format function does not return the required type.");
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)->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.");
263276

264-
else
265-
// This should not happen; it makes sure the code is ill-formed.
266-
static_assert(false, "The required formatter specialization is not formattable with its context.");
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+
}
267281
}
268282

269283
// The Psuedo constructor is constrained per [format.arg]/4.

libcxx/include/__format/format_functions.h

Lines changed: 23 additions & 1 deletion
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,7 +113,7 @@ class _LIBCPP_TEMPLATE_VIS __compile_time_handle {
91113
}
92114

93115
template <class _Tp>
94-
requires __formattable_with<_Tp, __format_context<_CharT>>
116+
requires __formattable_with_constexpr<_Tp, __format_context<_CharT>>
95117
_LIBCPP_HIDE_FROM_ABI constexpr void __enable() {
96118
__parse_ = [](basic_format_parse_context<_CharT>& __ctx) {
97119
formatter<_Tp, _CharT> __f;

0 commit comments

Comments
 (0)