From 3fdd37351f7543369be286bf4a7d1bf876d9a667 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Tue, 19 Mar 2024 01:12:41 +0800 Subject: [PATCH 1/3] Eliminate non-Standard public constructors for... `basic_format_arg::handle` and `basic_format_context` --- stl/inc/format | 59 +++++++++++-------- .../test.cpp | 47 +++++++++++++-- 2 files changed, 74 insertions(+), 32 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index ee03e07a63..e012b72d0d 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -708,9 +708,7 @@ public: private: const void* _Ptr; void(__cdecl* _Format)(basic_format_parse_context<_CharType>& _Parse_ctx, _Context& _Format_ctx, const void*); - friend basic_format_arg; - public: template explicit handle(_Ty& _Val) noexcept : _Ptr(_STD addressof(_Val)), @@ -719,17 +717,7 @@ public: 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>; - if constexpr (_Formattable_with_non_const<_Tq, _Context>) { - static_assert(_Formattable_with<_Tq, _Context>, - "The format() member function can't be called on const formatter. " - "To make the formatter usable, add const to format(). " - "See N4971 [format.arg]/12, [format.formattable], and [formatter.requirements]."); - } else { - static_assert(_Formattable_with<_Tq, _Context>, - "Cannot format an argument. " - "To make this type formattable, provide a formatter specialization. " - "See N4971 [format.arg]/12, [format.formattable], and [formatter.requirements]."); - } + static_assert(_Formattable_with<_Tq, _Context>); typename _Context::template formatter_type<_Td> _Formatter; _Parse_ctx.advance_to(_Formatter.parse(_Parse_ctx)); @@ -737,9 +725,15 @@ public: _Formatter.format(*const_cast<_Tq*>(static_cast(_Ptr)), _Format_ctx)); }) {} + public: void format(basic_format_parse_context<_CharType>& _Parse_ctx, _Context& _Format_ctx) const { _Format(_Parse_ctx, _Format_ctx, _Ptr); } + + template + _NODISCARD static handle _Make_from(_Ty& _Val) noexcept { + return handle{_Val}; + } }; #if defined(__clang__) || defined(__EDG__) // TRANSITION, LLVM-81774 (Clang), VSO-1956558 (EDG) @@ -755,7 +749,7 @@ public: // 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 { + _NODISCARD static basic_format_arg _Make_from(_Ty& _Val) noexcept { using _Erased_type = _Format_arg_traits<_Context>::template _Storage_type<_Ty>; if constexpr (is_same_v, char> && is_same_v<_CharType, wchar_t>) { return basic_format_arg(static_cast<_Erased_type>(static_cast(_Val))); @@ -892,7 +886,7 @@ auto _Format_arg_traits<_Context>::_Type_eraser() { return static_cast(nullptr); } else { int _Dummy{}; - return typename basic_format_arg<_Context>::handle{_Dummy}; + return basic_format_arg<_Context>::handle::_Make_from(_Dummy); } } @@ -2051,6 +2045,8 @@ private: if constexpr (is_same_v>, char> && is_same_v<_CharType, wchar_t>) { _Store_impl<_Erased_type>( _Arg_index, _Arg_type, static_cast<_Erased_type>(static_cast(_Val))); + } else if constexpr (is_same_v<_Erased_type, typename basic_format_arg<_Context>::handle>) { + _Store_impl<_Erased_type>(_Arg_index, _Arg_type, _Erased_type::_Make_from(_Val)); } #if !_HAS_CXX23 // Workaround towards N4950 [format.arg]/6.8 in C++20 @@ -2199,6 +2195,13 @@ private: basic_format_args _Args; _Lazy_locale _Loc; + constexpr basic_format_context(_Out&& _OutputIt_, const basic_format_args& _Ctx_args) + : _OutputIt(_STD move(_OutputIt_)), _Args(_Ctx_args) {} + + constexpr basic_format_context( + _Out&& _OutputIt_, const basic_format_args& _Ctx_args, const _Lazy_locale& _Loc_) + : _OutputIt(_STD move(_OutputIt_)), _Args(_Ctx_args), _Loc(_Loc_) {} + public: using iterator = _Out; using char_type = _CharT; @@ -2206,12 +2209,7 @@ public: template using formatter_type = formatter<_Ty, _CharT>; - constexpr basic_format_context(_Out _OutputIt_, basic_format_args _Ctx_args) - : _OutputIt(_STD move(_OutputIt_)), _Args(_Ctx_args) {} - - constexpr basic_format_context( - _Out _OutputIt_, basic_format_args _Ctx_args, const _Lazy_locale& _Loc_) - : _OutputIt(_STD move(_OutputIt_)), _Args(_Ctx_args), _Loc(_Loc_) {} + basic_format_context() = default; _NODISCARD basic_format_arg arg(size_t _Id) const noexcept { return _Args.get(_Id); @@ -2232,6 +2230,15 @@ public: _NODISCARD _Lazy_locale _Get_lazy_locale() const { return _Loc; } + + _NODISCARD static constexpr basic_format_context _Make_from( + _Out _OutputIt_, basic_format_args _Ctx_args) { + return basic_format_context{_STD move(_OutputIt_), _Ctx_args}; + } + _NODISCARD static constexpr basic_format_context _Make_from( + _Out _OutputIt_, basic_format_args _Ctx_args, const _Lazy_locale& _Loc_) { + return basic_format_context{_STD move(_OutputIt_), _Ctx_args, _Loc_}; + } }; #if _HAS_CXX23 @@ -3540,7 +3547,7 @@ struct _Default_arg_formatter { _OutputIt operator()(basic_format_arg<_Context>::handle _Handle) && { basic_format_parse_context<_CharT> _Parse_ctx({}); - basic_format_context<_OutputIt, _CharT> _Format_ctx(_STD move(_Out), _Args, _Loc); + auto _Format_ctx = _Context::_Make_from(_STD move(_Out), _Args, _Loc); _Handle.format(_Parse_ctx, _Format_ctx); return _Format_ctx.out(); } @@ -3626,11 +3633,11 @@ struct _Format_handler { _Context _Ctx; explicit _Format_handler(_OutputIt _Out, basic_string_view<_CharT> _Str, basic_format_args<_Context> _Format_args) - : _Parse_context(_Str), _Ctx(_STD move(_Out), _Format_args) {} + : _Parse_context(_Str), _Ctx(_Context::_Make_from(_STD move(_Out), _Format_args)) {} explicit _Format_handler(_OutputIt _Out, basic_string_view<_CharT> _Str, basic_format_args<_Context> _Format_args, const _Lazy_locale& _Loc) - : _Parse_context(_Str), _Ctx(_STD move(_Out), _Format_args, _Loc) {} + : _Parse_context(_Str), _Ctx(_Context::_Make_from(_STD move(_Out), _Format_args, _Loc)) {} void _On_text(const _CharT* _First, const _CharT* _Last) { _Ctx.advance_to(_RANGES _Copy_unchecked(_First, _Last, _Ctx.out()).out); @@ -4107,8 +4114,8 @@ protected: } basic_string<_CharT> _Tmp_buf; - basic_format_context>, _CharT> _Tmp_ctx{ - _STD back_inserter(_Tmp_buf), {}, _Fmt_ctx._Get_lazy_locale()}; + auto _Tmp_ctx = basic_format_context>, _CharT>::_Make_from( + _STD back_inserter(_Tmp_buf), {}, _Fmt_ctx._Get_lazy_locale()); _STD _Copy_unchecked(_Opening_bracket._Unchecked_begin(), _Opening_bracket._Unchecked_end(), _Tmp_ctx.out()); [&](index_sequence<_Indices...>) { 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 5966daf359..1a5a95113e 100644 --- a/tests/std/tests/P0645R10_text_formatting_custom_formatting/test.cpp +++ b/tests/std/tests/P0645R10_text_formatting_custom_formatting/test.cpp @@ -233,19 +233,47 @@ void test_mixed_custom_formattable_type() { test_custom_equiv_with_format_mixed(STR("{}{}"), nullptr); } +// Test that handle doesn't have public non-Standard constructors. template void test_basic_format_arg_handle_construction() { using handle = 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); + 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>); +} + +template +constexpr bool is_constructible_with_trailing_empty_brace_impl = requires { T(declval()..., {}); }; + +static_assert(is_constructible_with_trailing_empty_brace_impl); +static_assert(is_constructible_with_trailing_empty_brace_impl, int>); +static_assert(!is_constructible_with_trailing_empty_brace_impl, int, int>); + +// Test that basic_format_context doesn't have public non-Standard constructors. +template +void test_basic_format_context_construction() { + using context = basic_format_context; + + static_assert(is_default_constructible_v == is_default_constructible_v); + static_assert(is_copy_constructible_v == is_copy_constructible_v); + static_assert(is_move_constructible_v); + + static_assert(!is_constructible_v>); + static_assert(!is_constructible_v&>); + + static_assert(is_constructible_with_trailing_empty_brace_impl == is_default_constructible_v); + static_assert(!is_constructible_with_trailing_empty_brace_impl>); + static_assert(!is_constructible_with_trailing_empty_brace_impl&>); } int main() { @@ -262,5 +290,12 @@ int main() { test_basic_format_arg_handle_construction(); test_basic_format_arg_handle_construction(); test_basic_format_arg_handle_construction, wchar_t>(); + + test_basic_format_context_construction(); + test_basic_format_context_construction(); + test_basic_format_context_construction, char>(); + test_basic_format_context_construction(); + test_basic_format_context_construction(); + test_basic_format_context_construction, wchar_t>(); return 0; } From 87a61a1c94a1c1abb03f9e040c7e894cc1c0918b Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Mon, 18 Mar 2024 15:48:05 -0700 Subject: [PATCH 2/3] Use `_STL_INTERNAL_STATIC_ASSERT`. --- stl/inc/format | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/format b/stl/inc/format index e012b72d0d..a69eeee667 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -717,7 +717,7 @@ public: 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>; - static_assert(_Formattable_with<_Tq, _Context>); + _STL_INTERNAL_STATIC_ASSERT(_Formattable_with<_Tq, _Context>); typename _Context::template formatter_type<_Td> _Formatter; _Parse_ctx.advance_to(_Formatter.parse(_Parse_ctx)); From 7fdaf34da401d04180e5d70cb201e48f786b5059 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Mon, 18 Mar 2024 15:51:03 -0700 Subject: [PATCH 3/3] `basic_format_context` shouldn't be default-constructible. --- stl/inc/format | 2 -- .../tests/P0645R10_text_formatting_custom_formatting/test.cpp | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index a69eeee667..9e5d6cdc05 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -2209,8 +2209,6 @@ public: template using formatter_type = formatter<_Ty, _CharT>; - basic_format_context() = default; - _NODISCARD basic_format_arg arg(size_t _Id) const noexcept { return _Args.get(_Id); } 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 1a5a95113e..999ab832d8 100644 --- a/tests/std/tests/P0645R10_text_formatting_custom_formatting/test.cpp +++ b/tests/std/tests/P0645R10_text_formatting_custom_formatting/test.cpp @@ -264,14 +264,14 @@ template void test_basic_format_context_construction() { using context = basic_format_context; - static_assert(is_default_constructible_v == is_default_constructible_v); + static_assert(!is_default_constructible_v); static_assert(is_copy_constructible_v == is_copy_constructible_v); static_assert(is_move_constructible_v); static_assert(!is_constructible_v>); static_assert(!is_constructible_v&>); - static_assert(is_constructible_with_trailing_empty_brace_impl == is_default_constructible_v); + static_assert(!is_constructible_with_trailing_empty_brace_impl); static_assert(!is_constructible_with_trailing_empty_brace_impl>); static_assert(!is_constructible_with_trailing_empty_brace_impl&>); }