From ba22538206f8aea106d8048e18d139f1871badf1 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Wed, 22 May 2024 00:19:03 +0800 Subject: [PATCH 1/2] Copy format args in `tuple` formatters when needed --- stl/inc/format | 61 +++++++++++++------ .../P2286R8_text_formatting_tuple/test.cpp | 22 +++++++ .../test.cpp | 52 ++++++++++++++++ 3 files changed, 115 insertions(+), 20 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index a30c26c3158..0327ec05865 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -2447,11 +2447,17 @@ public: } }; -using _Fmt_it = back_insert_iterator<_Fmt_buffer>; -using _Fmt_wit = back_insert_iterator<_Fmt_buffer>; +template +using _Basic_fmt_it = back_insert_iterator<_Fmt_buffer<_CharT>>; + +using _Fmt_it = _Basic_fmt_it; +using _Fmt_wit = _Basic_fmt_it; + +template +using _Default_format_context = basic_format_context<_Basic_fmt_it<_CharT>, _CharT>; -_EXPORT_STD using format_context = basic_format_context<_Fmt_it, char>; -_EXPORT_STD using wformat_context = basic_format_context<_Fmt_wit, wchar_t>; +_EXPORT_STD using format_context = _Default_format_context; +_EXPORT_STD using wformat_context = _Default_format_context; #if _HAS_CXX23 _EXPORT_STD enum class range_format { disabled, map, set, sequence, string, debug_string }; @@ -4107,6 +4113,21 @@ private: _Fill_align_and_width_specs<_CharT> _Specs; + template + void _Format_to_context(_FormatContext& _Fmt_ctx, _ArgTypes&... _Args) const { + _STD _Copy_unchecked(_Opening_bracket._Unchecked_begin(), _Opening_bracket._Unchecked_end(), _Fmt_ctx.out()); + [&](index_sequence<_Indices...>) { + auto _Single_writer = [&](auto& _Arg) { + if constexpr (_Idx != 0) { + _STD _Copy_unchecked(_Separator._Unchecked_begin(), _Separator._Unchecked_end(), _Fmt_ctx.out()); + } + _STD get<_Idx>(_Underlying).format(_Arg, _Fmt_ctx); + }; + (_Single_writer.template operator()<_Indices>(_Args), ...); + }(index_sequence_for<_ArgTypes...>{}); + _STD _Copy_unchecked(_Closing_bracket._Unchecked_begin(), _Closing_bracket._Unchecked_end(), _Fmt_ctx.out()); + } + protected: static constexpr bool _Is_const_formattable = (formattable && ...); @@ -4121,26 +4142,26 @@ protected: _STD _Get_dynamic_specs<_Width_checker>(_Fmt_ctx.arg(static_cast(_Specs._Dynamic_width_index))); } - basic_string<_CharT> _Tmp_buf; - auto _Tmp_ctx = basic_format_context>, _CharT>::_Make_from( - _STD back_inserter(_Tmp_buf), {}, _Fmt_ctx._Get_lazy_locale()); + if (_Format_specs._Width <= 0) { + _Format_to_context(_Fmt_ctx, _Args...); + return _Fmt_ctx.out(); + } - _STD _Copy_unchecked(_Opening_bracket._Unchecked_begin(), _Opening_bracket._Unchecked_end(), _Tmp_ctx.out()); - [&](index_sequence<_Indices...>) { - auto _Single_writer = [&](auto& _Arg) { - if constexpr (_Idx != 0) { - _STD _Copy_unchecked(_Separator._Unchecked_begin(), _Separator._Unchecked_end(), _Tmp_ctx.out()); - } - _STD get<_Idx>(_Underlying).format(_Arg, _Tmp_ctx); - }; - (_Single_writer.template operator()<_Indices>(_Args), ...); - }(index_sequence_for<_ArgTypes...>{}); - _STD _Copy_unchecked(_Closing_bracket._Unchecked_begin(), _Closing_bracket._Unchecked_end(), _Tmp_ctx.out()); + basic_string<_CharT> _Tmp_str; + if constexpr (is_same_v<_FormatContext, _Default_format_context<_CharT>>) { + _Fmt_iterator_buffer>, _CharT> _Tmp_buf{ + _STD back_inserter(_Tmp_str)}; + auto _Tmp_ctx = _FormatContext::_Make_from( + _Basic_fmt_it<_CharT>{_Tmp_buf}, _Fmt_ctx._Get_args(), _Fmt_ctx._Get_lazy_locale()); + _Format_to_context(_Tmp_ctx, _Args...); + } else { + _CSTD abort(); // no basic_format_context object other than _Default_format_context can be created + } - const int _Width = _Measure_display_width<_CharT>(_Tmp_buf); + const int _Width = _Measure_display_width<_CharT>(_Tmp_str); return _STD _Write_aligned( _Fmt_ctx.out(), _Width, _Format_specs, _Fmt_align::_Left, [&](typename _FormatContext::iterator _Out) { - return _STD _Fmt_write(_STD move(_Out), basic_string_view<_CharT>{_Tmp_buf}); + return _STD _Fmt_write(_STD move(_Out), basic_string_view<_CharT>{_Tmp_str}); }); } diff --git a/tests/std/tests/P2286R8_text_formatting_tuple/test.cpp b/tests/std/tests/P2286R8_text_formatting_tuple/test.cpp index f00365ea87e..71ceed9b807 100644 --- a/tests/std/tests/P2286R8_text_formatting_tuple/test.cpp +++ b/tests/std/tests/P2286R8_text_formatting_tuple/test.cpp @@ -435,10 +435,32 @@ auto test_vformat_exception = []([[maybe_unused]] st } }; +// Also test that functions taking non-constructible basic_format_context specializations can be well-formed, +// despite that they can't be actually called. + +template +void test_unconstructible_format_context_for_raw_ptr(basic_format_context& ctx) { // COMPILE-ONLY + formatter>, CharT> formatter; + formatter.format(make_tuple(basic_string(STR("42"))), ctx); +} + +template +void test_unconstructible_format_context_for_back_inserter( + basic_format_context>, CharT>& ctx) { // COMPILE-ONLY + formatter>, CharT> formatter; + formatter.format(make_tuple(basic_string(STR("42"))), ctx); +} + int main() { run_tests(test_format, test_format_exception); run_tests(test_vformat, test_vformat_exception); run_tests(test_format, test_format_exception); run_tests(test_vformat, test_vformat_exception); + + (void) &test_unconstructible_format_context_for_raw_ptr; + (void) &test_unconstructible_format_context_for_raw_ptr; + + (void) &test_unconstructible_format_context_for_back_inserter; + (void) &test_unconstructible_format_context_for_back_inserter; } diff --git a/tests/std/tests/P2693R1_text_formatting_thread_id/test.cpp b/tests/std/tests/P2693R1_text_formatting_thread_id/test.cpp index d140576e814..7d5c929cfcd 100644 --- a/tests/std/tests/P2693R1_text_formatting_thread_id/test.cpp +++ b/tests/std/tests/P2693R1_text_formatting_thread_id/test.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -13,6 +14,7 @@ #include #include #include +#include #include "test_format_support.hpp" @@ -254,12 +256,62 @@ void check_invalid_specs() { } } +// Also test GH-4651 ": Underlying formatters of pair-or-tuple formatter cannot access format args" + +template +using default_format_parse_context = conditional_t, format_parse_context, + conditional_t, wformat_parse_context, void>>; + +template +struct substitute_arg {}; + +template +struct std::formatter, CharT> { + template + constexpr auto parse(ParseContext& ctx) { + auto it = ctx.begin(); + if (it != ctx.end() && *it != '}') { + throw format_error{"Expected empty spec"}; + } + + ctx.check_arg_id(I); + return it; + } + + template + auto format(substitute_arg, FormatContext& ctx) const { + auto visitor = [&](T val) -> FormatContext::iterator { + if constexpr (same_as) { + return ranges::copy(STR("monostate"sv), ctx.out()).out; + } else if constexpr (same_as::handle>) { + default_format_parse_context parse_ctx{STR("")}; + val.format(parse_ctx, ctx); + return ctx.out(); + } else { + return format_to(ctx.out(), STR("{}"), val); + } + }; + + return visit_format_arg(visitor, ctx.arg(I)); + } +}; + +template +void check_substitute_arg_with_tuple_formatters() { + assert(format(STR("{0:}"), tuple{substitute_arg<1>{}, substitute_arg<2>{}}, STR("thread::id"), thread::id{}) + == STR("(thread::id, 0)")); + assert(format(STR("{0:}"), pair{substitute_arg<1>{}, substitute_arg<2>{}}, STR("thread::id"), thread::id{}) + == STR("(thread::id, 0)")); +} + template void test() { check_formatting_of_default_constructed_thread_id(); check_formatting_of_default_constructed_thread_id(); check_formatting_of_default_constructed_thread_id(); + check_substitute_arg_with_tuple_formatters(); + const array checks = { // NB: those functions call 'this_thread::get_id' - let's check various ids check_formatting_of_this_thread_id, From dbf36f58d843053fa6d08552c53da81aa33cc41b Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Wed, 29 May 2024 03:29:49 -0700 Subject: [PATCH 2/2] Code review nitpicks. --- stl/inc/format | 2 +- .../std/tests/P2286R8_text_formatting_tuple/test.cpp | 11 ++++++----- .../tests/P2693R1_text_formatting_thread_id/test.cpp | 3 +++ 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index 0327ec05865..62e206c70e2 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -4150,7 +4150,7 @@ protected: basic_string<_CharT> _Tmp_str; if constexpr (is_same_v<_FormatContext, _Default_format_context<_CharT>>) { _Fmt_iterator_buffer>, _CharT> _Tmp_buf{ - _STD back_inserter(_Tmp_str)}; + back_insert_iterator{_Tmp_str}}; auto _Tmp_ctx = _FormatContext::_Make_from( _Basic_fmt_it<_CharT>{_Tmp_buf}, _Fmt_ctx._Get_args(), _Fmt_ctx._Get_lazy_locale()); _Format_to_context(_Tmp_ctx, _Args...); diff --git a/tests/std/tests/P2286R8_text_formatting_tuple/test.cpp b/tests/std/tests/P2286R8_text_formatting_tuple/test.cpp index 71ceed9b807..40e2c366df9 100644 --- a/tests/std/tests/P2286R8_text_formatting_tuple/test.cpp +++ b/tests/std/tests/P2286R8_text_formatting_tuple/test.cpp @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -436,19 +437,19 @@ auto test_vformat_exception = []([[maybe_unused]] st }; // Also test that functions taking non-constructible basic_format_context specializations can be well-formed, -// despite that they can't be actually called. +// despite the fact that they can't actually be called. template void test_unconstructible_format_context_for_raw_ptr(basic_format_context& ctx) { // COMPILE-ONLY - formatter>, CharT> formatter; - formatter.format(make_tuple(basic_string(STR("42"))), ctx); + formatter>, CharT> tuple_formatter; + tuple_formatter.format(make_tuple(basic_string(STR("42"))), ctx); } template void test_unconstructible_format_context_for_back_inserter( basic_format_context>, CharT>& ctx) { // COMPILE-ONLY - formatter>, CharT> formatter; - formatter.format(make_tuple(basic_string(STR("42"))), ctx); + formatter>, CharT> tuple_formatter; + tuple_formatter.format(make_tuple(basic_string(STR("42"))), ctx); } int main() { diff --git a/tests/std/tests/P2693R1_text_formatting_thread_id/test.cpp b/tests/std/tests/P2693R1_text_formatting_thread_id/test.cpp index 7d5c929cfcd..4e6fd42c555 100644 --- a/tests/std/tests/P2693R1_text_formatting_thread_id/test.cpp +++ b/tests/std/tests/P2693R1_text_formatting_thread_id/test.cpp @@ -12,8 +12,11 @@ #include #include #include +#include #include +#include #include +#include #include #include "test_format_support.hpp"