libstdc++: Avoid redundant value_type object in flat_set::emplace [PR119620]

flat_set::emplace (and flat_multiset's) currently unconditionally
constructs an object outside of the container, but if we're passed
a value_type object we can and should avoid that.

	PR libstdc++/119620

libstdc++-v3/ChangeLog:

	* include/std/flat_set (_Flat_set_impl::_M_try_emplace): Split
	out into two overloads, one taking at least one argument and one
	taking zero arguments.  Turn __k into an auto&& reference bound
	to __arg if it's already a value_type and otherwise bound to a
	lifetime-extended value_type temporary.
	* testsuite/23_containers/flat_multiset/1.cc (test08): New test.
	* testsuite/23_containers/flat_set/1.cc (test08): New test.

Reviewed-by: Tomasz Kamiński <tkaminsk@redhat.com>
Reviewed-by: Jonathan Wakely <jwakely@redhat.com>

diff --git a/libstdc++-v3/include/std/flat_set b/libstdc++-v3/include/std/flat_set
index a7b0b8aef151020336b8161b2094495ac630b02d..3e15d1af416288530f511a750069abeff3046971 100644 (file)
--- a/libstdc++-v3/include/std/flat_set
+++ b/libstdc++-v3/include/std/flat_set
@@ -350,12 +350,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       { return _M_cont.max_size(); }
 
       // modifiers
-      template<typename... _Args>
+      template<typename _Arg, typename... _Args>
        pair<iterator, bool>
-       _M_try_emplace(optional<const_iterator> __hint, _Args&&... __args)
+       _M_try_emplace(optional<const_iterator> __hint, _Arg&& __arg, _Args&&... __args)
        {
          // TODO: Simplify and audit the hint handling.
-         value_type __k(std::forward<_Args>(__args)...);
+         auto&& __k = [&] -> decltype(auto) {
+           if constexpr (sizeof...(_Args) == 0
+                         && same_as<remove_cvref_t<_Arg>, value_type>)
+             return std::forward<_Arg>(__arg);
+           else
+             return value_type(std::forward<_Arg>(__arg),
+                               std::forward<_Args>(__args)...);
+         }();
          typename container_type::iterator __it;
          int __r = -1, __s = -1;
          if (__hint.has_value()
@@ -397,11 +404,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
              return {__it, false};
 
          auto __guard = _M_make_clear_guard();
-         __it = _M_cont.insert(__it, std::move(__k));
+         __it = _M_cont.insert(__it, std::forward<decltype(__k)>(__k));
          __guard._M_disable();
          return {__it, true};
        }
 
+      template<typename... _Args>
+       pair<iterator, bool>
+       _M_try_emplace(optional<const_iterator> __hint)
+       { return _M_try_emplace(__hint, value_type()); }
+
       template<typename... _Args>
        requires is_constructible_v<value_type, _Args...>
        __emplace_result_t

diff --git a/libstdc++-v3/testsuite/23_containers/flat_multiset/1.cc b/libstdc++-v3/testsuite/23_containers/flat_multiset/1.cc
index dc3cecd9720c943ebc7ed1220a1bd7cad8481d1c..7d9a33c6b000a2b2e2c38e24712a1aab3984733e 100644 (file)
--- a/libstdc++-v3/testsuite/23_containers/flat_multiset/1.cc
+++ b/libstdc++-v3/testsuite/23_containers/flat_multiset/1.cc
@@ -214,6 +214,27 @@ void test07()
 #endif
 }
 
+void
+test08()
+{
+  // PR libstdc++/119620 -- flat_set::emplace always constructs element on the stack
+  static int copy_counter;
+  struct A {
+    A() { }
+    A(const A&) { ++copy_counter; }
+    A& operator=(const A&) { ++copy_counter; return *this; }
+    auto operator<=>(const A&) const = default;
+  };
+  std::vector<A> v;
+  v.reserve(2);
+  std::flat_multiset<A> s(std::move(v));
+  A a;
+  s.emplace(a);
+  VERIFY( copy_counter == 1 );
+  s.emplace(a);
+  VERIFY( copy_counter == 2 );
+}
+
 int
 main()
 {
@@ -225,4 +246,5 @@ main()
   test05();
   test06();
   test07();
+  test08();
 }

diff --git a/libstdc++-v3/testsuite/23_containers/flat_set/1.cc b/libstdc++-v3/testsuite/23_containers/flat_set/1.cc
index 90f5855859f7accea49a3b37a15ae295128cede9..ed24fab785b532d649239dd05771ed42e261e449 100644 (file)
--- a/libstdc++-v3/testsuite/23_containers/flat_set/1.cc
+++ b/libstdc++-v3/testsuite/23_containers/flat_set/1.cc
@@ -229,6 +229,25 @@ void test07()
 #endif
 }
 
+void
+test08()
+{
+  // PR libstdc++/119620 -- flat_set::emplace always constructs element on the stack
+  static int copy_counter;
+  struct A {
+    A() { }
+    A(const A&) { ++copy_counter; }
+    A& operator=(const A&) { ++copy_counter; return *this; }
+    auto operator<=>(const A&) const = default;
+  };
+  std::flat_set<A> s;
+  A a;
+  s.emplace(a);
+  VERIFY( copy_counter == 1 );
+  s.emplace(a);
+  VERIFY( copy_counter == 1 );
+}
+
 int
 main()
 {
@@ -240,4 +259,5 @@ main()
   test05();
   test06();
   test07();
+  test08();
 }