[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
Wed Jun 18 01:31:21 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/7] [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/7] 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
>From 19992589b9e22510ff4540408ffacf9588f3117f Mon Sep 17 00:00:00 2001
From: "A. Jiang" <de34 at live.cn>
Date: Fri, 13 Jun 2025 15:25:44 +0800
Subject: [PATCH 3/7] Missed direct visitation
---
libcxx/include/__format/format_functions.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libcxx/include/__format/format_functions.h b/libcxx/include/__format/format_functions.h
index 6428ac76c58b0..4ab034dc0db04 100644
--- a/libcxx/include/__format/format_functions.h
+++ b/libcxx/include/__format/format_functions.h
@@ -468,7 +468,7 @@ template <class _CharT>
if (auto __only_first_arg = __fmt == _LIBCPP_STATICALLY_WIDEN(_CharT, "{}");
__builtin_constant_p(__only_first_arg) && __only_first_arg) {
if (auto __arg = __args.get(0); __builtin_constant_p(__arg.__type_)) {
- return std::__visit_format_arg(
+ return std::__visit_format_arg<__format::__directly_visit_i128::__yes>(
[]<class _Tp>(_Tp&& __argument) -> optional<basic_string<_CharT>> {
if constexpr (is_same_v<remove_cvref_t<_Tp>, basic_string_view<_CharT>>) {
return basic_string<_CharT>{__argument};
>From 437de018ff7b02f0a782b03cdbea05d2a4f66115 Mon Sep 17 00:00:00 2001
From: "A. Jiang" <de34 at live.cn>
Date: Fri, 13 Jun 2025 17:03:17 +0800
Subject: [PATCH 4/7] Address review comments
---
libcxx/include/__format/format_arg.h | 4 ++
.../include/__format/parser_std_format_spec.h | 25 +++-----
.../format.arg/visit.pass.cpp | 24 +------
.../format.arg/visit.return_type.pass.cpp | 24 +------
.../format.arg/visit_format_arg.pass.cpp | 24 +------
.../format.arguments/format.arg/visitors.h | 62 +++++++++++++++++++
6 files changed, 79 insertions(+), 84 deletions(-)
create mode 100644 libcxx/test/std/utilities/format/format.arguments/format.arg/visitors.h
diff --git a/libcxx/include/__format/format_arg.h b/libcxx/include/__format/format_arg.h
index fc5b7de4ee8f4..5b26cd657eb72 100644
--- a/libcxx/include/__format/format_arg.h
+++ b/libcxx/include/__format/format_arg.h
@@ -96,6 +96,10 @@ _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);
}
+// Per [format.arg], the variant alternative types are fully specified, so we need to avoid direct visitation of 128-bit
+// extended integer types when the visitor is user-provided.
+// However, when the visitor is controlled by the libc++ itself, we can still perform direct visitation. See also
+// https://reviews.llvm.org/D138052.
enum class __directly_visit_i128 : bool { __no, __yes };
} // namespace __format
diff --git a/libcxx/include/__format/parser_std_format_spec.h b/libcxx/include/__format/parser_std_format_spec.h
index ee87deb9b6d55..336caa834e495 100644
--- a/libcxx/include/__format/parser_std_format_spec.h
+++ b/libcxx/include/__format/parser_std_format_spec.h
@@ -83,26 +83,21 @@ __parse_arg_id(_Iterator __begin, _Iterator __end, _ParseContext& __ctx) {
template <class _Context>
_LIBCPP_HIDE_FROM_ABI constexpr uint32_t __substitute_arg_id(basic_format_arg<_Context> __format_arg) {
- // [format.string.std]/8
- // If the corresponding formatting argument is not of integral type...
- // This wording allows char and bool too. LWG-3720 changes the wording to
- // If the corresponding formatting argument is not of standard signed or
- // unsigned integer type,
- // 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<__format::__directly_visit_i128::__yes>(
+ // [format.string.std]/10
+ // [...] The option is valid only if the corresponding formatting argument is of standard signed or unsigned integer
+ // type. [...]
+ // This means 128-bit extented integer types are invalid here.
+ return std::__visit_format_arg<__format::__directly_visit_i128::__no>(
[](auto __arg) -> uint32_t {
using _Type = decltype(__arg);
if constexpr (same_as<_Type, monostate>)
std::__throw_format_error("The argument index value is too large for the number of arguments supplied");
- // [format.string.std]/8
- // If { arg-idopt } is used in a width or precision, the value of the
- // corresponding formatting argument is used in its place. If the
- // corresponding formatting argument is not of standard signed or unsigned
- // integer type, or its value is negative for precision or non-positive for
- // width, an exception of type format_error is thrown.
+ // [format.string.std]/10
+ // If { arg-idopt } is used in a width or precision option, the value of the corresponding formatting argument
+ // is used as the value of the option. The option is valid only if the corresponding formatting argument is of
+ // standard signed or unsigned integer type. If its value is negative, an exception of type format_error is
+ // thrown.
//
// When an integral is used in a format function, it is stored as one of
// the types checked below. Other integral types are promoted. For example,
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 a63ac1fc3f41b..2013b41ceaaf6 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,34 +22,12 @@
#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;
-};
+#include "visitors.h"
template <class Context, class To, class From>
void test(From value) {
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 7d45b8684eb16..e32beea7d1156 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,12 +22,12 @@
#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"
+#include "visitors.h
// The expected result type shouldn't matter,therefore use a hardcoded value for simplicity.
using ExpectedResultType = bool;
@@ -118,28 +118,6 @@ 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) {
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 862abeac584be..55a58cff9a861 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,39 +19,17 @@
#include <cassert>
#include <limits>
#include <type_traits>
-#include <variant>
#include "constexpr_char_traits.h"
#include "test_macros.h"
#include "make_string.h"
#include "min_allocator.h"
+#include "visitors.h"
#if TEST_STD_VER >= 26 && defined(TEST_HAS_EXPLICIT_THIS_PARAMETER)
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);
diff --git a/libcxx/test/std/utilities/format/format.arguments/format.arg/visitors.h b/libcxx/test/std/utilities/format/format.arguments/format.arg/visitors.h
new file mode 100644
index 0000000000000..7cbad66e23f05
--- /dev/null
+++ b/libcxx/test/std/utilities/format/format.arguments/format.arg/visitors.h
@@ -0,0 +1,62 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef TEST_STD_UTILITIES_FORMAT_FORMAT_FORMAT_ARGUMENTS_FORMAT_ARG_VISITORS_H
+#define TEST_STD_UTILITIES_FORMAT_FORMAT_FORMAT_ARGUMENTS_FORMAT_ARG_VISITORS_H
+
+#include <concepts>
+#include <format>
+#include <string_view>
+#include <type_traits>
+#include <variant>
+
+// [format.arg]
+// namespace std {
+// template<class Context>
+// class basic_format_arg {
+// public:
+// class handle;
+// private:
+// [...]
+// using char_type = typename Context::char_type; // exposition only
+//
+// variant<monostate, bool, char_type,
+// int, unsigned int, long long int, unsigned long long int,
+// float, double, long double,
+// const char_type*, basic_string_view<char_type>,
+// const void*, handle> value; // exposition only
+// [...]
+// };
+// }
+
+template <class T, class Context>
+concept format_arg_visit_type_for =
+ std::same_as<T, std::monostate> || std::same_as<T, bool> || std::same_as<T, typename Context::char_type> ||
+ std::same_as<T, int> || std::same_as<T, unsigned int> || std::same_as<T, long long> ||
+ std::same_as<T, unsigned long long> || std::same_as<T, float> || std::same_as<T, double> ||
+ std::same_as<T, long double> || std::same_as<T, const typename Context::char_type*> ||
+ std::same_as<T, std::basic_string_view<typename Context::char_type>> || std::same_as<T, const void*> ||
+ std::same_as<T, typename std::basic_format_arg<Context>::handle>;
+
+// Verify that visitors don't see other types.
+
+template <class Context>
+struct limited_visitor {
+ void operator()(auto) const = delete;
+
+ void operator()(format_arg_visit_type_for<Context> auto) const {}
+};
+
+template <class Context>
+struct limited_int_visitor {
+ void operator()(auto) const = delete;
+
+ int operator()(format_arg_visit_type_for<Context> auto) const { return 42; }
+};
+
+#endif // TEST_STD_UTILITIES_FORMAT_FORMAT_FORMAT_ARGUMENTS_FORMAT_ARG_VISITORS_H
>From 3ca59cb23e062b237423f779e4255a5012f83072 Mon Sep 17 00:00:00 2001
From: "A. Jiang" <de34 at live.cn>
Date: Fri, 13 Jun 2025 17:39:39 +0800
Subject: [PATCH 5/7] Add missed `"`
---
.../format.arguments/format.arg/visit.return_type.pass.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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 e32beea7d1156..39cd757f1c568 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
@@ -27,7 +27,7 @@
#include "make_string.h"
#include "min_allocator.h"
#include "test_macros.h"
-#include "visitors.h
+#include "visitors.h"
// The expected result type shouldn't matter,therefore use a hardcoded value for simplicity.
using ExpectedResultType = bool;
>From d7b11676c7dc9b1ec04b04cac0e20e9fde9d950e Mon Sep 17 00:00:00 2001
From: "A. Jiang" <de34 at live.cn>
Date: Mon, 16 Jun 2025 18:14:51 +0800
Subject: [PATCH 6/7] Attempt to drop unused `<type_traits>`
---
.../std/utilities/format/format.arguments/format.arg/visitors.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/libcxx/test/std/utilities/format/format.arguments/format.arg/visitors.h b/libcxx/test/std/utilities/format/format.arguments/format.arg/visitors.h
index 7cbad66e23f05..a188a0089f99c 100644
--- a/libcxx/test/std/utilities/format/format.arguments/format.arg/visitors.h
+++ b/libcxx/test/std/utilities/format/format.arguments/format.arg/visitors.h
@@ -12,7 +12,6 @@
#include <concepts>
#include <format>
#include <string_view>
-#include <type_traits>
#include <variant>
// [format.arg]
>From d02714d71380c20038624dc4879f3300d75d1744 Mon Sep 17 00:00:00 2001
From: "A. Jiang" <de34 at live.cn>
Date: Wed, 18 Jun 2025 16:30:20 +0800
Subject: [PATCH 7/7] Avoid forward declaration for `__basic_format_arg_value`
---
libcxx/include/__format/format_arg.h | 147 +++++++++++++--------------
1 file changed, 72 insertions(+), 75 deletions(-)
diff --git a/libcxx/include/__format/format_arg.h b/libcxx/include/__format/format_arg.h
index 5b26cd657eb72..28bbc6114b94e 100644
--- a/libcxx/include/__format/format_arg.h
+++ b/libcxx/include/__format/format_arg.h
@@ -104,8 +104,79 @@ enum class __directly_visit_i128 : bool { __no, __yes };
} // namespace __format
+/// Contains the values used in basic_format_arg.
+///
+/// This is a separate type so it's possible to store the values and types in
+/// separate arrays.
template <class _Context>
-class __basic_format_arg_value;
+class __basic_format_arg_value {
+ using _CharT _LIBCPP_NODEBUG = typename _Context::char_type;
+
+public:
+ /// Contains the implementation for basic_format_arg::handle.
+ struct __handle {
+ template <class _Tp>
+ _LIBCPP_HIDE_FROM_ABI explicit __handle(_Tp& __v) noexcept
+ : __ptr_(std::addressof(__v)),
+ __format_([](basic_format_parse_context<_CharT>& __parse_ctx, _Context& __ctx, const void* __ptr) {
+ using _Dp = remove_const_t<_Tp>;
+ using _Qp = conditional_t<__formattable_with<const _Dp, _Context>, const _Dp, _Dp>;
+ static_assert(__formattable_with<_Qp, _Context>, "Mandated by [format.arg]/10");
+
+ typename _Context::template formatter_type<_Dp> __f;
+ __parse_ctx.advance_to(__f.parse(__parse_ctx));
+ __ctx.advance_to(__f.format(*const_cast<_Qp*>(static_cast<const _Dp*>(__ptr)), __ctx));
+ }) {}
+
+ const void* __ptr_;
+ void (*__format_)(basic_format_parse_context<_CharT>&, _Context&, const void*);
+ };
+
+ union {
+ monostate __monostate_;
+ bool __boolean_;
+ _CharT __char_type_;
+ int __int_;
+ unsigned __unsigned_;
+ long long __long_long_;
+ unsigned long long __unsigned_long_long_;
+# if _LIBCPP_HAS_INT128
+ __int128_t __i128_;
+ __uint128_t __u128_;
+# endif
+ float __float_;
+ double __double_;
+ long double __long_double_;
+ const _CharT* __const_char_type_ptr_;
+ basic_string_view<_CharT> __string_view_;
+ const void* __ptr_;
+ __handle __handle_;
+ };
+
+ // These constructors contain the exact storage type used. If adjustments are
+ // required, these will be done in __create_format_arg.
+
+ _LIBCPP_HIDE_FROM_ABI __basic_format_arg_value() noexcept : __monostate_() {}
+ _LIBCPP_HIDE_FROM_ABI __basic_format_arg_value(bool __value) noexcept : __boolean_(__value) {}
+ _LIBCPP_HIDE_FROM_ABI __basic_format_arg_value(_CharT __value) noexcept : __char_type_(__value) {}
+ _LIBCPP_HIDE_FROM_ABI __basic_format_arg_value(int __value) noexcept : __int_(__value) {}
+ _LIBCPP_HIDE_FROM_ABI __basic_format_arg_value(unsigned __value) noexcept : __unsigned_(__value) {}
+ _LIBCPP_HIDE_FROM_ABI __basic_format_arg_value(long long __value) noexcept : __long_long_(__value) {}
+ _LIBCPP_HIDE_FROM_ABI __basic_format_arg_value(unsigned long long __value) noexcept
+ : __unsigned_long_long_(__value) {}
+# if _LIBCPP_HAS_INT128
+ _LIBCPP_HIDE_FROM_ABI __basic_format_arg_value(__int128_t __value) noexcept : __i128_(__value) {}
+ _LIBCPP_HIDE_FROM_ABI __basic_format_arg_value(__uint128_t __value) noexcept : __u128_(__value) {}
+# endif
+ _LIBCPP_HIDE_FROM_ABI __basic_format_arg_value(float __value) noexcept : __float_(__value) {}
+ _LIBCPP_HIDE_FROM_ABI __basic_format_arg_value(double __value) noexcept : __double_(__value) {}
+ _LIBCPP_HIDE_FROM_ABI __basic_format_arg_value(long double __value) noexcept : __long_double_(__value) {}
+ _LIBCPP_HIDE_FROM_ABI __basic_format_arg_value(const _CharT* __value) noexcept : __const_char_type_ptr_(__value) {}
+ _LIBCPP_HIDE_FROM_ABI __basic_format_arg_value(basic_string_view<_CharT> __value) noexcept
+ : __string_view_(__value) {}
+ _LIBCPP_HIDE_FROM_ABI __basic_format_arg_value(const void* __value) noexcept : __ptr_(__value) {}
+ _LIBCPP_HIDE_FROM_ABI __basic_format_arg_value(__handle&& __value) noexcept : __handle_(std::move(__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.
@@ -227,80 +298,6 @@ _LIBCPP_HIDE_FROM_ABI _Rp __visit_format_arg(_Visitor&& __vis, basic_format_arg<
# endif // _LIBCPP_STD_VER >= 26 && _LIBCPP_HAS_EXPLICIT_THIS_PARAMETER
-/// Contains the values used in basic_format_arg.
-///
-/// This is a separate type so it's possible to store the values and types in
-/// separate arrays.
-template <class _Context>
-class __basic_format_arg_value {
- using _CharT _LIBCPP_NODEBUG = typename _Context::char_type;
-
-public:
- /// Contains the implementation for basic_format_arg::handle.
- struct __handle {
- template <class _Tp>
- _LIBCPP_HIDE_FROM_ABI explicit __handle(_Tp& __v) noexcept
- : __ptr_(std::addressof(__v)),
- __format_([](basic_format_parse_context<_CharT>& __parse_ctx, _Context& __ctx, const void* __ptr) {
- using _Dp = remove_const_t<_Tp>;
- using _Qp = conditional_t<__formattable_with<const _Dp, _Context>, const _Dp, _Dp>;
- static_assert(__formattable_with<_Qp, _Context>, "Mandated by [format.arg]/10");
-
- typename _Context::template formatter_type<_Dp> __f;
- __parse_ctx.advance_to(__f.parse(__parse_ctx));
- __ctx.advance_to(__f.format(*const_cast<_Qp*>(static_cast<const _Dp*>(__ptr)), __ctx));
- }) {}
-
- const void* __ptr_;
- void (*__format_)(basic_format_parse_context<_CharT>&, _Context&, const void*);
- };
-
- union {
- monostate __monostate_;
- bool __boolean_;
- _CharT __char_type_;
- int __int_;
- unsigned __unsigned_;
- long long __long_long_;
- unsigned long long __unsigned_long_long_;
-# if _LIBCPP_HAS_INT128
- __int128_t __i128_;
- __uint128_t __u128_;
-# endif
- float __float_;
- double __double_;
- long double __long_double_;
- const _CharT* __const_char_type_ptr_;
- basic_string_view<_CharT> __string_view_;
- const void* __ptr_;
- __handle __handle_;
- };
-
- // These constructors contain the exact storage type used. If adjustments are
- // required, these will be done in __create_format_arg.
-
- _LIBCPP_HIDE_FROM_ABI __basic_format_arg_value() noexcept : __monostate_() {}
- _LIBCPP_HIDE_FROM_ABI __basic_format_arg_value(bool __value) noexcept : __boolean_(__value) {}
- _LIBCPP_HIDE_FROM_ABI __basic_format_arg_value(_CharT __value) noexcept : __char_type_(__value) {}
- _LIBCPP_HIDE_FROM_ABI __basic_format_arg_value(int __value) noexcept : __int_(__value) {}
- _LIBCPP_HIDE_FROM_ABI __basic_format_arg_value(unsigned __value) noexcept : __unsigned_(__value) {}
- _LIBCPP_HIDE_FROM_ABI __basic_format_arg_value(long long __value) noexcept : __long_long_(__value) {}
- _LIBCPP_HIDE_FROM_ABI __basic_format_arg_value(unsigned long long __value) noexcept
- : __unsigned_long_long_(__value) {}
-# if _LIBCPP_HAS_INT128
- _LIBCPP_HIDE_FROM_ABI __basic_format_arg_value(__int128_t __value) noexcept : __i128_(__value) {}
- _LIBCPP_HIDE_FROM_ABI __basic_format_arg_value(__uint128_t __value) noexcept : __u128_(__value) {}
-# endif
- _LIBCPP_HIDE_FROM_ABI __basic_format_arg_value(float __value) noexcept : __float_(__value) {}
- _LIBCPP_HIDE_FROM_ABI __basic_format_arg_value(double __value) noexcept : __double_(__value) {}
- _LIBCPP_HIDE_FROM_ABI __basic_format_arg_value(long double __value) noexcept : __long_double_(__value) {}
- _LIBCPP_HIDE_FROM_ABI __basic_format_arg_value(const _CharT* __value) noexcept : __const_char_type_ptr_(__value) {}
- _LIBCPP_HIDE_FROM_ABI __basic_format_arg_value(basic_string_view<_CharT> __value) noexcept
- : __string_view_(__value) {}
- _LIBCPP_HIDE_FROM_ABI __basic_format_arg_value(const void* __value) noexcept : __ptr_(__value) {}
- _LIBCPP_HIDE_FROM_ABI __basic_format_arg_value(__handle&& __value) noexcept : __handle_(std::move(__value)) {}
-};
-
template <class _Context>
class _LIBCPP_NO_SPECIALIZATIONS basic_format_arg {
public:
More information about the libcxx-commits
mailing list