diff --git a/libcxx/include/__format/format_arg.h b/libcxx/include/__format/format_arg.h --- a/libcxx/include/__format/format_arg.h +++ b/libcxx/include/__format/format_arg.h @@ -45,16 +45,22 @@ /// It could be packed in 4-bits but that means a new type directly becomes an /// ABI break. The packed type is 64-bit so this reduces the maximum number of /// packed elements from 16 to 12. +/// +/// @note Some members of this enum are an extension. These extensions need +/// special behaviour in visit_format_arg. There they need to be wrapped in a +/// handle to satisfy the user observable behaviour. The internal function +/// __visit_format_arg doesn't do this wrapping. So in the format functions +/// this function is used to avoid unneeded overhead. enum class _LIBCPP_ENUM_VIS __arg_t : uint8_t { __none, __boolean, __char_type, __int, __long_long, - __i128, + __i128, // extension __unsigned, __unsigned_long_long, - __u128, + __u128, // extension __float, __double, __long_double, @@ -85,9 +91,11 @@ } // namespace __format +// This function is not user obervable, so it can directly use the non-standard +// types of the "variant". See __arg_t for more details. template -_LIBCPP_HIDE_FROM_ABI _LIBCPP_AVAILABILITY_FORMAT decltype(auto) visit_format_arg(_Visitor&& __vis, - basic_format_arg<_Context> __arg) { +_LIBCPP_HIDE_FROM_ABI _LIBCPP_AVAILABILITY_FORMAT decltype(auto) +__visit_format_arg(_Visitor&& __vis, basic_format_arg<_Context> __arg) { switch (__arg.__type_) { case __format::__arg_t::__none: return _VSTD::invoke(_VSTD::forward<_Visitor>(__vis), __arg.__value_.__monostate_); @@ -265,6 +273,28 @@ typename __basic_format_arg_value<_Context>::__handle& __handle_; }; +// 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 +_LIBCPP_HIDE_FROM_ABI _LIBCPP_AVAILABILITY_FORMAT decltype(auto) +visit_format_arg(_Visitor&& __vis, basic_format_arg<_Context> __arg) { + switch (__arg.__type_) { +# ifndef _LIBCPP_HAS_NO_INT128 + case __format::__arg_t::__i128: { + typename __basic_format_arg_value<_Context>::__handle __h{__arg.__value_.__i128_}; + return _VSTD::invoke(_VSTD::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 _VSTD::invoke(_VSTD::forward<_Visitor>(__vis), typename basic_format_arg<_Context>::handle{__h}); + } +# endif + default: + return _VSTD::__visit_format_arg(_VSTD::forward<_Visitor>(__vis), __arg); + } +} + #endif //_LIBCPP_STD_VER > 17 _LIBCPP_END_NAMESPACE_STD diff --git a/libcxx/include/__format/format_functions.h b/libcxx/include/__format/format_functions.h --- a/libcxx/include/__format/format_functions.h +++ b/libcxx/include/__format/format_functions.h @@ -184,6 +184,7 @@ __format::__compile_time_validate_integral(__ctx.arg(__formatter.__parser_.__precision_)); } +// This function is not user facing, so it can directly use the non-standard types of the "variant". template _LIBCPP_HIDE_FROM_ABI constexpr void __compile_time_visit_format_arg(basic_format_parse_context<_CharT>& __parse_ctx, __compile_time_basic_format_context<_CharT>& __ctx, @@ -263,7 +264,7 @@ else __format::__compile_time_visit_format_arg(__parse_ctx, __ctx, __type); } else - _VSTD::visit_format_arg( + _VSTD::__visit_format_arg( [&](auto __arg) { if constexpr (same_as) __throw_format_error("Argument index out of bounds"); diff --git a/libcxx/include/__format/parser_std_format_spec.h b/libcxx/include/__format/parser_std_format_spec.h --- a/libcxx/include/__format/parser_std_format_spec.h +++ b/libcxx/include/__format/parser_std_format_spec.h @@ -66,7 +66,15 @@ template _LIBCPP_HIDE_FROM_ABI constexpr uint32_t __substitute_arg_id(basic_format_arg<_Context> __format_arg) { - return visit_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 _VSTD::__visit_format_arg( [](auto __arg) -> uint32_t { using _Type = decltype(__arg); if constexpr (integral<_Type>) { diff --git a/libcxx/test/libcxx/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 rename from libcxx/test/libcxx/utilities/format/format.arguments/format.arg/visit_format_arg.pass.cpp rename to libcxx/test/std/utilities/format/format.arguments/format.arg/visit_format_arg.pass.cpp --- a/libcxx/test/libcxx/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 @@ -30,7 +30,7 @@ auto store = std::make_format_args(value); std::basic_format_args format_args{store}; - assert(format_args.__size() == 1); + LIBCPP_ASSERT(format_args.__size() == 1); assert(format_args.get(0)); auto result = std::visit_format_arg( @@ -49,6 +49,21 @@ assert(static_cast(result) == static_cast(value)); } +// Some types, as an extension, are stored in the variant. The Standard +// requires them to be observed as a handle. +template +void test_handle(T value) { + auto store = std::make_format_args(value); + std::basic_format_args format_args{store}; + + LIBCPP_ASSERT(format_args.__size() == 1); + assert(format_args.get(0)); + + std::visit_format_arg( + [](auto a) { assert((std::is_same_v::handle>)); }, + format_args.get(0)); +} + // Test specific for string and string_view. // // Since both result in a string_view there's no need to pass this as a @@ -58,7 +73,7 @@ auto store = std::make_format_args(value); std::basic_format_args format_args{store}; - assert(format_args.__size() == 1); + LIBCPP_ASSERT(format_args.__size() == 1); assert(format_args.get(0)); using CharT = typename Context::char_type; @@ -183,22 +198,8 @@ test(std::numeric_limits::max()); #ifndef TEST_HAS_NO_INT128 - test(std::numeric_limits<__int128_t>::min()); - test(std::numeric_limits::min()); - test(std::numeric_limits::min()); - test(std::numeric_limits::min()); - test(std::numeric_limits::min()); - test( - std::numeric_limits::min()); - test(0); - test( - std::numeric_limits::max()); - test(std::numeric_limits::max()); - test(std::numeric_limits::max()); - test(std::numeric_limits::max()); - test(std::numeric_limits::max()); - test(std::numeric_limits<__int128_t>::max()); -#endif + test_handle(0); +#endif // TEST_HAS_NO_INT128 // Test unsigned integer types. @@ -244,20 +245,8 @@ std::numeric_limits::max()); #ifndef TEST_HAS_NO_INT128 - test(0); - test( - std::numeric_limits::max()); - test( - std::numeric_limits::max()); - test( - std::numeric_limits::max()); - test( - std::numeric_limits::max()); - test( - std::numeric_limits::max()); - test( - std::numeric_limits<__uint128_t>::max()); -#endif + test_handle(0); +#endif // TEST_HAS_NO_INT128 // Test floating point types. diff --git a/libcxx/test/libcxx/utilities/format/format.arguments/format.args/get.pass.cpp b/libcxx/test/std/utilities/format/format.arguments/format.args/get.pass.cpp rename from libcxx/test/libcxx/utilities/format/format.arguments/format.args/get.pass.cpp rename to libcxx/test/std/utilities/format/format.arguments/format.args/get.pass.cpp --- a/libcxx/test/libcxx/utilities/format/format.arguments/format.args/get.pass.cpp +++ b/libcxx/test/std/utilities/format/format.arguments/format.args/get.pass.cpp @@ -37,6 +37,18 @@ format_args.get(0)); } +// Some types, as an extension, are stored in the variant. The Standard +// requires them to be observed as a handle. +template +void test_handle(T value) { + auto store = std::make_format_args(value); + std::basic_format_args format_args{store}; + + std::visit_format_arg( + [](auto a) { assert((std::is_same_v::handle>)); }, + format_args.get(0)); +} + // Test specific for string and string_view. // // Since both result in a string_view there's no need to pass this as a @@ -170,22 +182,8 @@ test(std::numeric_limits::max()); #ifndef TEST_HAS_NO_INT128 - test(std::numeric_limits<__int128_t>::min()); - test(std::numeric_limits::min()); - test(std::numeric_limits::min()); - test(std::numeric_limits::min()); - test(std::numeric_limits::min()); - test( - std::numeric_limits::min()); - test(0); - test( - std::numeric_limits::max()); - test(std::numeric_limits::max()); - test(std::numeric_limits::max()); - test(std::numeric_limits::max()); - test(std::numeric_limits::max()); - test(std::numeric_limits<__int128_t>::max()); -#endif + test_handle(0); +#endif // TEST_HAS_NO_INT128 // Test unsigned integer types. @@ -231,20 +229,8 @@ std::numeric_limits::max()); #ifndef TEST_HAS_NO_INT128 - test(0); - test( - std::numeric_limits::max()); - test( - std::numeric_limits::max()); - test( - std::numeric_limits::max()); - test( - std::numeric_limits::max()); - test( - std::numeric_limits::max()); - test( - std::numeric_limits<__uint128_t>::max()); -#endif + test_handle(0); +#endif // TEST_HAS_NO_INT128 // Test floating point types.