From e17685ed12aa513bb2111602d7a49e1a7a5604db Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Sun, 4 Jun 2023 01:22:55 +0800 Subject: [PATCH 1/3] Implement LWG-3631 Also make formerly exposition only constructors private. Add test coverage for construction of `handle` and inaccessibility of exposition only constructor(s). --- stl/inc/format | 223 ++++++++++-------- .../P0645R10_text_formatting_args/test.cpp | 43 +++- .../test.cpp | 22 ++ 3 files changed, 176 insertions(+), 112 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index 438300f2d7..eeb6c62d1d 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -565,14 +565,6 @@ concept _CharT_or_bool = same_as<_Ty, _CharT> || same_as<_Ty, bool>; template concept _Format_supported_charT = _Is_any_of_v<_CharT, char, wchar_t>; -template -concept _Has_formatter = requires(_Ty& _Val, _Context& _Ctx) { - _STD declval>>().format(_Val, _Ctx); -}; - -template -concept _Has_const_formatter = _Has_formatter, _Context>; - _EXPORT_STD template struct formatter; @@ -656,6 +648,48 @@ private: _EXPORT_STD using format_parse_context = basic_format_parse_context; _EXPORT_STD using wformat_parse_context = basic_format_parse_context; +template >> +concept _Formattable_with = semiregular<_Formatter> + && requires(_Formatter& __f, const _Formatter& __cf, _Ty&& __t, _Context __fc, + basic_format_parse_context __pc) { + { __f.parse(__pc) } -> same_as; + { __cf.format(__t, __fc) } -> same_as; + }; + +template +inline constexpr bool _Is_basic_string_like_for = false; + +template +inline constexpr bool _Is_basic_string_like_for, _CharT> = true; + +template +inline constexpr bool _Is_basic_string_like_for, _CharT> = true; + +template +struct _Format_arg_traits { + using _Char_type = typename _Context::char_type; + + // Function template _Type_eraser mirrors the type dispatching mechanism in the construction of basic_format_arg + // (N4950 [format.arg]). They determine the mapping from "raw" to "erased" argument type for _Format_arg_store. + template <_Formattable_with<_Context> _Ty> + static auto _Type_eraser(); + + template + using _Storage_type = decltype(_Type_eraser>()); + + template + static constexpr size_t _Storage_size = sizeof(_Storage_type<_Ty>); +}; + +_EXPORT_STD template +class basic_format_args; + +_FMT_P2286_BEGIN +template +struct _Format_handler; +_FMT_P2286_END + + _EXPORT_STD template class basic_format_arg { public: @@ -669,18 +703,16 @@ public: public: template - explicit handle(_Ty&& _Val) noexcept + explicit handle(_Ty& _Val) noexcept : _Ptr(_STD addressof(_Val)), _Format([](basic_format_parse_context<_CharType>& _Parse_ctx, _Context& _Format_ctx, const void* _Ptr) { - using _Value_type = remove_cvref_t<_Ty>; - typename _Context::template formatter_type<_Value_type> _Formatter; - using _Qualified_type = - conditional_t<_Has_const_formatter<_Value_type, _Context>, const _Value_type, _Value_type>; + using _Td = remove_const_t<_Ty>; + typename _Context::template formatter_type<_Td> _Formatter; _Parse_ctx.advance_to(_Formatter.parse(_Parse_ctx)); _Format_ctx.advance_to(_Formatter.format( - *const_cast<_Qualified_type*>(static_cast(_Ptr)), _Format_ctx)); + *const_cast<_Qual_for_handle<_Ty>*>(static_cast(_Ptr)), _Format_ctx)); }) { - static_assert(_Has_const_formatter<_Ty, _Context> || !is_const_v>); + static_assert(_Formattable_with<_Qual_for_handle<_Ty>, _Context>); } void format(basic_format_parse_context<_CharType>& _Parse_ctx, _Context& _Format_ctx) const { @@ -691,6 +723,37 @@ public: // TRANSITION, LLVM-49072 basic_format_arg() noexcept : _Active_state(_Basic_format_arg_type::_None), _No_state() {} + explicit operator bool() const noexcept { + return _Active_state != _Basic_format_arg_type::_None; + } + + // Function template _Make_from mirrors the exposition-only single-argument constructor template of + // basic_format_arg (N4950 [format.arg]). + template <_Formattable_with<_Context> _Ty> + static basic_format_arg _Make_from(_Ty& _Val) noexcept { + using _Erased_type = typename _Format_arg_traits<_Context>::template _Storage_type<_Ty>; +#if !_HAS_CXX23 + if constexpr (is_same_v<_Erased_type, basic_string_view<_CharType>>) { + return basic_format_arg(_Erased_type{_Val.data(), _Val.size()}); + } else +#endif // !_HAS_CXX23 + { + return basic_format_arg(static_cast<_Erased_type>(_Val)); + } + } + +private: + template + friend decltype(auto) visit_format_arg(_Visitor&&, basic_format_arg<_Ctx>); + + friend class basic_format_args<_Context>; + friend struct _Format_handler<_CharType>; + friend struct _Format_arg_traits<_Context>; + + // doesn't drop const-qualifier per an unnumbered LWG issue + template + using _Qual_for_handle = conditional_t<_Formattable_with, const _Ty, _Ty>; + explicit basic_format_arg(const int _Val) noexcept : _Active_state(_Basic_format_arg_type::_Int_type), _Int_state(_Val) {} explicit basic_format_arg(const unsigned int _Val) noexcept @@ -718,10 +781,6 @@ public: explicit basic_format_arg(const handle _Val) noexcept : _Active_state(_Basic_format_arg_type::_Custom_type), _Custom_state(_Val) {} - explicit operator bool() const noexcept { - return _Active_state != _Basic_format_arg_type::_None; - } - _Basic_format_arg_type _Active_state = _Basic_format_arg_type::_None; union { monostate _No_state = monostate{}; @@ -741,6 +800,43 @@ public: }; }; +template +template <_Formattable_with<_Context> _Ty> +auto _Format_arg_traits<_Context>::_Type_eraser() { + using _Td = remove_const_t<_Ty>; + // See N4950 [format.arg]/6 + if constexpr (is_same_v<_Td, bool>) { + return bool{}; + } else if constexpr (is_same_v<_Td, _Char_type>) { + return _Char_type{}; + } else if constexpr (is_same_v<_Td, char> && is_same_v<_Char_type, wchar_t>) { + return _Char_type{}; + } else if constexpr (signed_integral<_Td> && sizeof(_Td) <= sizeof(int)) { + return int{}; + } else if constexpr (unsigned_integral<_Td> && sizeof(_Td) <= sizeof(unsigned int)) { + return static_cast(42); + } else if constexpr (signed_integral<_Td> && sizeof(_Td) <= sizeof(long long)) { + return static_cast(42); + } else if constexpr (unsigned_integral<_Td> && sizeof(_Td) <= sizeof(unsigned long long)) { + return static_cast(42); + } else if constexpr (is_same_v<_Td, float>) { + return float{}; + } else if constexpr (is_same_v<_Td, double>) { + return double{}; + } else if constexpr (is_same_v<_Td, long double>) { + return static_cast(42); + } else if constexpr (_Is_basic_string_like_for<_Td, _Char_type>) { + return basic_string_view<_Char_type>{}; + } else if constexpr (_Is_any_of_v, _Char_type*, const _Char_type*>) { + return static_cast(nullptr); + } else if constexpr (is_void_v> || is_same_v<_Td, nullptr_t>) { + return static_cast(nullptr); + } else { + int _Dummy{}; + return typename basic_format_arg<_Context>::handle{_Dummy}; + } +} + _EXPORT_STD template decltype(auto) visit_format_arg(_Visitor&& _Vis, basic_format_arg<_Context> _Arg) { switch (_Arg._Active_state) { @@ -1821,65 +1917,6 @@ public: } }; -template -struct _Format_arg_traits { - using _Char_type = typename _Context::char_type; - - // These overloads mirror the exposition-only single-argument constructor - // set of basic_format_arg (N4928 [format.arg]). They determine the mapping - // from "raw" to "erased" argument type for _Format_arg_store. - template <_Has_formatter<_Context> _Ty> - static auto _Phony_basic_format_arg_constructor(_Ty&&) { - // per the proposed resolution of LWG-3631 - using _Td = remove_cvref_t<_Ty>; - // See N4928 [format.arg]/5 - if constexpr (is_same_v<_Td, bool>) { - return bool{}; - } else if constexpr (is_same_v<_Td, _Char_type>) { - return _Char_type{}; - } else if constexpr (is_same_v<_Td, char> && is_same_v<_Char_type, wchar_t>) { - return _Char_type{}; - } else if constexpr (signed_integral<_Td> && sizeof(_Td) <= sizeof(int)) { - return int{}; - } else if constexpr (unsigned_integral<_Td> && sizeof(_Td) <= sizeof(unsigned int)) { - return static_cast(42); - } else if constexpr (signed_integral<_Td> && sizeof(_Td) <= sizeof(long long)) { - return static_cast(42); - } else if constexpr (unsigned_integral<_Td> && sizeof(_Td) <= sizeof(unsigned long long)) { - return static_cast(42); - } else { - return typename basic_format_arg<_Context>::handle{42}; - } - } - - static auto _Phony_basic_format_arg_constructor(float) -> float; // not defined - static auto _Phony_basic_format_arg_constructor(double) -> double; // not defined - static auto _Phony_basic_format_arg_constructor(long double) -> long double; // not defined - - static auto _Phony_basic_format_arg_constructor(_Char_type*) -> const _Char_type*; // not defined - static auto _Phony_basic_format_arg_constructor(const _Char_type*) -> const _Char_type*; // not defined - - template - static auto _Phony_basic_format_arg_constructor(basic_string_view<_Char_type, _Traits>) - -> basic_string_view<_Char_type>; // not defined - - template - static auto _Phony_basic_format_arg_constructor(basic_string<_Char_type, _Traits, _Alloc>) - -> basic_string_view<_Char_type>; // not defined - - static auto _Phony_basic_format_arg_constructor(nullptr_t) -> const void*; // not defined - - template - requires is_void_v<_Ty> - static auto _Phony_basic_format_arg_constructor(_Ty*) -> const void*; // not defined - - template - using _Storage_type = decltype(_Phony_basic_format_arg_constructor(_STD declval<_Ty&>())); - - template - static constexpr size_t _Storage_size = sizeof(_Storage_type<_Ty>); -}; - struct _Format_arg_index { // TRANSITION, Should be templated on number of arguments for even less storage @@ -1900,9 +1937,6 @@ struct _Format_arg_index { size_t _Type_ : 4 {}; }; -_EXPORT_STD template -class basic_format_args; - template class _Format_arg_store { private: @@ -1974,7 +2008,7 @@ private: } #if !_HAS_CXX23 - // Workaround towards N4928 [format.arg]/9 and /10 in C++20 + // Workaround towards N4950 [format.arg]/6.8 in C++20 if constexpr (is_same_v<_Erased_type, basic_string_view<_CharType>>) { _Store_impl<_Erased_type>(_Arg_index, _Arg_type, _Erased_type{_Val.data(), _Val.size()}); } else @@ -3544,19 +3578,10 @@ struct _Formatter_base { _FormatCtx.arg(static_cast(_Specs._Dynamic_precision_index))); } - using _Erased_type = _Format_arg_traits<_FormatContext>::template _Storage_type<_Ty>; - - _Arg_formatter _Visitor{ - ._Ctx = _STD addressof(_FormatCtx), ._Specs = _STD addressof(_Format_specs)}; -#if !_HAS_CXX23 - if constexpr (is_same_v<_Erased_type, basic_string_view<_CharT>>) { - return _STD visit_format_arg( - _Visitor, basic_format_arg<_FormatContext>{_Erased_type{_Val.data(), _Val.size()}}); - } else -#endif // !_HAS_CXX23 - { - return _STD visit_format_arg(_Visitor, basic_format_arg<_FormatContext>{static_cast<_Erased_type>(_Val)}); - } + return _STD visit_format_arg( + _Arg_formatter{ + ._Ctx = _STD addressof(_FormatCtx), ._Specs = _STD addressof(_Format_specs)}, + basic_format_arg<_FormatContext>::_Make_from(_Val)); } private: @@ -3643,17 +3668,17 @@ _EXPORT_STD using wformat_args = basic_format_args; _EXPORT_STD template _NODISCARD auto make_format_args(_Args&&... _Vals) { - static_assert((_Has_formatter<_Args, _Context> && ...), + static_assert((_Formattable_with, _Context> && ...), "Cannot format an argument. To make type T formattable, provide a formatter specialization. " - "See N4928 [format.arg.store]/2 and [formatter.requirements]."); + "See N4950 [format.arg.store]/2 and [formatter.requirements]."); return _Format_arg_store<_Context, _Args...>{_Vals...}; } _EXPORT_STD template _NODISCARD auto make_wformat_args(_Args&&... _Vals) { - static_assert((_Has_formatter<_Args, wformat_context> && ...), + static_assert((_Formattable_with, wformat_context> && ...), "Cannot format an argument. To make type T formattable, provide a formatter specialization. " - "See N4928 [format.arg.store]/2 and [formatter.requirements]."); + "See N4950 [format.arg.store]/2 and [formatter.requirements]."); return _Format_arg_store{_Vals...}; } diff --git a/tests/std/tests/P0645R10_text_formatting_args/test.cpp b/tests/std/tests/P0645R10_text_formatting_args/test.cpp index d4b3024df3..98802790b3 100644 --- a/tests/std/tests/P0645R10_text_formatting_args/test.cpp +++ b/tests/std/tests/P0645R10_text_formatting_args/test.cpp @@ -90,35 +90,54 @@ void test_basic_format_arg() { basic_format_arg default_constructed; assert(!default_constructed); - basic_format_arg from_int{5}; + // test internal _Make_from mechanism + + constexpr auto as_lvalue = [](T&& t) noexcept -> T& { return static_cast(t); }; + + auto from_int = basic_format_arg::_Make_from(as_lvalue(5)); assert(from_int); - basic_format_arg from_unsigned{5u}; + auto from_unsigned = basic_format_arg::_Make_from(as_lvalue(5u)); assert(from_unsigned); - basic_format_arg from_long_long{5ll}; + auto from_long_long = basic_format_arg::_Make_from(as_lvalue(5ll)); assert(from_long_long); - basic_format_arg from_unsigned_long_long{5ull}; + auto from_unsigned_long_long = basic_format_arg::_Make_from(as_lvalue(5ull)); assert(from_unsigned_long_long); - basic_format_arg from_float{5.0f}; + auto from_float = basic_format_arg::_Make_from(as_lvalue(5.0f)); assert(from_float); - basic_format_arg from_double{5.0}; + auto from_double = basic_format_arg::_Make_from(as_lvalue(5.0)); assert(from_double); - basic_format_arg from_long_double{5.0L}; + auto from_long_double = basic_format_arg::_Make_from(as_lvalue(5.0L)); assert(from_long_double); - basic_format_arg from_pointer{static_cast(nullptr)}; + auto from_nullptr = basic_format_arg::_Make_from(as_lvalue(nullptr)); + assert(from_nullptr); + + auto from_pointer = basic_format_arg::_Make_from(as_lvalue(static_cast(nullptr))); assert(from_pointer); - basic_format_arg from_literal{get_input_literal()}; + auto from_literal = basic_format_arg::_Make_from(as_lvalue(get_input_literal())); assert(from_literal); - basic_format_arg from_string_view{get_input_sv()}; + auto from_string_view = basic_format_arg::_Make_from(as_lvalue(get_input_sv())); assert(from_string_view); + + // the exposition-only constructor of basic_format_arg shouldn't be accessible + static_assert(!is_constructible_v, int>); + static_assert(!is_constructible_v, unsigned int>); + static_assert(!is_constructible_v, long long>); + static_assert(!is_constructible_v, unsigned long long>); + static_assert(!is_constructible_v, float>); + static_assert(!is_constructible_v, double>); + static_assert(!is_constructible_v, long double>); + static_assert(!is_constructible_v, const char_type*>); + static_assert(!is_constructible_v, basic_string_view>); + static_assert(!is_constructible_v, const void*>); } } template @@ -218,9 +237,7 @@ static_assert(is_same_v<_Format_arg_traits::_Storage_type::_Storage_type, const char*>); static_assert(is_same_v<_Format_arg_traits::_Storage_type, const char*>); -// we rely on the _Storage_type to be int in: -// explicit basic_format_arg(const long _Val) noexcept -// : _Active_state(_Basic_format_arg_type::_Int_type), _Int_state(_Val) {} +// we rely on the _Storage_type to be int static_assert(is_same_v<_Format_arg_traits::_Storage_type, int>); static_assert(is_same_v<_Format_arg_traits::_Storage_type, unsigned int>); diff --git a/tests/std/tests/P0645R10_text_formatting_custom_formatting/test.cpp b/tests/std/tests/P0645R10_text_formatting_custom_formatting/test.cpp index c285fcdbaf..e15a100d78 100644 --- a/tests/std/tests/P0645R10_text_formatting_custom_formatting/test.cpp +++ b/tests/std/tests/P0645R10_text_formatting_custom_formatting/test.cpp @@ -233,6 +233,21 @@ void test_mixed_custom_formattable_type() { test_custom_equiv_with_format_mixed(STR("{}{}"), nullptr); } +template +void test_basic_format_arg_handle_construction() { + using handle = typename basic_format_arg>::handle; + + static_assert(is_constructible_v); + static_assert(is_constructible_v); + static_assert(!is_constructible_v); + static_assert(is_constructible_v); + + static_assert(is_constructible_v&>); + static_assert(is_constructible_v&>); + static_assert(!is_constructible_v>); + static_assert(is_constructible_v>); +} + int main() { test_format_family_overloads(); test_format_family_overloads(); @@ -240,5 +255,12 @@ int main() { test_custom_formattable_type(); test_mixed_custom_formattable_type(); test_mixed_custom_formattable_type(); + + test_basic_format_arg_handle_construction(); + test_basic_format_arg_handle_construction(); + test_basic_format_arg_handle_construction, char>(); + test_basic_format_arg_handle_construction(); + test_basic_format_arg_handle_construction(); + test_basic_format_arg_handle_construction, wchar_t>(); return 0; } From 27da05a84d5d90158e8c0410174aa0575c49f4ff Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Tue, 6 Jun 2023 23:45:07 +0800 Subject: [PATCH 2/3] Address @barcharcraz's review comments Inlining `_Qual_for_handle`. --- stl/inc/format | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index eeb6c62d1d..bc7b72003c 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -707,12 +707,16 @@ public: : _Ptr(_STD addressof(_Val)), _Format([](basic_format_parse_context<_CharType>& _Parse_ctx, _Context& _Format_ctx, const void* _Ptr) { using _Td = remove_const_t<_Ty>; + // doesn't drop const-qualifier per an unnumbered LWG issue + using _Tq = conditional_t<_Formattable_with, const _Ty, _Ty>; typename _Context::template formatter_type<_Td> _Formatter; _Parse_ctx.advance_to(_Formatter.parse(_Parse_ctx)); - _Format_ctx.advance_to(_Formatter.format( - *const_cast<_Qual_for_handle<_Ty>*>(static_cast(_Ptr)), _Format_ctx)); + _Format_ctx.advance_to( + _Formatter.format(*const_cast<_Tq*>(static_cast(_Ptr)), _Format_ctx)); }) { - static_assert(_Formattable_with<_Qual_for_handle<_Ty>, _Context>); + // ditto doesn't drop const-qualifier + using _Tq = conditional_t<_Formattable_with, const _Ty, _Ty>; + static_assert(_Formattable_with<_Tq, _Context>); } void format(basic_format_parse_context<_CharType>& _Parse_ctx, _Context& _Format_ctx) const { @@ -750,10 +754,6 @@ private: friend struct _Format_handler<_CharType>; friend struct _Format_arg_traits<_Context>; - // doesn't drop const-qualifier per an unnumbered LWG issue - template - using _Qual_for_handle = conditional_t<_Formattable_with, const _Ty, _Ty>; - explicit basic_format_arg(const int _Val) noexcept : _Active_state(_Basic_format_arg_type::_Int_type), _Int_state(_Val) {} explicit basic_format_arg(const unsigned int _Val) noexcept From 247345e7f47e457517eef2e79e39dc32c314bdfe Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Sat, 10 Jun 2023 10:26:29 -0700 Subject: [PATCH 3/3] Drop newline, use extended friends. --- stl/inc/format | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index bc7b72003c..56f5b11bb6 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -689,7 +689,6 @@ template struct _Format_handler; _FMT_P2286_END - _EXPORT_STD template class basic_format_arg { public: @@ -750,9 +749,9 @@ private: template friend decltype(auto) visit_format_arg(_Visitor&&, basic_format_arg<_Ctx>); - friend class basic_format_args<_Context>; - friend struct _Format_handler<_CharType>; - friend struct _Format_arg_traits<_Context>; + friend basic_format_args<_Context>; + friend _Format_handler<_CharType>; + friend _Format_arg_traits<_Context>; explicit basic_format_arg(const int _Val) noexcept : _Active_state(_Basic_format_arg_type::_Int_type), _Int_state(_Val) {}