https://gcc.gnu.org/g:54f01c74b1b43836d72c3ba8244fd1e679f3b3b3
commit r17-3903-g54f01c74b1b43836d72c3ba8244fd1e679f3b3b3 Author: Nathan Myers <[email protected]> Date: Wed Sep 2 09:43:15 2026 -0400 libstdc++: fix unordered_map heterogeneous insertion copying [PR126800] 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(). 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. Diff: --- 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 eff6c31d8272..5b98b1193ed5 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 c1d86b8eea34..87f997d3a6c9 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); } }
