[libc++][format] Don't instantiate direct __(u)int128_t visitation
#139662
+151
−52
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
frederick-vs-ja
added
libc++
|
@llvm/pr-subscribers-libcxx Author: A. Jiang (frederick-vs-ja) Changes... in standard There're some internal uses of Fixes #139582. Full diff: https://github.com/llvm/llvm-project/pull/139662.diff 6 Files Affected:
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..e3771fde01083 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
@@ -28,6 +28,28 @@
#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 +58,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 +85,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..a53fa9a64cb77 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
@@ -117,6 +117,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 +396,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));
|
... 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.
frederick-vs-ja
force-pushed
the
visit-no-direct-i128
branch
from
a5fe521 to
07a5963
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
... in standard
visit_format_argandbasic_format_arg::visitfunctions.There're some internal uses of
__visit_format_argwhere direct__(u)int128_tvisitation is valid. This PR doesn't change behavior of these uses.Fixes #139582.