Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 41 additions & 20 deletions stl/inc/format
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
Expand Down Expand Up @@ -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> && ...);

Expand All @@ -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{
_STD back_inserter(_Tmp_str)};
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
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});
});
}

Expand Down
22 changes: 22 additions & 0 deletions tests/std/tests/P2286R8_text_formatting_tuple/test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -435,10 +435,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 that they can't be actually called.
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved

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> formatter;
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
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
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
formatter<tuple<basic_string<CharT>>, CharT> formatter;
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>;
}
52 changes: 52 additions & 0 deletions tests/std/tests/P2693R1_text_formatting_thread_id/test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include <array>
#include <cassert>
#include <concepts>
#include <cstddef>
#include <execution>
#include <ios>
#include <locale>
Expand All @@ -13,6 +14,7 @@
#include <string>
#include <thread>
#include <type_traits>
#include <variant>

#include "test_format_support.hpp"

Expand Down Expand Up @@ -254,12 +256,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;
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
} 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)"));
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
}

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>,
Expand Down