From: Patrick Palka <ppalka@redhat.com>
Date: Fri, 4 Apr 2025 19:05:09 +0000 (-0400)
Subject: libstdc++: Avoid redundant value_type object in flat_set::emplace [PR119620]
X-Git-Tag: basepoints/gcc-16~342
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=2a36d22ab52d6ffce9a1fcaf7aca83336679e111;p=thirdparty%2Fgcc.git

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 a7b0b8aef15..3e15d1af416 100644
--- 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 dc3cecd9720..7d9a33c6b00 100644
--- 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 90f5855859f..ed24fab785b 100644
--- 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();
 }