Skip to content

Commit a5fe521

Browse files
[libc++][format] Don't instantiate direct __(u)int128_t visitation
... in standard `visit_format_arg` and `basic_format_arg::visit` functions. There're some internal uses of `__visit_format_arg` where direct `__(u)int128_t` visitation is valid. This PR doesn't change behavior of these uses.
1 parent 52ed679 commit a5fe521

File tree

6 files changed

+149
-52
lines changed

6 files changed

+149
-52
lines changed

libcxx/include/__format/format_arg.h

Lines changed: 29 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -96,11 +96,16 @@ _LIBCPP_HIDE_FROM_ABI constexpr __arg_t __get_packed_type(uint64_t __types, size
9696
return static_cast<__format::__arg_t>(__types & __packed_arg_t_mask);
9797
}
9898

99+
enum class __directly_visit_i128 : bool { __no, __yes };
100+
99101
} // namespace __format
100102

103+
template <class _Context>
104+
class __basic_format_arg_value;
105+
101106
// This function is not user observable, so it can directly use the non-standard
102107
// types of the "variant". See __arg_t for more details.
103-
template <class _Visitor, class _Context>
108+
template <__format::__directly_visit_i128 _DirectlyVisitingI128, class _Visitor, class _Context>
104109
_LIBCPP_HIDE_FROM_ABI decltype(auto) __visit_format_arg(_Visitor&& __vis, basic_format_arg<_Context> __arg) {
105110
switch (__arg.__type_) {
106111
case __format::__arg_t::__none:
@@ -115,7 +120,12 @@ _LIBCPP_HIDE_FROM_ABI decltype(auto) __visit_format_arg(_Visitor&& __vis, basic_
115120
return std::invoke(std::forward<_Visitor>(__vis), __arg.__value_.__long_long_);
116121
case __format::__arg_t::__i128:
117122
# if _LIBCPP_HAS_INT128
118-
return std::invoke(std::forward<_Visitor>(__vis), __arg.__value_.__i128_);
123+
if constexpr (_DirectlyVisitingI128 == __format::__directly_visit_i128::__yes)
124+
return std::invoke(std::forward<_Visitor>(__vis), __arg.__value_.__i128_);
125+
else {
126+
typename __basic_format_arg_value<_Context>::__handle __h{__arg.__value_.__i128_};
127+
return std::invoke(std::forward<_Visitor>(__vis), typename basic_format_arg<_Context>::handle{__h});
128+
}
119129
# else
120130
__libcpp_unreachable();
121131
# endif
@@ -125,7 +135,12 @@ _LIBCPP_HIDE_FROM_ABI decltype(auto) __visit_format_arg(_Visitor&& __vis, basic_
125135
return std::invoke(std::forward<_Visitor>(__vis), __arg.__value_.__unsigned_long_long_);
126136
case __format::__arg_t::__u128:
127137
# if _LIBCPP_HAS_INT128
128-
return std::invoke(std::forward<_Visitor>(__vis), __arg.__value_.__u128_);
138+
if constexpr (_DirectlyVisitingI128 == __format::__directly_visit_i128::__yes)
139+
return std::invoke(std::forward<_Visitor>(__vis), __arg.__value_.__u128_);
140+
else {
141+
typename __basic_format_arg_value<_Context>::__handle __h{__arg.__value_.__u128_};
142+
return std::invoke(std::forward<_Visitor>(__vis), typename basic_format_arg<_Context>::handle{__h});
143+
}
129144
# else
130145
__libcpp_unreachable();
131146
# endif
@@ -166,7 +181,10 @@ _LIBCPP_HIDE_FROM_ABI _Rp __visit_format_arg(_Visitor&& __vis, basic_format_arg<
166181
return std::invoke_r<_Rp>(std::forward<_Visitor>(__vis), __arg.__value_.__long_long_);
167182
case __format::__arg_t::__i128:
168183
# if _LIBCPP_HAS_INT128
169-
return std::invoke_r<_Rp>(std::forward<_Visitor>(__vis), __arg.__value_.__i128_);
184+
{
185+
typename __basic_format_arg_value<_Context>::__handle __h{__arg.__value_.__u128_};
186+
return std::invoke_r<_Rp>(std::forward<_Visitor>(__vis), typename basic_format_arg<_Context>::handle{__h});
187+
}
170188
# else
171189
__libcpp_unreachable();
172190
# endif
@@ -176,7 +194,10 @@ _LIBCPP_HIDE_FROM_ABI _Rp __visit_format_arg(_Visitor&& __vis, basic_format_arg<
176194
return std::invoke_r<_Rp>(std::forward<_Visitor>(__vis), __arg.__value_.__unsigned_long_long_);
177195
case __format::__arg_t::__u128:
178196
# if _LIBCPP_HAS_INT128
179-
return std::invoke_r<_Rp>(std::forward<_Visitor>(__vis), __arg.__value_.__u128_);
197+
{
198+
typename __basic_format_arg_value<_Context>::__handle __h{__arg.__value_.__u128_};
199+
return std::invoke_r<_Rp>(std::forward<_Visitor>(__vis), typename basic_format_arg<_Context>::handle{__h});
200+
}
180201
# else
181202
__libcpp_unreachable();
182203
# endif
@@ -291,42 +312,14 @@ class _LIBCPP_NO_SPECIALIZATIONS basic_format_arg {
291312
// the "variant" in a handle to stay conforming. See __arg_t for more details.
292313
template <class _Visitor>
293314
_LIBCPP_HIDE_FROM_ABI decltype(auto) visit(this basic_format_arg __arg, _Visitor&& __vis) {
294-
switch (__arg.__type_) {
295-
# if _LIBCPP_HAS_INT128
296-
case __format::__arg_t::__i128: {
297-
typename __basic_format_arg_value<_Context>::__handle __h{__arg.__value_.__i128_};
298-
return std::invoke(std::forward<_Visitor>(__vis), typename basic_format_arg<_Context>::handle{__h});
299-
}
300-
301-
case __format::__arg_t::__u128: {
302-
typename __basic_format_arg_value<_Context>::__handle __h{__arg.__value_.__u128_};
303-
return std::invoke(std::forward<_Visitor>(__vis), typename basic_format_arg<_Context>::handle{__h});
304-
}
305-
# endif
306-
default:
307-
return std::__visit_format_arg(std::forward<_Visitor>(__vis), __arg);
308-
}
315+
return std::__visit_format_arg<__format::__directly_visit_i128::__no>(std::forward<_Visitor>(__vis), __arg);
309316
}
310317

311318
// This function is user facing, so it must wrap the non-standard types of
312319
// the "variant" in a handle to stay conforming. See __arg_t for more details.
313320
template <class _Rp, class _Visitor>
314321
_LIBCPP_HIDE_FROM_ABI _Rp visit(this basic_format_arg __arg, _Visitor&& __vis) {
315-
switch (__arg.__type_) {
316-
# if _LIBCPP_HAS_INT128
317-
case __format::__arg_t::__i128: {
318-
typename __basic_format_arg_value<_Context>::__handle __h{__arg.__value_.__i128_};
319-
return std::invoke_r<_Rp>(std::forward<_Visitor>(__vis), typename basic_format_arg<_Context>::handle{__h});
320-
}
321-
322-
case __format::__arg_t::__u128: {
323-
typename __basic_format_arg_value<_Context>::__handle __h{__arg.__value_.__u128_};
324-
return std::invoke_r<_Rp>(std::forward<_Visitor>(__vis), typename basic_format_arg<_Context>::handle{__h});
325-
}
326-
# endif
327-
default:
328-
return std::__visit_format_arg<_Rp>(std::forward<_Visitor>(__vis), __arg);
329-
}
322+
return std::__visit_format_arg<_Rp>(std::forward<_Visitor>(__vis), __arg);
330323
}
331324

332325
# endif // _LIBCPP_STD_VER >= 26 && _LIBCPP_HAS_EXPLICIT_THIS_PARAMETER
@@ -376,21 +369,7 @@ _LIBCPP_DEPRECATED_IN_CXX26
376369
# endif
377370
_LIBCPP_HIDE_FROM_ABI decltype(auto)
378371
visit_format_arg(_Visitor&& __vis, basic_format_arg<_Context> __arg) {
379-
switch (__arg.__type_) {
380-
# if _LIBCPP_HAS_INT128
381-
case __format::__arg_t::__i128: {
382-
typename __basic_format_arg_value<_Context>::__handle __h{__arg.__value_.__i128_};
383-
return std::invoke(std::forward<_Visitor>(__vis), typename basic_format_arg<_Context>::handle{__h});
384-
}
385-
386-
case __format::__arg_t::__u128: {
387-
typename __basic_format_arg_value<_Context>::__handle __h{__arg.__value_.__u128_};
388-
return std::invoke(std::forward<_Visitor>(__vis), typename basic_format_arg<_Context>::handle{__h});
389-
}
390-
# endif // _LIBCPP_STD_VER >= 26 && _LIBCPP_HAS_EXPLICIT_THIS_PARAMETER
391-
default:
392-
return std::__visit_format_arg(std::forward<_Visitor>(__vis), __arg);
393-
}
372+
return std::__visit_format_arg<__format::__directly_visit_i128::__no>(std::forward<_Visitor>(__vis), __arg);
394373
}
395374

396375
#endif // _LIBCPP_STD_VER >= 20

libcxx/include/__format/format_functions.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,7 @@ __handle_replacement_field(_Iterator __begin, _Iterator __end, _ParseCtx& __pars
272272
else if (__parse)
273273
__format::__compile_time_visit_format_arg(__parse_ctx, __ctx, __type);
274274
} else
275-
std::__visit_format_arg(
275+
std::__visit_format_arg<__format::__directly_visit_i128::__yes>(
276276
[&](auto __arg) {
277277
if constexpr (same_as<decltype(__arg), monostate>)
278278
std::__throw_format_error("The argument index value is too large for the number of arguments supplied");

libcxx/include/__format/parser_std_format_spec.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ _LIBCPP_HIDE_FROM_ABI constexpr uint32_t __substitute_arg_id(basic_format_arg<_C
9191
// This means the 128-bit will not be valid anymore.
9292
// TODO FMT Verify this resolution is accepted and add a test to verify
9393
// 128-bit integrals fail and switch to visit_format_arg.
94-
return std::__visit_format_arg(
94+
return std::__visit_format_arg<__format::__directly_visit_i128::__yes>(
9595
[](auto __arg) -> uint32_t {
9696
using _Type = decltype(__arg);
9797
if constexpr (same_as<_Type, monostate>)

libcxx/test/std/utilities/format/format.arguments/format.arg/visit.pass.cpp

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,28 @@
2828
#include "min_allocator.h"
2929
#include "test_macros.h"
3030

31+
template <class Context>
32+
struct limited_visitor {
33+
using CharT = Context::char_type;
34+
35+
void operator()(std::monostate) const {}
36+
void operator()(bool) const {}
37+
void operator()(CharT) const {}
38+
void operator()(int) const {}
39+
void operator()(unsigned int) const {}
40+
void operator()(long long) const {}
41+
void operator()(unsigned long long) const {}
42+
void operator()(float) const {}
43+
void operator()(double) const {}
44+
void operator()(long double) const {}
45+
void operator()(const CharT*) const {}
46+
void operator()(std::basic_string_view<CharT>) const {}
47+
void operator()(const void*) const {}
48+
void operator()(const std::basic_format_arg<Context>::handle&) const {}
49+
50+
void operator()(auto) const = delete;
51+
};
52+
3153
template <class Context, class To, class From>
3254
void test(From value) {
3355
auto store = std::make_format_args<Context>(value);
@@ -36,6 +58,9 @@ void test(From value) {
3658
LIBCPP_ASSERT(format_args.__size() == 1);
3759
assert(format_args.get(0));
3860

61+
// https://github.com/llvm/llvm-project/issues/139582
62+
format_args.get(0).visit(limited_visitor<Context>{});
63+
3964
auto result = format_args.get(0).visit([v = To(value)](auto a) -> To {
4065
if constexpr (std::is_same_v<To, decltype(a)>) {
4166
assert(v == a);
@@ -60,6 +85,9 @@ void test_handle(T value) {
6085
LIBCPP_ASSERT(format_args.__size() == 1);
6186
assert(format_args.get(0));
6287

88+
// https://github.com/llvm/llvm-project/issues/139582
89+
format_args.get(0).visit(limited_visitor<Context>{});
90+
6391
format_args.get(0).visit([](auto a) {
6492
assert((std::is_same_v<decltype(a), typename std::basic_format_arg<Context>::handle>));
6593
});

libcxx/test/std/utilities/format/format.arguments/format.arg/visit.return_type.pass.cpp

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,48 @@ void test_string_view(From value, ExpectedR expectedValue) {
117117
assert(result == expectedValue);
118118
}
119119

120+
template <class Context>
121+
struct limited_int_visitor {
122+
using CharT = Context::char_type;
123+
124+
int operator()(std::monostate) const { return 1; }
125+
int operator()(bool) const { return 2; }
126+
int operator()(CharT) const { return 3; }
127+
int operator()(int) const { return 4; }
128+
int operator()(unsigned int) const { return 5; }
129+
int operator()(long long) const { return 6; }
130+
int operator()(unsigned long long) const { return 7; }
131+
int operator()(float) const { return 8; }
132+
int operator()(double) const { return 9; }
133+
int operator()(long double) const { return 10; }
134+
int operator()(const CharT*) const { return 11; }
135+
int operator()(std::basic_string_view<CharT>) const { return 12; }
136+
int operator()(const void*) const { return 13; }
137+
int operator()(const std::basic_format_arg<Context>::handle&) const { return 14; }
138+
139+
void operator()(auto) const = delete;
140+
};
141+
142+
// https://github.com/llvm/llvm-project/issues/139582
143+
template <class Context, class ExpectedR, class From>
144+
void test_limited_visitation(From value) {
145+
auto store = std::make_format_args<Context>(value);
146+
std::basic_format_args<Context> format_args{store};
147+
148+
LIBCPP_ASSERT(format_args.__size() == 1);
149+
assert(format_args.get(0));
150+
151+
if constexpr (std::is_void_v<ExpectedR>) {
152+
format_args.get(0).template visit<ExpectedR>(limited_int_visitor<Context>{});
153+
static_assert(
154+
std::is_same_v<void, decltype(format_args.get(0).template visit<ExpectedR>(limited_int_visitor<Context>{}))>);
155+
} else {
156+
std::same_as<ExpectedR> decltype(auto) result =
157+
format_args.get(0).template visit<ExpectedR>(limited_int_visitor<Context>{});
158+
assert(result);
159+
}
160+
}
161+
120162
template <class CharT>
121163
void test() {
122164
using Context = std::basic_format_context<CharT*, CharT>;
@@ -354,6 +396,25 @@ void test() {
354396
test<Context, const void*, ExpectedResultType>(static_cast<void*>(&i), visited);
355397
const int ci = 0;
356398
test<Context, const void*, ExpectedResultType>(static_cast<const void*>(&ci), visited);
399+
400+
// https://github.com/llvm/llvm-project/issues/139582
401+
// Test limited visitors.
402+
test_limited_visitation<Context, void>(true);
403+
test_limited_visitation<Context, void>(42);
404+
test_limited_visitation<Context, void>(0.5);
405+
test_limited_visitation<Context, void>(str);
406+
#ifndef TEST_HAS_NO_INT128
407+
test_limited_visitation<Context, void>(__int128_t{0});
408+
test_limited_visitation<Context, void>(__uint128_t{0});
409+
#endif // TEST_HAS_NO_INT128
410+
test_limited_visitation<Context, long>(true);
411+
test_limited_visitation<Context, long>(42);
412+
test_limited_visitation<Context, long>(0.5);
413+
test_limited_visitation<Context, long>(str);
414+
#ifndef TEST_HAS_NO_INT128
415+
test_limited_visitation<Context, long>(__int128_t{0});
416+
test_limited_visitation<Context, long>(__uint128_t{0});
417+
#endif // TEST_HAS_NO_INT128
357418
}
358419

359420
int main(int, char**) {

libcxx/test/std/utilities/format/format.arguments/format.arg/visit_format_arg.pass.cpp

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#include <cassert>
2020
#include <limits>
2121
#include <type_traits>
22+
#include <variant>
2223

2324
#include "constexpr_char_traits.h"
2425
#include "test_macros.h"
@@ -29,6 +30,28 @@
2930
TEST_CLANG_DIAGNOSTIC_IGNORED("-Wdeprecated-declarations")
3031
#endif
3132

33+
template <class Context>
34+
struct limited_visitor {
35+
using CharT = Context::char_type;
36+
37+
void operator()(std::monostate) const {}
38+
void operator()(bool) const {}
39+
void operator()(CharT) const {}
40+
void operator()(int) const {}
41+
void operator()(unsigned int) const {}
42+
void operator()(long long) const {}
43+
void operator()(unsigned long long) const {}
44+
void operator()(float) const {}
45+
void operator()(double) const {}
46+
void operator()(long double) const {}
47+
void operator()(const CharT*) const {}
48+
void operator()(std::basic_string_view<CharT>) const {}
49+
void operator()(const void*) const {}
50+
void operator()(const std::basic_format_arg<Context>::handle&) const {}
51+
52+
void operator()(auto) const = delete;
53+
};
54+
3255
template <class Context, class To, class From>
3356
void test(From value) {
3457
auto store = std::make_format_args<Context>(value);
@@ -37,6 +60,9 @@ void test(From value) {
3760
LIBCPP_ASSERT(format_args.__size() == 1);
3861
assert(format_args.get(0));
3962

63+
// https://github.com/llvm/llvm-project/issues/139582
64+
std::visit_format_arg(limited_visitor<Context>{}, format_args.get(0));
65+
4066
auto result = std::visit_format_arg(
4167
[v = To(value)](auto a) -> To {
4268
if constexpr (std::is_same_v<To, decltype(a)>) {
@@ -63,6 +89,9 @@ void test_handle(T value) {
6389
LIBCPP_ASSERT(format_args.__size() == 1);
6490
assert(format_args.get(0));
6591

92+
// https://github.com/llvm/llvm-project/issues/139582
93+
std::visit_format_arg(limited_visitor<Context>{}, format_args.get(0));
94+
6695
std::visit_format_arg(
6796
[](auto a) { assert((std::is_same_v<decltype(a), typename std::basic_format_arg<Context>::handle>)); },
6897
format_args.get(0));

0 commit comments

Comments
 (0)