[PATCH] libstdc++: fix unordered_map heterogeneous insertion copying [PR126800]
Jonathan Wakely
jwakely@redhat.com
Thu Sep 3 13:48:01 GMT 2026
On Wed, 2 Sept 2026 at 18:50, Nathan Myers <ncm@cantrip.org> wrote:
>
> PR126800 reports that heterogeneous insertions fail to avoid
> copying the key when no insertion occurs. They call an existing
> member try_emplace that had invoked the wrong helper for their
> new usage case, and tests failed to detect the error. This patch
> calls the correct helper, and amends tests for heterogeneous
> insert operations op[], at(), try_emplace(), and
> insert_or_replace().
OK for trunk and gcc-16, thanks.
>
> libstdc++-v3/Changelog:
> PR libstdc++/126800
> * include/bits/hashtable.h: (try_emplace): Call correct helper.
> * testsuite/23_containers/unordered_map/modifiers/hetero/insert.cc:
> Check whether key arguments have been copied.
> ---
> libstdc++-v3/include/bits/hashtable.h | 2 +-
> .../unordered_map/modifiers/hetero/insert.cc | 125 ++++++++++++++----
> 2 files changed, 99 insertions(+), 28 deletions(-)
>
> diff --git a/libstdc++-v3/include/bits/hashtable.h b/libstdc++-v3/include/bits/hashtable.h
> index eff6c31d827..5b98b1193ed 100644
> --- a/libstdc++-v3/include/bits/hashtable.h
> +++ b/libstdc++-v3/include/bits/hashtable.h
> @@ -1107,7 +1107,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> // Note we ignore the hint argument.
> __hash_code __code;
> size_type __bkt;
> - if (auto __loc = _M_locate(__k))
> + if (auto __loc = _M_locate_tr(__k))
> return { iterator(__loc), false };
> else
> {
> diff --git a/libstdc++-v3/testsuite/23_containers/unordered_map/modifiers/hetero/insert.cc b/libstdc++-v3/testsuite/23_containers/unordered_map/modifiers/hetero/insert.cc
> index c1d86b8eea3..87f997d3a6c 100644
> --- a/libstdc++-v3/testsuite/23_containers/unordered_map/modifiers/hetero/insert.cc
> +++ b/libstdc++-v3/testsuite/23_containers/unordered_map/modifiers/hetero/insert.cc
> @@ -6,6 +6,7 @@
> #include <utility>
> #include <functional>
> #include <compare>
> +#include <exception>
> #include <testsuite_hooks.h>
>
> struct Y;
> @@ -20,11 +21,12 @@ struct X {
>
> struct Y {
> std::string s;
> + mutable bool copied_from = false; // [PR126800]
> Y() = default;
> Y(Y&& y) : s(std::move(y.s)) { y.s.clear(); }
> - Y(const Y& y) = default;
> + Y(const Y& y) : s(y.s) { y.copied_from = true; }
> Y& operator=(Y&& y) { s = std::move(y.s); y.s.clear(); return *this; }
> - Y& operator=(const Y& y) = default;
> + Y& operator=(const Y& y) { s = y.s; y.copied_from = true; return *this; }
> Y(std::string_view sv) : s(sv) {}
> Y(int n) : s(std::string('a', n)) {}
> Y(const Y& a, const Y& b) : s(a.s + "1" + b.s) { }
> @@ -36,7 +38,7 @@ struct Y {
> };
>
> X::X(Y&& y) : s(std::move(y.s)) { y.s.clear(); }
> -X::X(const Y& y) : s(y.s) {}
> +X::X(const Y& y) : s(y.s) { y.copied_from = true; }
>
> struct Hash {
> using is_transparent = void;
> @@ -52,22 +54,33 @@ void test_op_bracket()
> std::unordered_map<X, Y, Hash, Equal> amap;
> amap.insert({{X{"abc"}, 1}, {X{"def"}, 2}, {X{"ghi"}, 3}});
>
> - Y x{"dei"}, y{"deh"}, z{"deg"};
> - amap[z] = 4;
> + Y z{"deg"};
> + amap[z] = Y{4};
> VERIFY(amap.size() == 4);
> VERIFY(z.s.size() == 3); // not moved from.
> + VERIFY(z.copied_from);
> + VERIFY(amap[Y{"deg"}] == Y{4});
>
> - amap[std::move(z)] = 5;
> + Y zz{"deg"};
> + amap[std::move(zz)] = Y{5};
> VERIFY(amap.size() == 4);
> - VERIFY(z.s.size() == 3); // not moved from.
> + VERIFY(zz.s.size() == 3); // not moved from.
> + VERIFY(!zz.copied_from);
> + VERIFY(amap[Y{"deg"}] == Y{5});
>
> - VERIFY(amap[std::move(y)] == Y{});
> + Y y{"deh"};
> + amap[y] = Y{6};
> VERIFY(amap.size() == 5);
> - VERIFY(y.s.empty()); // moved from.
> + VERIFY(y.s.size() == 3); // not moved from.
> + VERIFY(y.copied_from);
> + VERIFY(amap[Y{"deh"}] == Y{6});
>
> - amap[std::move(x)] = 7;
> + Y x{"dei"};
> + amap[std::move(x)] = Y{7};
> VERIFY(amap.size() == 6);
> VERIFY(x.s.empty()); // moved from
> + VERIFY(!x.copied_from);
> + VERIFY(amap[Y{"dei"}] == Y{7});
> }
>
> void test_at()
> @@ -81,21 +94,24 @@ void test_at()
> VERIFY(2 == amap.at(x));
> VERIFY(amap.size() == 3);
> VERIFY(x.s.size() == 3); // not moved from
> + VERIFY(!x.copied_from);
> VERIFY(4 == (amap.at(x) = 4));
> VERIFY(amap.size() == 3);
> VERIFY(x.s.size() == 3); // not moved from
> + VERIFY(!x.copied_from);
> }
> catch(...) { VERIFY(false); }
>
> Y z{"deg"};
> try
> {
> - amap.at(z) = 4;
> + amap.at(z) = Y{1};
> VERIFY(false); // Should have thrown.
> }
> catch (std::out_of_range&) { VERIFY(amap.size() == 3); }
> catch (...) { VERIFY(false); } // Wrong exception.
> VERIFY(z.s.size() == 3); // not moved from
> + VERIFY(!z.copied_from);
>
> Y y{"deh"};
> auto const& amapr{amap};
> @@ -108,6 +124,7 @@ void test_at()
> catch (...) { VERIFY(false); } // Wrong exception.
> VERIFY(amapr.size() == 3);
> VERIFY(y.s.size() == 3); // not moved from
> + VERIFY(!y.copied_from);
> }
>
> void test_try_emplace()
> @@ -117,10 +134,15 @@ void test_try_emplace()
>
> { // Fail, already there
> auto a = amap;
> - auto [it, res] = a.try_emplace(Y{"def"}, Y{"xyz"});
> + Y y{"def"}, z{"xyz"};
> + auto [it, res] = a.try_emplace(std::move(y), z);
> VERIFY(!res);
> VERIFY(a.size() == 3);
> VERIFY(a.at(Y{"def"}) == Y{2});
> + VERIFY(y.s.size() == 3); // not moved from
> + VERIFY(!y.copied_from);
> + VERIFY(z.s.size() == 3); // not moved from
> + VERIFY(!z.copied_from);
> }
> { // Fail, already there, move
> auto a = amap;
> @@ -130,13 +152,16 @@ void test_try_emplace()
> VERIFY(a.size() == 3);
> VERIFY(a.at(Y{"def"}) == Y{2});
> VERIFY(y.s.size() == 3); // not moved from
> + VERIFY(!y.copied_from);
> VERIFY(z.s.size() == 3); // not moved from
> + VERIFY(!z.copied_from);
> }
> { // Succeed, construct
> auto a = amap;
> - Y m("m"), n("n"), o("o"), p("p"), dek("dek");
> + Y m("m"), n("n"), o("o"), p("p");
> {
> - auto [it, res] = a.try_emplace(Y{"deg"}, m, n);
> + Y y{"deg"};
> + auto [it, res] = a.try_emplace(std::move(y), m, n);
> VERIFY(res);
> VERIFY(a.size() == 4);
> VERIFY(it->first == X{"deg"});
> @@ -145,39 +170,47 @@ void test_try_emplace()
> VERIFY(n.s.size() == 1);
> }
> {
> - auto [it, res] = a.try_emplace(Y{"deh"}, m, std::move(n));
> + Y y{"deh"};
> + auto [it, res] = a.try_emplace(std::move(y), m, std::move(n));
> VERIFY(res);
> VERIFY(a.size() == 5);
> VERIFY(it->first == X{"deh"});
> VERIFY(it->second == Y{"m2n"});
> VERIFY(m.s.size() == 1);
> VERIFY(n.s.empty());
> + VERIFY(!y.copied_from);
> }
> {
> - auto [it, res] = a.try_emplace(Y{"dei"}, std::move(m), o);
> + Y y{"dei"};
> + auto [it, res] = a.try_emplace(std::move(y), std::move(m), o);
> VERIFY(res);
> VERIFY(a.size() == 6);
> VERIFY(it->first == X{"dei"});
> VERIFY(it->second == Y{"m3o"});
> VERIFY(m.s.empty());
> VERIFY(o.s.size() == 1);
> + VERIFY(!y.copied_from);
> }
> {
> - auto [it, res] = a.try_emplace(Y{"dej"}, std::move(o), std::move(p));
> + Y y{"dej"};
> + auto [it, res] = a.try_emplace(std::move(y), std::move(o), std::move(p));
> VERIFY(res);
> VERIFY(a.size() == 7);
> VERIFY(it->first == X{"dej"});
> VERIFY(it->second == Y{"o4p"});
> VERIFY(o.s.empty());
> VERIFY(p.s.empty());
> + VERIFY(!y.copied_from);
> }
> {
> - auto [it, res] = a.try_emplace(std::move(dek), Y("q"), Y("r"));
> + Y y{"dek"};
> + auto [it, res] = a.try_emplace(std::move(y), Y("q"), Y("r"));
> VERIFY(res);
> VERIFY(a.size() == 8);
> - VERIFY(dek.s.empty());
> + VERIFY(y.s.empty());
> VERIFY(it->first == X{"dek"});
> VERIFY(it->second == Y{"q4r"});
> + VERIFY(!y.copied_from);
> }
> }
> { // Succeed, move
> @@ -190,16 +223,20 @@ void test_try_emplace()
> VERIFY(a.size() == 4);
> VERIFY(y.s.empty()); // moved from
> VERIFY(z.s.empty()); // moved from
> + VERIFY(!y.copied_from);
> + VERIFY(!z.copied_from);
> }
> { // Hinted, fail
> auto a = amap;
> Y y{"def"}, z{"xyz"};
> - auto it = a.try_emplace(a.begin(), std::move(y), std::move(z));
> + auto it = a.try_emplace(a.begin(), std::move(y), z);
> VERIFY(a.size() == 3);
> VERIFY(it->first == X{"def"});
> VERIFY(it->second == Y{2});
> VERIFY(y.s.size() == 3); // not moved from
> VERIFY(z.s.size() == 3); // not moved from
> + VERIFY(!y.copied_from);
> + VERIFY(!z.copied_from);
> }
> { // Hinted, fail, move
> auto a = amap;
> @@ -210,48 +247,66 @@ void test_try_emplace()
> VERIFY(it->second == Y{2});
> VERIFY(y.s.size() == 3); // not moved from
> VERIFY(z.s.size() == 3); // not moved from
> + VERIFY(!y.copied_from);
> + VERIFY(!z.copied_from);
> }
> { // Hinted, succeed, construct
> auto a = amap;
> - Y m("m"), n("n"), o("o"), p("p"), dek("dek");
> + Y m("m"), n("n"), o("o"), p("p");
> {
> - auto it = a.try_emplace(a.begin(), Y{"deg"}, m, n);
> + Y y{"deg"};
> + auto it = a.try_emplace(a.begin(), y, m, n);
> VERIFY(a.size() == 4);
> VERIFY(it->first == X{"deg"});
> VERIFY(it->second == Y{"m1n"});
> VERIFY(m.s.size() == 1);
> VERIFY(n.s.size() == 1);
> + VERIFY(y.s.size() == 3);
> + VERIFY(y.copied_from);
> }
> {
> - auto it = a.try_emplace(a.begin(), Y{"deh"}, m, std::move(n));
> + Y y{"deh"};
> + auto it = a.try_emplace(a.begin(), y, m, std::move(n));
> VERIFY(a.size() == 5);
> VERIFY(it->first == X{"deh"});
> VERIFY(it->second == Y{"m2n"});
> VERIFY(m.s.size() == 1);
> VERIFY(n.s.empty());
> + VERIFY(y.s.size() == 3);
> + VERIFY(y.copied_from);
> }
> {
> - auto it = a.try_emplace(a.begin(), Y{"dei"}, std::move(m), o);
> + Y y{"dei"};
> + auto it = a.try_emplace(a.begin(), y, std::move(m), o);
> VERIFY(a.size() == 6);
> VERIFY(it->first == X{"dei"});
> VERIFY(it->second == Y{"m3o"});
> VERIFY(m.s.empty());
> VERIFY(o.s.size() == 1);
> + VERIFY(y.s.size() == 3);
> + VERIFY(y.copied_from);
> }
> {
> - auto it = a.try_emplace(a.begin(), Y{"dej"}, std::move(o), std::move(p));
> + Y y{"dej"};
> + auto it = a.try_emplace(a.begin(), y, std::move(o), std::move(p));
> VERIFY(a.size() == 7);
> VERIFY(it->first == X{"dej"});
> VERIFY(it->second == Y{"o4p"});
> VERIFY(o.s.empty());
> VERIFY(p.s.empty());
> + VERIFY(y.s.size() == 3);
> + VERIFY(y.copied_from);
> }
> {
> - auto it = a.try_emplace(a.begin(), std::move(dek), Y("q"), Y("r"));
> + Y y{"dek"};
> + auto it = a.try_emplace(a.begin(), std::move(y), Y("q"), Y("r"));
> VERIFY(a.size() == 8);
> - VERIFY(dek.s.empty());
> + VERIFY(y.s.empty());
> VERIFY(it->first == X{"dek"});
> VERIFY(it->second == Y{"q4r"});
> + VERIFY(it->second == Y{"q4r"});
> + VERIFY(y.s.empty()); // moved from
> + VERIFY(!y.copied_from);
> }
> }
> { // Hinted, succeed, move
> @@ -262,7 +317,9 @@ void test_try_emplace()
> VERIFY(it->second == Y{"xyz"});
> VERIFY(a.size() == 4);
> VERIFY(y.s.empty()); // moved from
> + VERIFY(!y.copied_from);
> VERIFY(z.s.empty()); // moved from
> + VERIFY(!z.copied_from);
> }
> }
>
> @@ -279,7 +336,9 @@ void test_insert_or_assign()
> VERIFY(a.size() == 3);
> VERIFY(a.at(Y{"def"}) == Y{"xyz"});
> VERIFY(y.s.size() == 3); // not moved from
> + VERIFY(!y.copied_from);
> VERIFY(z.s.size() == 3); // not moved from
> + VERIFY(z.copied_from);
> }
> { // Already there, move
> auto a = amap;
> @@ -289,7 +348,9 @@ void test_insert_or_assign()
> VERIFY(a.size() == 3);
> VERIFY(a.at(Y{"def"}) == Y{"xyz"});
> VERIFY(y.s.size() == 3); // not moved from
> + VERIFY(!y.copied_from);
> VERIFY(z.s.empty()); // moved from
> + VERIFY(!z.copied_from);
> }
> { // Succeed, move
> auto a = amap;
> @@ -300,7 +361,9 @@ void test_insert_or_assign()
> VERIFY(it->second == Y{"xyz"});
> VERIFY(a.size() == 4);
> VERIFY(y.s.empty()); // moved from
> + VERIFY(!y.copied_from);
> VERIFY(z.s.empty()); // moved from
> + VERIFY(!z.copied_from);
> }
> { // Hinted, already there, replace
> auto a = amap;
> @@ -310,7 +373,9 @@ void test_insert_or_assign()
> VERIFY(it->first == X{"def"});
> VERIFY(it->second == Y{"xyz"});
> VERIFY(y.s.size() == 3); // not moved from
> + VERIFY(!y.copied_from);
> VERIFY(z.s.size() == 3); // not moved from
> + VERIFY(z.copied_from);
> }
> { // Hinted, already there, move
> auto a = amap;
> @@ -320,7 +385,9 @@ void test_insert_or_assign()
> VERIFY(it->first == X{"def"});
> VERIFY(it->second == Y{"xyz"});
> VERIFY(y.s.size() == 3); // not moved from
> + VERIFY(!y.copied_from);
> VERIFY(z.s.empty()); // moved from
> + VERIFY(!z.copied_from);
> }
> { // Hinted, succeed
> auto a = amap;
> @@ -330,7 +397,9 @@ void test_insert_or_assign()
> VERIFY(it->second == Y{"xyz"});
> VERIFY(a.size() == 4);
> VERIFY(y.s.size() == 3); // not moved from
> + VERIFY(y.copied_from);
> VERIFY(z.s.size() == 3); // not moved from
> + VERIFY(z.copied_from);
> }
> { // Hinted, succeed, move
> auto a = amap;
> @@ -340,7 +409,9 @@ void test_insert_or_assign()
> VERIFY(it->second == Y{"xyz"});
> VERIFY(a.size() == 4);
> VERIFY(y.s.empty()); // moved from
> + VERIFY(!y.copied_from);
> VERIFY(z.s.empty()); // moved from
> + VERIFY(!z.copied_from);
> }
> }
>
> --
> 2.55.0
>
More information about the Libstdc++
mailing list