On Thu, 4 Sept 2025 at 13:52, Tomasz Kamiński <[email protected]> wrote:
>
> When producing output, the libstdc++ format implementation only uses
> _Sink_iter
> specializations. Since users cannot construct basic_format_context, this is
> the
> only iterator type actually used. The __format_padded helper relies on this
> property to efficiently pad sequences from tuples and ranges.
>
> However, the standard's formattable concept requires a generic format function
> in formatters that works with an any iterator type. This is intended to
"with any iterator type"
> future-proof the implementation by allowing new format_context types.
> Previously,
> libstdc++ used back_insert_iterator<basic_string<_CharT>> for this purpose.
>
> Normally, concept checks only instantiate function signatures, but with
> user-defined formatters and deduced return types, the function body and all
> called functions are instantiated. This could trigger a static assertion error
> in the range/tuple formatter that assumed the iterator was a _Sink_iter
> (see included test).
>
> This patch resolves the issue by replacing the _Iter_for_t alias with the
> internal _Drop_iter. This iterator sematnics is to drop elements, so
"iterator's semantics"
> __format_padded can handle it by simply returning the input iterator, which
> still produces the required behavior [1].
>
> An alternative of using _Sink_iter was considered but rejected because it
> would
> allow formatters to pass formattable requirements while only supporting
> format_context and wformat_context, which seems counter to the design intent
> (the std/format/formatter/concept.cc fails).
>
> [1] The standard's wording defines format functions as producing an output
> representation, but does not explicitly require a formatter to be invoked
> for each element. This allows the use of _Drop_iter to pass the concept check
> without generating any output.
>
> PR libstdc++/121765
>
> libstdc++-v3/ChangeLog:
>
> * include/std/format (__format::_Drop_iter): Define.
> (_Iter_for_t::type): Change alias to _Drop_iter.
> (__format::__format_padded): Return __fc.out() for
> _Drop_iter.
> * testsuite/std/format/pr121765.cc: New test.
> ---
> The description of this patch is longer that then change, but I think
> to record why I have choosen particular direction.
Yes, the detailed commit msg is useful, thanks.
>
> Tested on x86_64-linux locally. OK for trunk?
OK with the typo fixes above, and using _Drop_iter in the
__format_padded if-constexpr check.
>
> libstdc++-v3/include/std/format | 120 ++++++++++++------
> libstdc++-v3/testsuite/std/format/pr121765.cc | 53 ++++++++
> 2 files changed, 136 insertions(+), 37 deletions(-)
> create mode 100644 libstdc++-v3/testsuite/std/format/pr121765.cc
>
> diff --git a/libstdc++-v3/include/std/format b/libstdc++-v3/include/std/format
> index d63c6fc9efd..e6377aaa2ed 100644
> --- a/libstdc++-v3/include/std/format
> +++ b/libstdc++-v3/include/std/format
> @@ -56,7 +56,7 @@
> #include <bits/ranges_base.h> // input_range, range_reference_t
> #include <bits/ranges_util.h> // subrange
> #include <bits/ranges_algobase.h> // ranges::copy
> -#include <bits/stl_iterator.h> // back_insert_iterator, counted_iterator
> +#include <bits/stl_iterator.h> // counted_iterator
> #include <bits/stl_pair.h> // __is_pair
> #include <bits/unicode.h> // __is_scalar_value, _Utf_view, etc.
> #include <bits/utility.h> // tuple_size_v
> @@ -110,10 +110,14 @@ namespace __format
> template<typename _CharT>
> class _Sink_iter;
>
> + // Output iterator that ignores the characters
> + template<typename _CharT>
> + class _Drop_iter;
> +
> // An unspecified output iterator type used in the `formattable` concept.
> template<typename _CharT>
> struct _Iter_for
> - { using type = back_insert_iterator<basic_string<_CharT>>; };
> + { using type = _Drop_iter<_CharT>; };
>
> template<typename _CharT>
> using __format_context = basic_format_context<_Sink_iter<_CharT>,
> _CharT>;
> @@ -3135,6 +3139,43 @@ _GLIBCXX_END_NAMESPACE_CONTAINER
> /// @cond undocumented
> namespace __format
> {
> + template<typename _CharT>
> + class _Drop_iter
> + {
> + public:
> + using iterator_category = output_iterator_tag;
> + using value_type = void;
> + using difference_type = ptrdiff_t;
> + using pointer = void;
> + using reference = void;
> +
> + _Drop_iter() = default;
> + _Drop_iter(const _Drop_iter&) = default;
> + _Drop_iter& operator=(const _Drop_iter&) = default;
> +
> + [[__gnu__::__always_inline__]]
> + constexpr _Drop_iter&
> + operator=(_CharT __c)
> + { return *this; }
> +
> + [[__gnu__::__always_inline__]]
> + constexpr _Drop_iter&
> + operator=(basic_string_view<_CharT> __s)
> + { return *this; }
> +
> + [[__gnu__::__always_inline__]]
> + constexpr _Drop_iter&
> + operator*() { return *this; }
> +
> + [[__gnu__::__always_inline__]]
> + constexpr _Drop_iter&
> + operator++() { return *this; }
> +
> + [[__gnu__::__always_inline__]]
> + constexpr _Drop_iter
> + operator++(int) { return *this; }
> + };
> +
> template<typename _CharT>
> class _Sink_iter
> {
> @@ -5503,42 +5544,47 @@ namespace __format
> const _Spec<_CharT>& __spec,
> _Callback&& __call)
> {
> - // This is required to implement formatting with padding,
> - // as we need to format to temporary buffer, using the same iterator.
> - static_assert(is_same_v<_Out, __format::_Sink_iter<_CharT>>);
> -
> - const size_t __padwidth = __spec._M_get_width(__fc);
> - if (__padwidth == 0)
> - return __call(__fc);
> -
> - struct _Restore_out
> - {
> - _Restore_out(basic_format_context<_Sink_iter<_CharT>, _CharT>& __fc)
> - : _M_ctx(std::addressof(__fc)), _M_out(__fc.out())
> - { }
> -
> - void
> - _M_disarm()
> - { _M_ctx = nullptr; }
> -
> - ~_Restore_out()
> - {
> - if (_M_ctx)
> - _M_ctx->advance_to(_M_out);
> - }
> -
> - private:
> - basic_format_context<_Sink_iter<_CharT>, _CharT>* _M_ctx;
> - _Sink_iter<_CharT> _M_out;
> - };
> + if constexpr (is_same_v<_Out, _Iter_for_t<_CharT>>)
> + return __fc.out();
> + else
> + {
> + // This is required to implement formatting with padding,
> + // as we need to format to temporary buffer, using the same
> iterator.
> + static_assert(is_same_v<_Out, _Sink_iter<_CharT>>);
> +
> + const size_t __padwidth = __spec._M_get_width(__fc);
> + if (__padwidth == 0)
> + return __call(__fc);
> +
> + struct _Restore_out
> + {
> + _Restore_out(basic_format_context<_Sink_iter<_CharT>, _CharT>&
> __fc)
> + : _M_ctx(std::addressof(__fc)), _M_out(__fc.out())
> + { }
> +
> + void
> + _M_disarm()
> + { _M_ctx = nullptr; }
> +
> + ~_Restore_out()
> + {
> + if (_M_ctx)
> + _M_ctx->advance_to(_M_out);
> + }
>
> - _Restore_out __restore(__fc);
> - _Padding_sink<_Sink_iter<_CharT>, _CharT> __sink(__fc.out(),
> __padwidth);
> - __fc.advance_to(__sink.out());
> - __call(__fc);
> - __fc.advance_to(__sink._M_finish(__spec._M_align, __spec._M_fill));
> - __restore._M_disarm();
> - return __fc.out();
> + private:
> + basic_format_context<_Sink_iter<_CharT>, _CharT>* _M_ctx;
> + _Sink_iter<_CharT> _M_out;
> + };
> +
> + _Restore_out __restore(__fc);
> + _Padding_sink<_Sink_iter<_CharT>, _CharT> __sink(__fc.out(),
> __padwidth);
> + __fc.advance_to(__sink.out());
> + __call(__fc);
> + __fc.advance_to(__sink._M_finish(__spec._M_align, __spec._M_fill));
> + __restore._M_disarm();
> + return __fc.out();
> + }
> }
>
> template<size_t _Pos, typename _Tp, typename _CharT>
> diff --git a/libstdc++-v3/testsuite/std/format/pr121765.cc
> b/libstdc++-v3/testsuite/std/format/pr121765.cc
> new file mode 100644
> index 00000000000..1358fc11052
> --- /dev/null
> +++ b/libstdc++-v3/testsuite/std/format/pr121765.cc
> @@ -0,0 +1,53 @@
> +// { dg-do compile { target c++23 } }
> +
> +#include <format>
> +#include <utility>
> +
> +struct MyPair
> +{
> + int x;
> + int y;
> +};
> +
> +template<typename CharT>
> +struct std::formatter<MyPair, CharT>
> +{
> + template<typename ParseContext>
> + auto parse(ParseContext& pc)
> + { return _formatter.parse(pc); }
> +
> + template<typename FormatContext>
> + auto format(const MyPair& mp, FormatContext& fc) const
> + { return _formatter.format(std::make_pair(mp.x, mp.y), fc); }
> +
> +private:
> + std::formatter<std::pair<int, int>, CharT> _formatter;
> +};
> +
> +static_assert(std::formattable<MyPair, char>);
> +static_assert(std::formattable<MyPair, wchar_t>);
> +
> +struct MyRange
> +{
> + int* begin;
> + int* end;
> +};
> +
> +template<typename CharT>
> +struct std::formatter<MyRange, CharT>
> +{
> + template<typename ParseContext>
> + auto parse(ParseContext& pc)
> + { return _formatter.parse(pc); }
> +
> + template<typename FormatContext>
> + auto format(const MyRange& mp, FormatContext& fc) const
> + { return _formatter.format(std::span<int>(mp.begin, mp.end), fc); }
> +
> +private:
> + std::formatter<std::span<int>, CharT> _formatter;
> +};
> +
> +static_assert(std::formattable<MyRange, char>);
> +static_assert(std::formattable<MyRange, wchar_t>);
> +
> --
> 2.51.0
>
