Skip to content

[libc++][format] Don't instantiate direct __(u)int128_t visitation #139662

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 29 additions & 50 deletions libcxx/include/__format/format_arg.h
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion libcxx/include/__format/format_functions.h
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
2 changes: 1 addition & 1 deletion libcxx/include/__format/parser_std_format_spec.h
Original file line number Diff line number Diff line change
Expand Up @@ -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>)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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>));
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include <cassert>
#include <format>
#include <type_traits>
#include <variant>

#include "constexpr_char_traits.h"
#include "make_string.h"
Expand Down Expand Up @@ -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>;
Expand Down Expand Up @@ -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**) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include <cassert>
#include <limits>
#include <type_traits>
#include <variant>

#include "constexpr_char_traits.h"
#include "test_macros.h"
Expand All @@ -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);
Expand All @@ -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)>) {
Expand All @@ -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));
Expand Down
Loading