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.


> 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.

>
> >     };
> >
> >   // 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
> >
> >
>
>

Reply via email to