[libcxx-commits] [libcxx] [libc++] Opt `std::*set` out of map key extraction optimization (PR #220452)
George Burgess IV via libcxx-commits
libcxx-commits at lists.llvm.org
Wed Sep 2 08:48:23 PDT 2026
https://github.com/gburgessiv updated https://github.com/llvm/llvm-project/pull/220452
>From 6716e09c9f075fc8603c4e0c6b21de9fd9bf4bca Mon Sep 17 00:00:00 2001
From: George Burgess IV <gbiv at google.com>
Date: Tue, 1 Sep 2026 15:06:36 -0600
Subject: [PATCH 1/2] [libc++] Opt `std::*set` out of map key extraction
optimization
PR #154512 (relanded by #155565) removed `__can_extract_map_key`, which
had a blanket opt-out for `std::*set`s. The new logic does not have that
opt-out, leading to `std::set`s being incorrectly constructed.
The new regression tests demonstrate this, but essentially the idea is:
1. Have `std::set<T> foo;`
2. Call `foo.emplace(some_t,
arg_that_influences_comparisons_or_hashes);`
3. The emplace will internally search using `some_t` as the key, *not*
`T(some_t, arg_that_influences_comparisons_or_hashes);`
This opts out `std::*set` from this optimization to match previous
behavior.
Tests and fix were produced by an LLM. I reviewed them and they seem
reasonable to me, though I don't have a strong background in libc++
testing conventions.
Fixes #220451
---
libcxx/include/__hash_table | 2 +-
libcxx/include/__tree | 6 +++---
libcxx/include/__utility/try_key_extraction.h | 17 +++++++++++------
.../containers/associative/set/emplace.pass.cpp | 10 ++++++++++
.../containers/unord/unord.set/emplace.pass.cpp | 10 ++++++++++
5 files changed, 35 insertions(+), 10 deletions(-)
diff --git a/libcxx/include/__hash_table b/libcxx/include/__hash_table
index ed58789378bc1..4874e4e084c38 100644
--- a/libcxx/include/__hash_table
+++ b/libcxx/include/__hash_table
@@ -788,7 +788,7 @@ public:
template <class... _Args>
_LIBCPP_HIDE_FROM_ABI pair<iterator, bool> __emplace_unique(_Args&&... __args) {
- return std::__try_key_extraction<key_type>(
+ return std::__try_key_extraction<key_type, value_type>(
[this](const key_type& __key, _Args&&... __args2) {
size_t __hash = hash_function()(__key);
size_type __bc = bucket_count();
diff --git a/libcxx/include/__tree b/libcxx/include/__tree
index 7c9a65ac3fbda..7cfa8e86fdc02 100644
--- a/libcxx/include/__tree
+++ b/libcxx/include/__tree
@@ -1074,7 +1074,7 @@ public:
template <class... _Args>
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX26 pair<iterator, bool> __emplace_unique(_Args&&... __args) {
- return std::__try_key_extraction<key_type>(
+ return std::__try_key_extraction<key_type, value_type>(
[this](const key_type& __key, _Args&&... __args2) {
auto [__parent, __child] = __find_equal(__key);
__node_pointer __r = std::__static_fancy_pointer_cast<__node_pointer>(__child);
@@ -1105,7 +1105,7 @@ public:
template <class... _Args>
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX26 pair<iterator, bool>
__emplace_hint_unique(const_iterator __p, _Args&&... __args) {
- return std::__try_key_extraction<key_type>(
+ return std::__try_key_extraction<key_type, value_type>(
[this, __p](const key_type& __key, _Args&&... __args2) {
__node_base_pointer __dummy;
auto [__parent, __child] = __find_equal(__p, __dummy, __key);
@@ -1182,7 +1182,7 @@ public:
using __reference = decltype(*__first);
for (; __first != __last; ++__first) {
- std::__try_key_extraction<key_type>(
+ std::__try_key_extraction<key_type, value_type>(
[this, &__max_node](const key_type& __key, __reference&& __val) {
if (value_comp()(__max_node->__get_value(), __key)) { // __key > __max_node
__node_holder __nd = __construct_node(std::forward<__reference>(__val));
diff --git a/libcxx/include/__utility/try_key_extraction.h b/libcxx/include/__utility/try_key_extraction.h
index 3423d746dee7f..d56a953f28ce2 100644
--- a/libcxx/include/__utility/try_key_extraction.h
+++ b/libcxx/include/__utility/try_key_extraction.h
@@ -27,13 +27,14 @@
_LIBCPP_BEGIN_NAMESPACE_STD
-template <class _KeyT, class _Ret, class _WithKey, class _WithoutKey, class... _Args>
+template <class _KeyT, class _ValT, class _Ret, class _WithKey, class _WithoutKey, class... _Args>
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX26 _Ret
__try_key_extraction_impl(__priority_tag<0>, _WithKey, _WithoutKey __without_key, _Args&&... __args) {
return __without_key(std::forward<_Args>(__args)...);
}
template <class _KeyT,
+ class _ValT,
class _Ret,
class _WithKey,
class _WithoutKey,
@@ -45,11 +46,12 @@ __try_key_extraction_impl(__priority_tag<1>, _WithKey __with_key, _WithoutKey, _
}
template <class _KeyT,
+ class _ValT,
class _Ret,
class _WithKey,
class _WithoutKey,
class _Arg,
- __enable_if_t<__is_pair_v<__remove_const_ref_t<_Arg> > &&
+ __enable_if_t<!is_same<_KeyT, _ValT>::value && __is_pair_v<__remove_const_ref_t<_Arg> > &&
is_same<__remove_const_t<typename __remove_const_ref_t<_Arg>::first_type>, _KeyT>::value,
int> = 0>
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX26 _Ret
@@ -58,12 +60,13 @@ __try_key_extraction_impl(__priority_tag<1>, _WithKey __with_key, _WithoutKey, _
}
template <class _KeyT,
+ class _ValT,
class _Ret,
class _WithKey,
class _WithoutKey,
class _Arg1,
class _Arg2,
- __enable_if_t<is_same<_KeyT, __remove_const_ref_t<_Arg1> >::value, int> = 0>
+ __enable_if_t<!is_same<_KeyT, _ValT>::value && is_same<_KeyT, __remove_const_ref_t<_Arg1> >::value, int> = 0>
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX26 _Ret
__try_key_extraction_impl(__priority_tag<1>, _WithKey __with_key, _WithoutKey, _Arg1&& __arg1, _Arg2&& __arg2) {
return __with_key(__arg1, std::forward<_Arg1>(__arg1), std::forward<_Arg2>(__arg2));
@@ -71,13 +74,15 @@ __try_key_extraction_impl(__priority_tag<1>, _WithKey __with_key, _WithoutKey, _
#ifndef _LIBCPP_CXX03_LANG
template <class _KeyT,
+ class _ValT,
class _Ret,
class _WithKey,
class _WithoutKey,
class _PiecewiseConstruct,
class _Tuple1,
class _Tuple2,
- __enable_if_t<is_same<__remove_const_ref_t<_PiecewiseConstruct>, piecewise_construct_t>::value &&
+ __enable_if_t<!is_same<_KeyT, _ValT>::value &&
+ is_same<__remove_const_ref_t<_PiecewiseConstruct>, piecewise_construct_t>::value &&
__is_tuple_v<_Tuple1> && tuple_size<_Tuple1>::value == 1 &&
is_same<__remove_const_ref_t<typename tuple_element<0, _Tuple1>::type>, _KeyT>::value,
int> = 0>
@@ -101,11 +106,11 @@ _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX26 _Ret __try_key_extraction_im
// arguments. Otherwise it calls the `__without_key` function with all of the arguments.
//
// Both `__with_key` and `__without_key` must take all arguments by reference.
-template <class _KeyT, class _WithKey, class _WithoutKey, class... _Args>
+template <class _KeyT, class _ValT, class _WithKey, class _WithoutKey, class... _Args>
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX26 decltype(std::declval<_WithoutKey>()(std::declval<_Args>()...))
__try_key_extraction(_WithKey __with_key, _WithoutKey __without_key, _Args&&... __args) {
using _Ret = decltype(__without_key(std::forward<_Args>(__args)...));
- return std::__try_key_extraction_impl<_KeyT, _Ret>(
+ return std::__try_key_extraction_impl<_KeyT, _ValT, _Ret>(
__priority_tag<1>(), __with_key, __without_key, std::forward<_Args>(__args)...);
}
diff --git a/libcxx/test/std/containers/associative/set/emplace.pass.cpp b/libcxx/test/std/containers/associative/set/emplace.pass.cpp
index 55ef489d2a6ba..eb194d440e65b 100644
--- a/libcxx/test/std/containers/associative/set/emplace.pass.cpp
+++ b/libcxx/test/std/containers/associative/set/emplace.pass.cpp
@@ -17,6 +17,7 @@
#include <cassert>
#include <set>
+#include <string>
#include "../../Emplaceable.h"
#include "DefaultOnly.h"
@@ -92,6 +93,15 @@ TEST_CONSTEXPR_CXX26 bool test() {
assert(std::get<1>(res));
assert(set.begin() == std::get<0>(res));
}
+ { // Regression test for #220451
+ // Make sure emplace with multiple arguments doesn't extract the first argument as a key for sets.
+ std::set<std::string> s;
+ s.emplace("foo");
+ auto res = s.emplace(std::string("ofoo"), 1);
+ assert(!res.second);
+ assert(s.size() == 1);
+ assert(*s.begin() == "foo");
+ }
return true;
}
diff --git a/libcxx/test/std/containers/unord/unord.set/emplace.pass.cpp b/libcxx/test/std/containers/unord/unord.set/emplace.pass.cpp
index 24e85a1ab7f54..60a0e85cbe487 100644
--- a/libcxx/test/std/containers/unord/unord.set/emplace.pass.cpp
+++ b/libcxx/test/std/containers/unord/unord.set/emplace.pass.cpp
@@ -18,6 +18,7 @@
// pair<iterator, bool> emplace(Args&&... args);
#include <cassert>
+#include <string>
#include <unordered_set>
#include "../../Emplaceable.h"
@@ -75,6 +76,15 @@ int main(int, char**) {
assert(std::get<1>(res));
assert(set.begin() == std::get<0>(res));
}
+ { // Regression test for #220451
+ // Make sure emplace with multiple arguments doesn't extract the first argument as a key for unordered sets.
+ std::unordered_set<std::string> s;
+ s.emplace("foo");
+ auto res = s.emplace(std::string("ofoo"), 1);
+ assert(!res.second);
+ assert(s.size() == 1);
+ assert(*s.begin() == "foo");
+ }
return 0;
}
>From 1327c0c528eaed507ffdd60d3d078df803670077 Mon Sep 17 00:00:00 2001
From: George Burgess IV <gbiv at google.com>
Date: Wed, 2 Sep 2026 09:46:34 -0600
Subject: [PATCH 2/2] fix CI - std::string isn't sufficiently constexpr
gcc-16 failed with -std=c++26 because `error: non-constant condition for
static assertion`. Try exempting new test from constant evaluation,
since that's not really relevant here
---
libcxx/test/std/containers/associative/set/emplace.pass.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/libcxx/test/std/containers/associative/set/emplace.pass.cpp b/libcxx/test/std/containers/associative/set/emplace.pass.cpp
index eb194d440e65b..cd9fc88860505 100644
--- a/libcxx/test/std/containers/associative/set/emplace.pass.cpp
+++ b/libcxx/test/std/containers/associative/set/emplace.pass.cpp
@@ -93,7 +93,8 @@ TEST_CONSTEXPR_CXX26 bool test() {
assert(std::get<1>(res));
assert(set.begin() == std::get<0>(res));
}
- { // Regression test for #220451
+ if (!TEST_IS_CONSTANT_EVALUATED) {
+ // Regression test for #220451
// Make sure emplace with multiple arguments doesn't extract the first argument as a key for sets.
std::set<std::string> s;
s.emplace("foo");
More information about the libcxx-commits
mailing list