On Mon, 13 Jul 2026 at 16:22, Patrick Palka <[email protected]> wrote:
>
> Tested on x86_64-pc-linux-gnu, does this look OK for trunk and perhaps 16?
> I'm not sure if it's worth avoiding dereferencing *__first twice in this
> special case, but I figured we might as well?
>
> -- >8 --
>
> When inserting a range of pair-like elements we can avoid constructing a
> value_type temporary and instead obtain the corresponding key and value
> to insert directly from *__first.
So to check I understand this correctly, this is related to LWG 4499
but we already implemented what it says in the issue, because we
already had a conversion to value_type (and we don't implement
flat_set::insert_range as specified in the standard anyway, so there
was never any ambiguity with the vector::insert(const_iterator,
initializer_list<T>) member).
But our loop that implements insert_range can be optimized to avoid
the conversion to value_type in some cases (as hinted at in the issue
discussion).
The patch is OK for trunk and gcc-16. I'd like to get rid of the extra
braces that Tomasz pointed out, his other suggestions seem ok too, but
your call if you want to do that.
>
> libstdc++-v3/ChangeLog:
>
> * include/std/flat_map (flat_map::_M_insert): Avoid constructing
> value_type temporary when the iterator already has pair-like
> elements.
> ---
> libstdc++-v3/include/std/flat_map | 17 ++++++++++++++---
> 1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/libstdc++-v3/include/std/flat_map
> b/libstdc++-v3/include/std/flat_map
> index b82d41b4f5e1..1815ced51225 100644
> --- a/libstdc++-v3/include/std/flat_map
> +++ b/libstdc++-v3/include/std/flat_map
> @@ -637,9 +637,20 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> auto __n = size();
> for (; __first != __last; ++__first)
> {
> - value_type __value = *__first;
> - _M_cont.keys.emplace_back(std::move(__value.first));
> - _M_cont.values.emplace_back(std::move(__value.second));
> + if constexpr (__pair_like<iter_reference_t<_Iter>>)
> + {
> + auto&& __value = *__first;
> + _M_cont.keys.emplace_back
> + (std::get<0>(std::forward<decltype(__value)>(__value)));
> + _M_cont.values.emplace_back
> + (std::get<1>(std::forward<decltype(__value)>(__value)));
> + }
> + else
> + {
> + value_type __value = *__first;
> + _M_cont.keys.emplace_back(std::move(__value.first));
> + _M_cont.values.emplace_back(std::move(__value.second));
> + }
> }
> auto __zv = views::zip(_M_cont.keys, _M_cont.values);
> if (__is_sorted)
> --
> 2.55.0.122.gf85a7e6620
>
