[committed] libstdc++: Fix std::format_to_n return value [PR110990]

[committed] libstdc++: Fix std::format_to_n return value [PR110990]

[Jonathan Wakely]

Tested x86_64-linux. Pushed to trunk. Backport to gcc-13 also needed.

-- >8 --

When writing to a contiguous iterator, std::format_to_n(out, n, ...)
always returns out + n, even if it wrote fewer than n characters to the
iterator.

The problem is in the _M_finish() member function of the _Iter_sink
specialization for contiguous iterators. _M_finish() calls _M_overflow()
to update its count of characters written, so it can return the count of
characters that would be written if there was room. But _M_overflow()
assumes it's only called when the buffer is full, and so switches to the
internal buffer. _M_finish() then thinks that if the internal buffer is
in use, we already wrote at least n characters and so returns out+n as
the output position.

We can fix the problem by adding a check in _M_overflow() so that we
don't update the count and switch to the internal buffer unless we've
run out of room, i.e. _M_unused().size() is zero. The caller then needs
to be prepared for _M_count not being the final total, and so add
_M_used.size() to it.

However, there's not actually any need for _M_finish() to call
_M_overflow() to get the count. We now need to use _M_count and
_M_used.size() to get the total anyway so _M_overflow() doesn't help
with that. And we don't need to use _M_overflow() to flush unwritten
characters to the output, because the specialization for contiguous
iterators always writes directly to the output without buffering (except
when we've exceeded the maximum number of characters, in which case we
want to discard the buffered characters anyway). So _M_finish() can be
simplified and can avoid calling _M_overflow().

This change also fixes some member functions of other sink classes to
only call _M_overflow() when there are characters in the buffer, which
is needed to meet _M_overflow's precondition that _M_used().size()!=0.

libstdc++-v3/ChangeLog:

	PR libstdc++/110990
	* include/std/format (_Seq_sink::get): Only call _M_overflow if
	its precondition is met.
	(_Iter_sink::_M_finish): Likewise.
	(_Iter_sink<C, ContigIter>::_M_overflow): Only switch to the
	internal buffer after running out of space.
	(_Iter_sink<C, ContigIter>::_M_finish): Do not use _M_overflow.
	(_Counting_sink::count): Likewise.
	* testsuite/std/format/functions/format_to_n.cc: Check cases
	where the output fits into the buffer.
---
 libstdc++-v3/include/std/format               | 37 +++++++++++--------
 .../std/format/functions/format_to_n.cc       | 17 +++++++++
 2 files changed, 38 insertions(+), 16 deletions(-)

diff --git a/libstdc++-v3/include/std/format b/libstdc++-v3/include/std/format
index 23da6b008c5..f4520ff3f74 100644
--- a/libstdc++-v3/include/std/format
+++ b/libstdc++-v3/include/std/format
@@ -2484,7 +2484,8 @@ namespace __format
       _Seq
       get() &&
       {
-	_Seq_sink::_M_overflow();
+	if (this->_M_used().size() != 0)
+	  _Seq_sink::_M_overflow();
 	return std::move(_M_seq);
       }
     };
@@ -2540,7 +2541,8 @@ namespace __format
       format_to_n_result<_OutIter>
       _M_finish() &&
       {
-	_Iter_sink::_M_overflow();
+	if (this->_M_used().size() != 0)
+	  _Iter_sink::_M_overflow();
 	iter_difference_t<_OutIter> __count(_M_count);
 	return { std::move(_M_out), __count };
       }
@@ -2567,8 +2569,11 @@ namespace __format
 
     protected:
       void
-      _M_overflow()
+      _M_overflow() override
       {
+	if (this->_M_unused().size() != 0)
+	  return; // No need to switch to internal buffer yet.
+
 	auto __s = this->_M_used();
 	_M_count += __s.size();
 
@@ -2632,15 +2637,18 @@ namespace __format
       format_to_n_result<_OutIter>
       _M_finish() &&
       {
-	_Iter_sink::_M_overflow();
-	iter_difference_t<_OutIter> __count(_M_count);
 	auto __s = this->_M_used();
-	auto __last = _M_first;
-	if (__s.data() == _M_buf) // Wrote at least _M_max characters.
-	  __last += _M_max;
-	else
-	  __last += iter_difference_t<_OutIter>(__s.size());
-	return { __last, __count };
+	if (__s.data() == _M_buf)
+	  {
+	    // Switched to internal buffer, so must have written _M_max.
+	    iter_difference_t<_OutIter> __count(_M_count + __s.size());
+	    return { _M_first + _M_max, __count };
+	  }
+	else // Not using internal buffer yet
+	  {
+	    iter_difference_t<_OutIter> __count(__s.size());
+	    return { _M_first + __count, __count };
+	  }
       }
     };
 
@@ -3882,11 +3890,8 @@ namespace __format
 
       [[__gnu__::__always_inline__]]
       size_t
-      count()
-      {
-	_Counting_sink::_M_overflow();
-	return this->_M_count;
-      }
+      count() const
+      { return this->_M_count + this->_M_used().size(); }
     };
 #else
   template<typename _CharT>
diff --git a/libstdc++-v3/testsuite/std/format/functions/format_to_n.cc b/libstdc++-v3/testsuite/std/format/functions/format_to_n.cc
index 846bda30fdf..f7df3ed36dc 100644
--- a/libstdc++-v3/testsuite/std/format/functions/format_to_n.cc
+++ b/libstdc++-v3/testsuite/std/format/functions/format_to_n.cc
@@ -88,9 +88,26 @@ test_move_only()
   VERIFY( wlen == 11 );
 }
 
+void
+test_pr110990()
+{
+  // PR libstdc++/110990 - format_to_n returns wrong value
+
+  char buf[2];
+  auto [ptr, len] = std::format_to_n(buf, 2, "x");
+  VERIFY( len == 1 );
+  VERIFY( ptr == buf + len );
+
+  wchar_t wbuf[3];
+  auto [wptr, wlen] = std::format_to_n(wbuf, 3, L"yz");
+  VERIFY( wlen == 2 );
+  VERIFY( wptr == wbuf + wlen );
+}
+
 int main()
 {
   test();
   test_wchar();
   test_move_only();
+  test_pr110990();
 }
-- 
2.41.0