On Fri, Jul 10, 2026 at 9:25 AM agicy 的 QQ 邮箱 <[email protected]> wrote:
> Thank you for the detailed review. Let me go through the points: > > > On Jul 9, 2026, at 21:38, Tomasz Kaminski <[email protected]> wrote: > > > > On Thu, Jul 9, 2026 at 3:26 PM Tomasz Kaminski <[email protected]> > wrote: > >> >> >> On Thu, Jul 9, 2026 at 1:20 PM Anlai Lu <[email protected]> wrote: >> >>> Add partial specialization of _Iter_sink for ostreambuf_iterator >>> that inherits _Streambuf_sink, replacing per-character sputc with >>> bulk sputn and zero-copy put-area writes. >>> >>> All counting and truncation (_M_max) is handled in this >>> specialization so that _Streambuf_sink stays a pure I/O layer. >>> _M_overflow counts all characters and only writes up to the limit, >>> so format_to_n can compute the total output length. _M_discarding >>> returns false for the same reason. Stack writes go through >>> _M_out._M_put() which tracks failure on the iterator. >>> >>> libstdc++-v3/ChangeLog: >>> >>> * include/std/format >>> (_Iter_sink<ostreambuf_iterator>): New partial specialization. >>> >>> Signed-off-by: Anlai Lu <[email protected]> >>> --- >>> libstdc++-v3/include/std/format | 77 +++++++++++++++++++++++++++++++++ >>> 1 file changed, 77 insertions(+) >>> >>> diff --git a/libstdc++-v3/include/std/format >>> b/libstdc++-v3/include/std/format >>> index cb6cc4592..b14ed9746 100644 >>> --- a/libstdc++-v3/include/std/format >>> +++ b/libstdc++-v3/include/std/format >>> @@ -3843,6 +3843,83 @@ namespace __format >>> } >>> }; >>> >>> + // Specialization replacing per-character sputc with bulk sputn >>> + // and zero-copy writes into the streambuf's put area. >>> + template<typename _CharT, typename _Traits> >>> + class _Iter_sink<_CharT, ostreambuf_iterator<_CharT, _Traits>> >>> + : public _Streambuf_sink<_CharT, _Traits> >>> + { >>> + using _Base = _Streambuf_sink<_CharT, _Traits>; >>> + using typename _Base::_Sink_state; >>> + using _OutIter = ostreambuf_iterator<_CharT, _Traits>; >>> + using __diff_t = iter_difference_t<_OutIter>; >>> + _OutIter _M_out; >>> + __diff_t _M_max; >>> >> Would use size_t here, we can convert it back to iter difference >> of size_t, but will make check simpler (because we will never hit -1 >> characters). >> > Good suggestion. This patch uses iter_difference_t because the generic > _Iter_sink uses iter_difference_t for _M_max (only _Ptr_sink uses size_t > for both), so keeping iter_difference_t here stays consistent within > the family. I wonder if unifying both _Iter_sink to size_t would be > better done as a follow-up? > We need to use iter_difference in the generic case, as the result type may be integer-class-type, i.e. a class that can represent values bigger than size_t. So we need to handle situation when someone calls format_to that would write more than size_t characters for such iterators. However, of ostreambuf_iterator we know that difference type is ptrdiff_t, so size_t will be bigger than any actually representable difference beteween iterators. So in short, _Ptr_sink and this iter could use size_t. But we cannot use it in generic template. So I would change this one. > + >>> + protected: >>> + size_t _M_count = 0; >>> + >>> + _GLIBCXX_CONSTEXPR_FORMAT void >>> + _M_overflow() override >>> + { >>> + auto __s = this->_M_used(); >>> + if (__s.empty()) [[unlikely]] >>> + return; >>> >> I would implement this as follows, comments are for explanation, but does >> not need to >> be in final code. >> size_t __prev = _M_count; >> _M_count += n; >> >> // We already wrote more than max, just clear the buffer. >> if (__prev >= _M_max) >> this->_M_rewind(); >> // We havent reached the maximum, just write to the sink >> else if (_M_count < _M_max) >> _Streambuf_sink::_M_oveflow(); >> // Last write pushed us over maximum, we just need to >> // handle last charcters >> else >> { >> // Limit then number of written characters >> this->_M_reset(__s, _M_max - __prev); >> _Streambuf_sink::_M_oveflow(); // Write this to stream >> > We could use something like _M_flush() here, that will "write" pending > characters (call pbump or write buffer), without reseting the span. > I would extract the following code from _Stream_sink::_M_overflow > (and call it from there). > > A _M_flush would indeed simplify things in principle. > > The reason I kept them separate is that the stack-buffer flush is > different in the two classes: > > - _Streambuf_sink calls sputn() directly (and ignores the return value, > since it has no iterator to track failure on), > - while the _Iter_sink specialization goes through _M_out._M_put(), > which checks sputn's return value and sets _M_failed on the iterator. > Oh, I see know. However, I believe that it would be valuable for the _Streambuf_sink to keep flag if we failed writing to it. Because, in that case we could return true from _M_discarding and stop writting to that sink (I am looking towards model when we could may use _Streambuf_sink to implement print to ostream*). This way the information about the failure need not to be propagate from _M_flush, it could be queried from base. * I need to study standard if that is trully allowed, but it would be great to do that. > > There is a existing _Padding_sink::_M_flush() that returns void. But in > _Streambuf_sink case we would need the caller to somehow know whether > the write succeeded; > I would put the tracking in the _Streambuf_sink due above. > returning the number of characters written and > exposing _M_set_failed() in _Iter_sink could bridge the gap, but that > feels heavier than keeping the overflow logic. > And we can just pass the flag to the iterator constructor. > > _M_reset(_M_buf); // Use buffer to store and write >> remaining characters. >> } >> This way we do not need _M_pbump. >> >> We also need to override _M_reserve to avoid reserving more than _M_max >> characters, >> something like: >> _GLIBCXX_CONSTEXPR_FORMAT typename _Sink<_CharT>::_Reservation >> _M_reserve(size_t __n) override >> { >> if (_M_max - _M_count < __n) >> return { nullptr; } >> return _Streambuf_sink::_M_reserve(__n); >> } >> >> And then _M_bump to count written characters: >> _M_bump(size_t __n) >> { >> _M_count += __n; >> _Streambuf_sink::_M_bump(__n); >> // We written up to _M_max (_M_reserve prevents us from writting >> more). >> if (_M_count >= _M_max) >> _M_reset(_M_buf); >> } >> _ >> + >> >>> + size_t __n = __s.size(); >>> + size_t __commit = 0; >>> + if (_M_max < 0) >>> + __commit = __n; >>> + else if (_M_count < static_cast<size_t>(_M_max)) >>> + { >>> + size_t __max = _M_max - _M_count; >>> + __commit = min(__n, __max); >>> + } >>> + >>> + _M_count += __n; >>> + switch (this->_M_state) >>> + { >>> + case _Sink_state::_S_stack: >>> + // _M_put checks sputn's return value and sets >>> + // _M_failed on the iterator on short write, so >>> + // the caller sees the failure. >>> + _M_out._M_put(__s.data(), __commit); >>> + break; >>> + case _Sink_state::_S_put_area: >>> + this->_M_pbump(__commit); >>> + break; >>> + } >>> + >>> + if (!this->_M_use_put_area()) >>> + this->_M_use_stackbuf(); >>> + } >>> + >>> + _GLIBCXX_CONSTEXPR_FORMAT bool >>> + _M_discarding() const override >>> >> We of course will override _M_discarding to false here again. > + { >>> + return false; >>> + } >>> + >>> + public: >>> + [[__gnu__::__always_inline__]] >>> + _GLIBCXX_CONSTEXPR_FORMAT explicit >>> + _Iter_sink(_OutIter __out, __diff_t __max = -1) >>> + : _Base(__out._M_get_sbuf()), _M_out(__out), _M_max(__max) >>> + { } >>> + >>> + using _Base::out; >>> + >>> + _GLIBCXX_CONSTEXPR_FORMAT format_to_n_result<_OutIter> >>> + _M_finish() && >>> + { >>> + if (this->_M_used().size() != 0) >>> + _M_overflow(); >>> >> We could also call _M_flush here, as we do not need to restore the > buffer. > >> + >>> + iter_difference_t<_OutIter> __count(_M_count); >>> >> We still do not restore the failed flag. >> > For the put-area path, __safe_pbump is just a pointer increment and > never fails. For the stack path, _M_out._M_put() already sets the > iterator's _M_failed internally when sputn returns short: > Ah, I see. My model was to reuse _M_flush, and track failed in _Streambuf_sink. > > ostreambuf_iterator& > _M_put(const _CharT* __ws, streamsize __len) > { > if (__builtin_expect(!_M_failed, true) > && __builtin_expect(this->_M_sbuf->sputn(__ws, __len) != __len, > false)) > _M_failed = true; > return *this; > } > > + return { std::move(_M_out), __count }; >>> + } >>> + }; >>> + >>> // Used for contiguous iterators. >>> // No buffer is used, characters are written straight to the iterator. >>> // We do not know the size of the output range, so the span size just >>> grows >>> -- >>> 2.34.1 >> >> >
