Tested x86_64-linux. Pushed to trunk.
-- >8 --
Fix an incorrect call to _Sink::_M_reserve() which should have passed
the __n parameter. This was not actually a problem because it was in an
discarded statement, because only the _Seq_sink<basic_string<C>>
specialization was used.
Also add some branch prediction hints, explanatory comments, and debug
mode assertions to _Seq_sink.
libstdc++-v3/ChangeLog:
* include/std/format (_Seq_sink): Fix missing argument in
discarded statement. Add comments, likely/unlikely attributes
and debug assertions as sanity checks.
---
libstdc++-v3/include/std/format | 40 +++++++++++++++++++++++----------
1 file changed, 28 insertions(+), 12 deletions(-)
diff --git a/libstdc++-v3/include/std/format b/libstdc++-v3/include/std/format
index 1f8cd5c06be..6204fd0e3c1 100644
--- a/libstdc++-v3/include/std/format
+++ b/libstdc++-v3/include/std/format
@@ -2609,15 +2609,13 @@ namespace __format
virtual _Reservation
_M_reserve(size_t __n)
{
- auto __avail = _M_unused();
- if (__n <= __avail.size())
+ if (__n <= _M_unused().size())
return { this };
if (__n <= _M_span.size()) // Cannot meet the request.
{
_M_overflow(); // Make more space available.
- __avail = _M_unused();
- if (__n <= __avail.size())
+ if (__n <= _M_unused().size())
return { this };
}
return { nullptr };
@@ -2669,24 +2667,38 @@ namespace __format
_M_overflow() override
{
auto __s = this->_M_used();
- if (__s.empty())
- return;
+ if (__s.empty()) [[unlikely]]
+ return; // Nothing in the buffer to transfer to _M_seq.
+
+ // If _M_reserve was called then _M_bump must have been called too.
+ _GLIBCXX_DEBUG_ASSERT(__s.data() != _M_seq.data());
+
if constexpr (__is_specialization_of<_Seq, basic_string>)
_M_seq.append(__s.data(), __s.size());
else
_M_seq.insert(_M_seq.end(), __s.begin(), __s.end());
+
+ // Make the whole of _M_buf available for the next write:
this->_M_rewind();
}
typename _Sink<_CharT>::_Reservation
_M_reserve(size_t __n) override
{
+ // We might already have n characters available in this->_M_unused(),
+ // but the whole point of this function is to be an optimization for
+ // the std::format("{}", x) case. We want to avoid writing to _M_buf
+ // and then copying that into a basic_string if possible, so this
+ // function prefers to create space directly in _M_seq rather than
+ // using _M_buf.
+
if constexpr (__is_specialization_of<_Seq, basic_string>
|| __is_specialization_of<_Seq, vector>)
{
- // Flush the buffer to _M_seq first:
- if (this->_M_used().size())
- _M_overflow();
+ // Flush the buffer to _M_seq first (should not be needed).
+ if (this->_M_used().size()) [[unlikely]]
+ _Seq_sink::_M_overflow();
+
// Expand _M_seq to make __n new characters available:
const auto __sz = _M_seq.size();
if constexpr (is_same_v<string, _Seq> || is_same_v<wstring, _Seq>)
@@ -2696,12 +2708,14 @@ namespace __format
});
else
_M_seq.resize(__sz + __n);
- // Set _M_used() to be a span over the original part of _M_seq:
+
+ // Set _M_used() to be a span over the original part of _M_seq
+ // and _M_unused() to be the extra capacity we just created:
this->_M_reset(_M_seq, __sz);
return { this };
}
else // Try to use the base class' buffer.
- return _Sink<_CharT>::_M_reserve();
+ return _Sink<_CharT>::_M_reserve(__n);
}
void
@@ -2710,8 +2724,10 @@ namespace __format
if constexpr (__is_specialization_of<_Seq, basic_string>
|| __is_specialization_of<_Seq, vector>)
{
+ auto __s = this->_M_used();
+ _GLIBCXX_DEBUG_ASSERT(__s.data() == _M_seq.data());
// Truncate the sequence to the part that was actually written to:
- _M_seq.resize(this->_M_used().size() + __n);
+ _M_seq.resize(__s.size() + __n);
// Switch back to using buffer:
this->_M_reset(this->_M_buf);
}
--
2.43.0
