<format>: Copy basic_format_args in tuple formatters when needed (#4681)

Co-authored-by: Stephan T. Lavavej <[email protected]>

Author: frederick-vs-ja

stl/inc/format

@@ -2447,11 +2447,17 @@ public:
     }
 };
 
-using _Fmt_it  = back_insert_iterator<_Fmt_buffer<char>>;
-using _Fmt_wit = back_insert_iterator<_Fmt_buffer<wchar_t>>;
+template <class _CharT>
+using _Basic_fmt_it = back_insert_iterator<_Fmt_buffer<_CharT>>;
+
+using _Fmt_it  = _Basic_fmt_it<char>;
+using _Fmt_wit = _Basic_fmt_it<wchar_t>;
+
+template <class _CharT>
+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<char>;
+_EXPORT_STD using wformat_context = _Default_format_context<wchar_t>;
 
 #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 <class _FormatContext, class... _ArgTypes>
+    void _Format_to_context(_FormatContext& _Fmt_ctx, _ArgTypes&... _Args) const {
+        _STD _Copy_unchecked(_Opening_bracket._Unchecked_begin(), _Opening_bracket._Unchecked_end(), _Fmt_ctx.out());
+        [&]<size_t... _Indices>(index_sequence<_Indices...>) {
+            auto _Single_writer = [&]<size_t _Idx>(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<const _Types, _CharT> && ...);
 
@@ -4121,26 +4142,26 @@ protected:
                 _STD _Get_dynamic_specs<_Width_checker>(_Fmt_ctx.arg(static_cast<size_t>(_Specs._Dynamic_width_index)));
         }
 
-        basic_string<_CharT> _Tmp_buf;
-        auto _Tmp_ctx = basic_format_context<back_insert_iterator<basic_string<_CharT>>, _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());
-        [&]<size_t... _Indices>(index_sequence<_Indices...>) {
-            auto _Single_writer = [&]<size_t _Idx>(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<back_insert_iterator<basic_string<_CharT>>, _CharT> _Tmp_buf{
+                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...);
+        } 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});
             });
     }
 

tests/std/tests/P2286R8_text_formatting_tuple/test.cpp

@@ -18,6 +18,7 @@
 #include <concepts>
 #include <cstddef>
 #include <format>
+#include <iterator>
 #include <string>
 #include <string_view>
 #include <tuple>
@@ -435,10 +436,32 @@ auto test_vformat_exception = []<class CharT, class... Args>([[maybe_unused]] st
     }
 };
 
+// Also test that functions taking non-constructible basic_format_context specializations can be well-formed,
+// despite the fact that they can't actually be called.
+
+template <class CharT>
+void test_unconstructible_format_context_for_raw_ptr(basic_format_context<CharT*, CharT>& ctx) { // COMPILE-ONLY
+    formatter<tuple<basic_string<CharT>>, CharT> tuple_formatter;
+    tuple_formatter.format(make_tuple(basic_string<CharT>(STR("42"))), ctx);
+}
+
+template <class CharT>
+void test_unconstructible_format_context_for_back_inserter(
+    basic_format_context<back_insert_iterator<basic_string<CharT>>, CharT>& ctx) { // COMPILE-ONLY
+    formatter<tuple<basic_string<CharT>>, CharT> tuple_formatter;
+    tuple_formatter.format(make_tuple(basic_string<CharT>(STR("42"))), ctx);
+}
+
 int main() {
     run_tests<char>(test_format, test_format_exception);
     run_tests<char>(test_vformat, test_vformat_exception);
 
     run_tests<wchar_t>(test_format, test_format_exception);
     run_tests<wchar_t>(test_vformat, test_vformat_exception);
+
+    (void) &test_unconstructible_format_context_for_raw_ptr<char>;
+    (void) &test_unconstructible_format_context_for_raw_ptr<wchar_t>;
+
+    (void) &test_unconstructible_format_context_for_back_inserter<char>;
+    (void) &test_unconstructible_format_context_for_back_inserter<wchar_t>;
 }

tests/std/tests/P2693R1_text_formatting_thread_id/test.cpp

@@ -5,14 +5,19 @@
 #include <array>
 #include <cassert>
 #include <concepts>
+#include <cstddef>
 #include <execution>
 #include <ios>
 #include <locale>
 #include <ranges>
 #include <sstream>
 #include <string>
+#include <string_view>
 #include <thread>
+#include <tuple>
 #include <type_traits>
+#include <utility>
+#include <variant>
 
 #include "test_format_support.hpp"
 
@@ -254,12 +259,62 @@ void check_invalid_specs() {
     }
 }
 
+// Also test GH-4651 "<format>: Underlying formatters of pair-or-tuple formatter cannot access format args"
+
+template <class CharT>
+using default_format_parse_context = conditional_t<is_same_v<CharT, char>, format_parse_context,
+    conditional_t<is_same_v<CharT, wchar_t>, wformat_parse_context, void>>;
+
+template <size_t I>
+struct substitute_arg {};
+
+template <size_t I, class CharT>
+struct std::formatter<substitute_arg<I>, CharT> {
+    template <class ParseContext>
+    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 <class FormatContext>
+    auto format(substitute_arg<I>, FormatContext& ctx) const {
+        auto visitor = [&]<class T>(T val) -> FormatContext::iterator {
+            if constexpr (same_as<T, monostate>) {
+                return ranges::copy(STR("monostate"sv), ctx.out()).out;
+            } else if constexpr (same_as<T, typename basic_format_arg<FormatContext>::handle>) {
+                default_format_parse_context<CharT> 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 <class CharT>
+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 <class CharT>
 void test() {
     check_formatting_of_default_constructed_thread_id<CharT, FormatFn>();
     check_formatting_of_default_constructed_thread_id<CharT, VFormatFn>();
     check_formatting_of_default_constructed_thread_id<CharT, MoveOnlyFormat>();
 
+    check_substitute_arg_with_tuple_formatters<CharT>();
+
     const array checks = {
         // NB: those functions call 'this_thread::get_id' - let's check various ids
         check_formatting_of_this_thread_id<CharT, FormatFn>,