On Tue, 6 May 2025 at 12:42, Tomasz Kaminski <tkami...@redhat.com> wrote:
>
>
>
> On Tue, May 6, 2025 at 1:34 PM Jonathan Wakely <jwak...@redhat.com> wrote:
>>
>> On 05/05/25 14:44 +0200, Tomasz Kamiński wrote:
>> >This patch provides an equality operator between _Sink_iter and
>> >default_sentinel,
>> >that returns true, if any further writes to the _Sink_iter and underlying
>> >_Sink,
>> >will be discared, and thus can be omitted. This operator is implemented in
>> >terms
>> >new of _M_discarding virtual function in _Sink.
>> >
>> >Currently only the _Padding_sink reports discarding mode of if width of
>> >sequence
>> >characters is greater than _M_maxwidth (precision), or underlying _Sink is
>> >discarding characters. The _M_discarding override, is separate function from
>> >_M_ignoring, that remain annotated with [[__gnu__::__always_inline__]].
>> >
>> >Despite having notion of maximum characters to be written (_M_max),
>> >_Iter_sink
>> >nevers discard characters, as the total number of characters that would be
>> >written
>> >needs to be returned by format_to_n. This is documented in-source by
>> >providing an
>> >_Iter_sink::_M_discarding override, that always returns false.
>> >
>> >The function is currently queried only by the _Padding_sinks, that may be
>> >stacked
>> >for example a range is formatted, with padding with being specified both
>> >for range
>> >itself and it's elements. The state of underlying sink is checked during
>> >construction
>> >and after each write (_M_sync_discarding).
>> >
>> >libstdc++-v3/ChangeLog:
>> >
>> > * include/std/format (__Sink_iter<_CharT>::operator==)
>> > (__Sink<_CharT>::_M_discarding, _Iter_sink<_CharT,
>> > _OutIter>::_M_discarding)
>> > (_Padding_sinl<_CharT, _Out>::_M_padwidth)
>> > (_Padding_sink<_CharT, _Out>::_M_maxwidth): Remove const.
>> > (_Padding_sink<_CharT, _Out>::_M_sync_discarding)
>> > (_Padding_sink<_CharT, _Out>::_M_discarding): Define.
>> > (_Padding_sink<_CharT, _Out>::_Padding_sink(_Out, size_t, size_tt))
>> > (_Padding_sink<_CharT, _Out>::_M_force_update):
>> > (_Padding_sink<_CharT, _Out>::_M_flush): Call _M_sync_discarding.
>> > (_Padding_sink<_CharT, _Out>::_Padding_sink(_Out, size_t)): Delegate.
>> >---
>> > libstdc++-v3/include/std/format | 66 +++++++++++++++++++++++++++------
>> > 1 file changed, 54 insertions(+), 12 deletions(-)
>> >
>> >diff --git a/libstdc++-v3/include/std/format
>> >b/libstdc++-v3/include/std/format
>> >index 054ce350440..08a28fc92da 100644
>> >--- a/libstdc++-v3/include/std/format
>> >+++ b/libstdc++-v3/include/std/format
>> >@@ -3144,6 +3144,10 @@ namespace __format
>> > auto
>> > _M_reserve(size_t __n) const
>> > { return _M_sink->_M_reserve(__n); }
>> >+
>> >+ friend bool
>> >+ operator==(_Sink_iter __it, default_sentinel_t)
>> >+ { return __it._M_sink->_M_discarding(); }
>>
>> I don't love using default_sentinel like this.
>>
>> Conceptually an output iterator that compares equal to a sentinel but
>> is still writable seems ... odd.
>
> For me this didn't feel odd, just what I would intuitively think of "safe"
> output iterator,
> that discards writes after bounds are reached.
Yes, and this is purely an internal type, not user-facing, so it's
reasonable to rely on the guarantee that writing past the sentinel is
not a bug, just discarded.
>>
>> Did you choose this API just to avoid
>> introducing a member function that isn't part of the usual iterator
>> API?
>
> Yes, but I perceived this as something natural, so I just went for it.
> I can just expose _M_discarding member in the _Sink_iter.
Let's wait for comments from anybody else, maybe others like your
solution more than I do. I don't hate it, I just wonder if it's a bit
too "clever" and so isn't obvious to readers. But maybe others will
disagree.
>>
>>
>> > };
>> >
>> > // Abstract base class for type-erased character sinks.
>> >@@ -3263,6 +3267,11 @@ namespace __format
>> > _M_bump(size_t __n)
>> > { _M_next += __n; }
>> >
>> >+ // Returns true if the _Sink is discarding incoming characters.
>> >+ virtual bool
>> >+ _M_discarding() const
>> >+ { return false; }
>> >+
>> > public:
>> > _Sink(const _Sink&) = delete;
>> > _Sink& operator=(const _Sink&) = delete;
>> >@@ -3488,6 +3497,15 @@ namespace __format
>> > _M_count += __s.size();
>> > }
>> >
>> >+ bool
>> >+ _M_discarding() const override
>> >+ {
>> >+ // format_to_n return total number of characters, that would be
>> >written,
>> >+ // thus we need to see and all characters.
>>
>> "see and all characters" seems wrong, does it mean "see and format all
>> characters"? Or somethign else?
>>
>> >+ // see https://eel.is/c%2B%2Bdraft/format#functions-22
>>
>> These links aren't necessarily permanent, so I would prefer a "dated"
>> reference to a particular standard (or working paper), e.g.
>> // see C++20 [format.functions] p20
>>
>> N.B. it was p20 in C++20, but p22 in C++23, so if you prefr to link to
>> the newer standard, then:
>>
>> // see C++23 [format.functions] p22
>>
>> >+ return false;
>> >+ }
>> >+
>> > public:
>> > [[__gnu__::__always_inline__]]
>> > explicit
>> >@@ -3550,6 +3568,15 @@ namespace __format
>> > }
>> > }
>> >
>> >+ bool
>> >+ _M_discarding() const override
>> >+ {
>> >+ // format_to_n return total number of characters, that would be
>> >written,
>> >+ // thus we need to see and all characters.
>> >+ // see https://eel.is/c%2B%2Bdraft/format#functions-22
>>
>> Same comment about "see and all" and the standard reference.
>>
>> >+ return false;
>> >+ }
>> >+
>> > typename _Sink<_CharT>::_Reservation
>> > _M_reserve(size_t __n) final
>> > {
>> >@@ -3636,17 +3663,15 @@ namespace __format
>> > template<typename _Out, typename _CharT>
>> > class _Padding_sink : public _Str_sink<_CharT>
>> > {
>> >- const size_t _M_padwidth;
>> >- const size_t _M_maxwidth;
>> >+ size_t _M_padwidth;
>> >+ size_t _M_maxwidth;
>> > _Out _M_out;
>> > size_t _M_printwidth;
>> >
>> > [[__gnu__::__always_inline__]]
>> > bool
>> > _M_ignoring() const
>> >- {
>> >- return _M_printwidth >= _M_maxwidth;
>> >- }
>> >+ { return _M_printwidth >= _M_maxwidth; }
>> >
>> > [[__gnu__::__always_inline__]]
>> > bool
>> >@@ -3659,12 +3684,21 @@ namespace __format
>> > return false;
>> > }
>> >
>> >+ void
>> >+ _M_sync_discarding()
>> >+ {
>> >+ if constexpr (is_same_v<_Out, _Sink_iter<_CharT>>)
>> >+ if (_M_out == default_sentinel)
>> >+ _M_maxwidth = _M_printwidth;
>> >+ }
>> >+
>> > void
>> > _M_flush()
>> > {
>> > span<_CharT> __new = this->_M_used();
>> > basic_string_view<_CharT> __str(__new.data(), __new.size());
>> > _M_out = __format::__write(std::move(_M_out), __str);
>> >+ _M_sync_discarding();
>> > this->_M_rewind();
>> > }
>> >
>> >@@ -3682,7 +3716,10 @@ namespace __format
>> > // We have more characters than padidng, no padding is needed,
>> > // write direclty to _M_out.
>> > if (_M_printwidth >= _M_padwidth)
>> >- _M_out = __format::__write(std::move(_M_out), __str);
>> >+ {
>> >+ _M_out = __format::__write(std::move(_M_out), __str);
>> >+ _M_sync_discarding();
>> >+ }
>> > // We reached _M_maxwidth that is smaller than _M_padwidth.
>> > // Store the prefix sequence in _M_seq, and free _M_buf.
>> > else
>> >@@ -3718,6 +3755,10 @@ namespace __format
>> > _Str_sink<_CharT>::_M_overflow();
>> > }
>> >
>> >+ bool
>> >+ _M_discarding() const override
>> >+ { return _M_ignoring(); }
>> >+
>> > typename _Sink<_CharT>::_Reservation
>> > _M_reserve(size_t __n) override
>> > {
>> >@@ -3752,15 +3793,16 @@ namespace __format
>> >
>> > public:
>> > [[__gnu__::__always_inline__]]
>> >- explicit _Padding_sink(_Out __out, size_t __padwidth)
>> >- : _M_padwidth(__padwidth), _M_maxwidth(-1),
>> >+ explicit
>> >+ _Padding_sink(_Out __out, size_t __padwidth, size_t __maxwidth)
>> >+ : _M_padwidth(__padwidth), _M_maxwidth(__maxwidth),
>> > _M_out(std::move(__out)), _M_printwidth(0)
>> >- { }
>> >+ { _M_sync_discarding(); }
>> >
>> > [[__gnu__::__always_inline__]]
>> >- explicit _Padding_sink(_Out __out, size_t __padwidth, size_t
>> >__maxwidth)
>> >- : _M_padwidth(__padwidth), _M_maxwidth(__maxwidth),
>> >- _M_out(std::move(__out)), _M_printwidth(0)
>> >+ explicit
>> >+ _Padding_sink(_Out __out, size_t __padwidth)
>> >+ : _Padding_sink(std::move(__out), __padwidth, (size_t)-1)
>> > { }
>> >
>> > _Out
>> >--
>> >2.49.0
>> >
>> >
>>
