[libcxx-commits] [libcxx] [libc++][format] Don't instantiate direct `__(u)int128_t` visitation (PR #139662)
A. Jiang via libcxx-commits
libcxx-commits at lists.llvm.org
Thu Jun 12 18:07:03 PDT 2025
https://github.com/frederick-vs-ja updated https://github.com/llvm/llvm-project/pull/139662
>From 07a5963d48edb01dcb13a0ebdcdc362e1284e10e Mon Sep 17 00:00:00 2001
From: "A. Jiang" <de34 at live.cn>
Date: Tue, 13 May 2025 11:13:41 +0800
Subject: [PATCH 1/2] [libc++][format] Avoid 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.
---
libcxx/include/__format/format_arg.h | 79 +++++++------------
libcxx/include/__format/format_functions.h | 2 +-
.../include/__format/parser_std_format_spec.h | 2 +-
.../format.arg/visit.pass.cpp | 29 +++++++
.../format.arg/visit.return_type.pass.cpp | 62 +++++++++++++++
.../format.arg/visit_format_arg.pass.cpp | 29 +++++++
6 files changed, 151 insertions(+), 52 deletions(-)
diff --git a/libcxx/include/__format/format_arg.h b/libcxx/include/__format/format_arg.h
index ed5e76275ea87..0f61a6d754ee5 100644
--- a/libcxx/include/__format/format_arg.h
+++ b/libcxx/include/__format/format_arg.h
@@ -96,11 +96,16 @@ _LIBCPP_HIDE_FROM_ABI constexpr __arg_t __get_packed_type(uint64_t __types, size
return static_cast<__format::__arg_t>(__types & __packed_arg_t_mask);
}
+enum class __directly_visit_i128 : bool { __no, __yes };
+
} // namespace __format
+template <class _Context>
+class __basic_format_arg_value;
+
// This function is not user observable, so it can directly use the non-standard
// types of the "variant". See __arg_t for more details.
-template <class _Visitor, class _Context>
+template <__format::__directly_visit_i128 _DirectlyVisitingI128, class _Visitor, class _Context>
_LIBCPP_HIDE_FROM_ABI decltype(auto) __visit_format_arg(_Visitor&& __vis, basic_format_arg<_Context> __arg) {
switch (__arg.__type_) {
case __format::__arg_t::__none:
@@ -115,7 +120,12 @@ _LIBCPP_HIDE_FROM_ABI decltype(auto) __visit_format_arg(_Visitor&& __vis, basic_
return std::invoke(std::forward<_Visitor>(__vis), __arg.__value_.__long_long_);
case __format::__arg_t::__i128:
# if _LIBCPP_HAS_INT128
- return std::invoke(std::forward<_Visitor>(__vis), __arg.__value_.__i128_);
+ if constexpr (_DirectlyVisitingI128 == __format::__directly_visit_i128::__yes)
+ return std::invoke(std::forward<_Visitor>(__vis), __arg.__value_.__i128_);
+ else {
+ typename __basic_format_arg_value<_Context>::__handle __h{__arg.__value_.__i128_};
+ return std::invoke(std::forward<_Visitor>(__vis), typename basic_format_arg<_Context>::handle{__h});
+ }
# else
__libcpp_unreachable();
# endif
@@ -125,7 +135,12 @@ _LIBCPP_HIDE_FROM_ABI decltype(auto) __visit_format_arg(_Visitor&& __vis, basic_
return std::invoke(std::forward<_Visitor>(__vis), __arg.__value_.__unsigned_long_long_);
case __format::__arg_t::__u128:
# if _LIBCPP_HAS_INT128
- return std::invoke(std::forward<_Visitor>(__vis), __arg.__value_.__u128_);
+ if constexpr (_DirectlyVisitingI128 == __format::__directly_visit_i128::__yes)
+ return std::invoke(std::forward<_Visitor>(__vis), __arg.__value_.__u128_);
+ else {
+ typename __basic_format_arg_value<_Context>::__handle __h{__arg.__value_.__u128_};
+ return std::invoke(std::forward<_Visitor>(__vis), typename basic_format_arg<_Context>::handle{__h});
+ }
# else
__libcpp_unreachable();
# endif
@@ -166,7 +181,10 @@ _LIBCPP_HIDE_FROM_ABI _Rp __visit_format_arg(_Visitor&& __vis, basic_format_arg<
return std::invoke_r<_Rp>(std::forward<_Visitor>(__vis), __arg.__value_.__long_long_);
case __format::__arg_t::__i128:
# if _LIBCPP_HAS_INT128
- return std::invoke_r<_Rp>(std::forward<_Visitor>(__vis), __arg.__value_.__i128_);
+ {
+ typename __basic_format_arg_value<_Context>::__handle __h{__arg.__value_.__u128_};
+ return std::invoke_r<_Rp>(std::forward<_Visitor>(__vis), typename basic_format_arg<_Context>::handle{__h});
+ }
# else
__libcpp_unreachable();
# endif
@@ -176,7 +194,10 @@ _LIBCPP_HIDE_FROM_ABI _Rp __visit_format_arg(_Visitor&& __vis, basic_format_arg<
return std::invoke_r<_Rp>(std::forward<_Visitor>(__vis), __arg.__value_.__unsigned_long_long_);
case __format::__arg_t::__u128:
# if _LIBCPP_HAS_INT128
- return std::invoke_r<_Rp>(std::forward<_Visitor>(__vis), __arg.__value_.__u128_);
+ {
+ typename __basic_format_arg_value<_Context>::__handle __h{__arg.__value_.__u128_};
+ return std::invoke_r<_Rp>(std::forward<_Visitor>(__vis), typename basic_format_arg<_Context>::handle{__h});
+ }
# else
__libcpp_unreachable();
# endif
@@ -291,42 +312,14 @@ class _LIBCPP_NO_SPECIALIZATIONS basic_format_arg {
// the "variant" in a handle to stay conforming. See __arg_t for more details.
template <class _Visitor>
_LIBCPP_HIDE_FROM_ABI decltype(auto) visit(this basic_format_arg __arg, _Visitor&& __vis) {
- switch (__arg.__type_) {
-# if _LIBCPP_HAS_INT128
- case __format::__arg_t::__i128: {
- typename __basic_format_arg_value<_Context>::__handle __h{__arg.__value_.__i128_};
- return std::invoke(std::forward<_Visitor>(__vis), typename basic_format_arg<_Context>::handle{__h});
- }
-
- case __format::__arg_t::__u128: {
- typename __basic_format_arg_value<_Context>::__handle __h{__arg.__value_.__u128_};
- return std::invoke(std::forward<_Visitor>(__vis), typename basic_format_arg<_Context>::handle{__h});
- }
-# endif
- default:
- return std::__visit_format_arg(std::forward<_Visitor>(__vis), __arg);
- }
+ return std::__visit_format_arg<__format::__directly_visit_i128::__no>(std::forward<_Visitor>(__vis), __arg);
}
// This function is user facing, so it must wrap the non-standard types of
// the "variant" in a handle to stay conforming. See __arg_t for more details.
template <class _Rp, class _Visitor>
_LIBCPP_HIDE_FROM_ABI _Rp visit(this basic_format_arg __arg, _Visitor&& __vis) {
- switch (__arg.__type_) {
-# if _LIBCPP_HAS_INT128
- case __format::__arg_t::__i128: {
- typename __basic_format_arg_value<_Context>::__handle __h{__arg.__value_.__i128_};
- return std::invoke_r<_Rp>(std::forward<_Visitor>(__vis), typename basic_format_arg<_Context>::handle{__h});
- }
-
- case __format::__arg_t::__u128: {
- typename __basic_format_arg_value<_Context>::__handle __h{__arg.__value_.__u128_};
- return std::invoke_r<_Rp>(std::forward<_Visitor>(__vis), typename basic_format_arg<_Context>::handle{__h});
- }
-# endif
- default:
- return std::__visit_format_arg<_Rp>(std::forward<_Visitor>(__vis), __arg);
- }
+ return std::__visit_format_arg<_Rp>(std::forward<_Visitor>(__vis), __arg);
}
# endif // _LIBCPP_STD_VER >= 26 && _LIBCPP_HAS_EXPLICIT_THIS_PARAMETER
@@ -376,21 +369,7 @@ _LIBCPP_DEPRECATED_IN_CXX26
# endif
_LIBCPP_HIDE_FROM_ABI decltype(auto)
visit_format_arg(_Visitor&& __vis, basic_format_arg<_Context> __arg) {
- switch (__arg.__type_) {
-# if _LIBCPP_HAS_INT128
- case __format::__arg_t::__i128: {
- typename __basic_format_arg_value<_Context>::__handle __h{__arg.__value_.__i128_};
- return std::invoke(std::forward<_Visitor>(__vis), typename basic_format_arg<_Context>::handle{__h});
- }
-
- case __format::__arg_t::__u128: {
- typename __basic_format_arg_value<_Context>::__handle __h{__arg.__value_.__u128_};
- return std::invoke(std::forward<_Visitor>(__vis), typename basic_format_arg<_Context>::handle{__h});
- }
-# endif // _LIBCPP_STD_VER >= 26 && _LIBCPP_HAS_EXPLICIT_THIS_PARAMETER
- default:
- return std::__visit_format_arg(std::forward<_Visitor>(__vis), __arg);
- }
+ return std::__visit_format_arg<__format::__directly_visit_i128::__no>(std::forward<_Visitor>(__vis), __arg);
}
#endif // _LIBCPP_STD_VER >= 20
diff --git a/libcxx/include/__format/format_functions.h b/libcxx/include/__format/format_functions.h
index 74fec9f2761e0..adf3dea3cd632 100644
--- a/libcxx/include/__format/format_functions.h
+++ b/libcxx/include/__format/format_functions.h
@@ -272,7 +272,7 @@ __handle_replacement_field(_Iterator __begin, _Iterator __end, _ParseCtx& __pars
else if (__parse)
__format::__compile_time_visit_format_arg(__parse_ctx, __ctx, __type);
} else
- std::__visit_format_arg(
+ std::__visit_format_arg<__format::__directly_visit_i128::__yes>(
[&](auto __arg) {
if constexpr (same_as<decltype(__arg), monostate>)
std::__throw_format_error("The argument index value is too large for the number of arguments supplied");
diff --git a/libcxx/include/__format/parser_std_format_spec.h b/libcxx/include/__format/parser_std_format_spec.h
index 99ab3dc23c295..ee87deb9b6d55 100644
--- a/libcxx/include/__format/parser_std_format_spec.h
+++ b/libcxx/include/__format/parser_std_format_spec.h
@@ -91,7 +91,7 @@ _LIBCPP_HIDE_FROM_ABI constexpr uint32_t __substitute_arg_id(basic_format_arg<_C
// This means the 128-bit will not be valid anymore.
// TODO FMT Verify this resolution is accepted and add a test to verify
// 128-bit integrals fail and switch to visit_format_arg.
- return std::__visit_format_arg(
+ return std::__visit_format_arg<__format::__directly_visit_i128::__yes>(
[](auto __arg) -> uint32_t {
using _Type = decltype(__arg);
if constexpr (same_as<_Type, monostate>)
diff --git a/libcxx/test/std/utilities/format/format.arguments/format.arg/visit.pass.cpp b/libcxx/test/std/utilities/format/format.arguments/format.arg/visit.pass.cpp
index 20e0a5ed66bd0..a63ac1fc3f41b 100644
--- a/libcxx/test/std/utilities/format/format.arguments/format.arg/visit.pass.cpp
+++ b/libcxx/test/std/utilities/format/format.arguments/format.arg/visit.pass.cpp
@@ -22,12 +22,35 @@
#include <cassert>
#include <format>
#include <type_traits>
+#include <variant>
#include "constexpr_char_traits.h"
#include "make_string.h"
#include "min_allocator.h"
#include "test_macros.h"
+template <class Context>
+struct limited_visitor {
+ using CharT = Context::char_type;
+
+ void operator()(std::monostate) const {}
+ void operator()(bool) const {}
+ void operator()(CharT) const {}
+ void operator()(int) const {}
+ void operator()(unsigned int) const {}
+ void operator()(long long) const {}
+ void operator()(unsigned long long) const {}
+ void operator()(float) const {}
+ void operator()(double) const {}
+ void operator()(long double) const {}
+ void operator()(const CharT*) const {}
+ void operator()(std::basic_string_view<CharT>) const {}
+ void operator()(const void*) const {}
+ void operator()(const std::basic_format_arg<Context>::handle&) const {}
+
+ void operator()(auto) const = delete;
+};
+
template <class Context, class To, class From>
void test(From value) {
auto store = std::make_format_args<Context>(value);
@@ -36,6 +59,9 @@ void test(From value) {
LIBCPP_ASSERT(format_args.__size() == 1);
assert(format_args.get(0));
+ // https://github.com/llvm/llvm-project/issues/139582
+ format_args.get(0).visit(limited_visitor<Context>{});
+
auto result = format_args.get(0).visit([v = To(value)](auto a) -> To {
if constexpr (std::is_same_v<To, decltype(a)>) {
assert(v == a);
@@ -60,6 +86,9 @@ void test_handle(T value) {
LIBCPP_ASSERT(format_args.__size() == 1);
assert(format_args.get(0));
+ // https://github.com/llvm/llvm-project/issues/139582
+ format_args.get(0).visit(limited_visitor<Context>{});
+
format_args.get(0).visit([](auto a) {
assert((std::is_same_v<decltype(a), typename std::basic_format_arg<Context>::handle>));
});
diff --git a/libcxx/test/std/utilities/format/format.arguments/format.arg/visit.return_type.pass.cpp b/libcxx/test/std/utilities/format/format.arguments/format.arg/visit.return_type.pass.cpp
index 8a79dd4d50f20..7d45b8684eb16 100644
--- a/libcxx/test/std/utilities/format/format.arguments/format.arg/visit.return_type.pass.cpp
+++ b/libcxx/test/std/utilities/format/format.arguments/format.arg/visit.return_type.pass.cpp
@@ -22,6 +22,7 @@
#include <cassert>
#include <format>
#include <type_traits>
+#include <variant>
#include "constexpr_char_traits.h"
#include "make_string.h"
@@ -117,6 +118,48 @@ void test_string_view(From value, ExpectedR expectedValue) {
assert(result == expectedValue);
}
+template <class Context>
+struct limited_int_visitor {
+ using CharT = Context::char_type;
+
+ int operator()(std::monostate) const { return 1; }
+ int operator()(bool) const { return 2; }
+ int operator()(CharT) const { return 3; }
+ int operator()(int) const { return 4; }
+ int operator()(unsigned int) const { return 5; }
+ int operator()(long long) const { return 6; }
+ int operator()(unsigned long long) const { return 7; }
+ int operator()(float) const { return 8; }
+ int operator()(double) const { return 9; }
+ int operator()(long double) const { return 10; }
+ int operator()(const CharT*) const { return 11; }
+ int operator()(std::basic_string_view<CharT>) const { return 12; }
+ int operator()(const void*) const { return 13; }
+ int operator()(const std::basic_format_arg<Context>::handle&) const { return 14; }
+
+ void operator()(auto) const = delete;
+};
+
+// https://github.com/llvm/llvm-project/issues/139582
+template <class Context, class ExpectedR, class From>
+void test_limited_visitation(From value) {
+ auto store = std::make_format_args<Context>(value);
+ std::basic_format_args<Context> format_args{store};
+
+ LIBCPP_ASSERT(format_args.__size() == 1);
+ assert(format_args.get(0));
+
+ if constexpr (std::is_void_v<ExpectedR>) {
+ format_args.get(0).template visit<ExpectedR>(limited_int_visitor<Context>{});
+ static_assert(
+ std::is_same_v<void, decltype(format_args.get(0).template visit<ExpectedR>(limited_int_visitor<Context>{}))>);
+ } else {
+ std::same_as<ExpectedR> decltype(auto) result =
+ format_args.get(0).template visit<ExpectedR>(limited_int_visitor<Context>{});
+ assert(result);
+ }
+}
+
template <class CharT>
void test() {
using Context = std::basic_format_context<CharT*, CharT>;
@@ -354,6 +397,25 @@ void test() {
test<Context, const void*, ExpectedResultType>(static_cast<void*>(&i), visited);
const int ci = 0;
test<Context, const void*, ExpectedResultType>(static_cast<const void*>(&ci), visited);
+
+ // https://github.com/llvm/llvm-project/issues/139582
+ // Test limited visitors.
+ test_limited_visitation<Context, void>(true);
+ test_limited_visitation<Context, void>(42);
+ test_limited_visitation<Context, void>(0.5);
+ test_limited_visitation<Context, void>(str);
+#ifndef TEST_HAS_NO_INT128
+ test_limited_visitation<Context, void>(__int128_t{0});
+ test_limited_visitation<Context, void>(__uint128_t{0});
+#endif // TEST_HAS_NO_INT128
+ test_limited_visitation<Context, long>(true);
+ test_limited_visitation<Context, long>(42);
+ test_limited_visitation<Context, long>(0.5);
+ test_limited_visitation<Context, long>(str);
+#ifndef TEST_HAS_NO_INT128
+ test_limited_visitation<Context, long>(__int128_t{0});
+ test_limited_visitation<Context, long>(__uint128_t{0});
+#endif // TEST_HAS_NO_INT128
}
int main(int, char**) {
diff --git a/libcxx/test/std/utilities/format/format.arguments/format.arg/visit_format_arg.pass.cpp b/libcxx/test/std/utilities/format/format.arguments/format.arg/visit_format_arg.pass.cpp
index d99675a71f321..862abeac584be 100644
--- a/libcxx/test/std/utilities/format/format.arguments/format.arg/visit_format_arg.pass.cpp
+++ b/libcxx/test/std/utilities/format/format.arguments/format.arg/visit_format_arg.pass.cpp
@@ -19,6 +19,7 @@
#include <cassert>
#include <limits>
#include <type_traits>
+#include <variant>
#include "constexpr_char_traits.h"
#include "test_macros.h"
@@ -29,6 +30,28 @@
TEST_CLANG_DIAGNOSTIC_IGNORED("-Wdeprecated-declarations")
#endif
+template <class Context>
+struct limited_visitor {
+ using CharT = Context::char_type;
+
+ void operator()(std::monostate) const {}
+ void operator()(bool) const {}
+ void operator()(CharT) const {}
+ void operator()(int) const {}
+ void operator()(unsigned int) const {}
+ void operator()(long long) const {}
+ void operator()(unsigned long long) const {}
+ void operator()(float) const {}
+ void operator()(double) const {}
+ void operator()(long double) const {}
+ void operator()(const CharT*) const {}
+ void operator()(std::basic_string_view<CharT>) const {}
+ void operator()(const void*) const {}
+ void operator()(const std::basic_format_arg<Context>::handle&) const {}
+
+ void operator()(auto) const = delete;
+};
+
template <class Context, class To, class From>
void test(From value) {
auto store = std::make_format_args<Context>(value);
@@ -37,6 +60,9 @@ void test(From value) {
LIBCPP_ASSERT(format_args.__size() == 1);
assert(format_args.get(0));
+ // https://github.com/llvm/llvm-project/issues/139582
+ std::visit_format_arg(limited_visitor<Context>{}, format_args.get(0));
+
auto result = std::visit_format_arg(
[v = To(value)](auto a) -> To {
if constexpr (std::is_same_v<To, decltype(a)>) {
@@ -63,6 +89,9 @@ void test_handle(T value) {
LIBCPP_ASSERT(format_args.__size() == 1);
assert(format_args.get(0));
+ // https://github.com/llvm/llvm-project/issues/139582
+ std::visit_format_arg(limited_visitor<Context>{}, format_args.get(0));
+
std::visit_format_arg(
[](auto a) { assert((std::is_same_v<decltype(a), typename std::basic_format_arg<Context>::handle>)); },
format_args.get(0));
>From 2c2025de33c3a7fbc72f0f4d80bb473965b95ad9 Mon Sep 17 00:00:00 2001
From: "A. Jiang" <de34 at live.cn>
Date: Fri, 13 Jun 2025 09:06:56 +0800
Subject: [PATCH 2/2] Fix copy-pasta as suggested by @ldionne
Co-authored-by: Louis Dionne <ldionne.2 at gmail.com>
---
libcxx/include/__format/format_arg.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libcxx/include/__format/format_arg.h b/libcxx/include/__format/format_arg.h
index 0f61a6d754ee5..fc5b7de4ee8f4 100644
--- a/libcxx/include/__format/format_arg.h
+++ b/libcxx/include/__format/format_arg.h
@@ -182,7 +182,7 @@ _LIBCPP_HIDE_FROM_ABI _Rp __visit_format_arg(_Visitor&& __vis, basic_format_arg<
case __format::__arg_t::__i128:
# if _LIBCPP_HAS_INT128
{
- typename __basic_format_arg_value<_Context>::__handle __h{__arg.__value_.__u128_};
+ typename __basic_format_arg_value<_Context>::__handle __h{__arg.__value_.__i128_};
return std::invoke_r<_Rp>(std::forward<_Visitor>(__vis), typename basic_format_arg<_Context>::handle{__h});
}
# else
More information about the libcxx-commits
mailing list