Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

<xhash>: Get rid of tricks for transparent overloads of hash containers #5208

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions stl/inc/hash_map
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,10 @@ namespace stdext {

template <class... _Args>
using _In_place_key_extractor = _STD _In_place_key_extract_map<_Kty, _Args...>;
template <class>
using _Deduce_key = const _Kty&;
using key_equal = _Tr;

using key_equal = _Tr;

static constexpr bool _Has_transparent_overloads = false;

#if _HAS_CXX20 && defined(__EDG__) // TRANSITION, DevCom-10678753
template <class, class>
Expand Down
7 changes: 4 additions & 3 deletions stl/inc/hash_set
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,10 @@ namespace stdext {

template <class... _Args>
using _In_place_key_extractor = _STD _In_place_key_extract_set<_Kty, _Args...>;
template <class>
using _Deduce_key = const _Kty&;
using key_equal = _Tr;

using key_equal = _Tr;

static constexpr bool _Has_transparent_overloads = false;

#if _HAS_CXX20 && defined(__EDG__) // TRANSITION, DevCom-10678753
template <class, class>
Expand Down
78 changes: 61 additions & 17 deletions stl/inc/xhash
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,7 @@ _STD_BEGIN
template <class _Kty, class _Hasher, class _Keyeq>
struct _Uhash_choose_transparency {
// transparency selector for non-transparent hashed containers
template <class>
using _Deduce_key = const _Kty&;
static constexpr bool _Has_transparent_overloads = false;

#if _HAS_CXX20 && defined(__EDG__) // TRANSITION, DevCom-10678753
template <class, class>
Expand All @@ -110,8 +109,7 @@ template <class _Kty, class _Hasher, class _Keyeq>
requires _Is_transparent_v<_Hasher> && _Is_transparent_v<_Keyeq>
struct _Uhash_choose_transparency<_Kty, _Hasher, _Keyeq> {
// transparency selector for transparent hashed containers
template <class _Keyty>
using _Deduce_key = const _Keyty&;
static constexpr bool _Has_transparent_overloads = true;

template <class _Container, class _Kx>
static constexpr bool _Supports_transparency =
Expand Down Expand Up @@ -1225,25 +1223,43 @@ private:
}

public:
template <class _Keyty = void>
_NODISCARD iterator find(typename _Traits::template _Deduce_key<_Keyty> _Keyval) {
_NODISCARD iterator find(const key_type& _Keyval) {
return _List._Make_iter(_Find(_Keyval, _Traitsobj(_Keyval)));
}

#if _HAS_CXX20
template <class _KeyTy>
requires _Traits::_Has_transparent_overloads
_NODISCARD iterator find(const _KeyTy& _Keyval) {
return _List._Make_iter(_Find(_Keyval, _Traitsobj(_Keyval)));
}
#endif // _HAS_CXX20

template <class _Keyty = void>
_NODISCARD const_iterator find(typename _Traits::template _Deduce_key<_Keyty> _Keyval) const {
_NODISCARD const_iterator find(const key_type& _Keyval) const {
return _List._Make_const_iter(_Find(_Keyval, _Traitsobj(_Keyval)));
}

#if _HAS_CXX20
template <class _Keyty = void>
_NODISCARD bool contains(_Traits::template _Deduce_key<_Keyty> _Keyval) const {
template <class _KeyTy>
requires _Traits::_Has_transparent_overloads
_NODISCARD const_iterator find(const _KeyTy& _Keyval) const {
return _List._Make_const_iter(_Find(_Keyval, _Traitsobj(_Keyval)));
}
#endif // _HAS_CXX20

#if _HAS_CXX20
_NODISCARD bool contains(const key_type& _Keyval) const {
return static_cast<bool>(_Find_last(_Keyval, _Traitsobj(_Keyval))._Duplicate);
}

template <class _KeyTy>
requires _Traits::_Has_transparent_overloads
_NODISCARD bool contains(const _KeyTy& _Keyval) const {
return static_cast<bool>(_Find_last(_Keyval, _Traitsobj(_Keyval))._Duplicate);
}
#endif // _HAS_CXX20

template <class _Keyty = void>
_NODISCARD size_type count(typename _Traits::template _Deduce_key<_Keyty> _Keyval) const {
_NODISCARD size_type count(const key_type& _Keyval) const {
const size_t _Hashval = _Traitsobj(_Keyval);
if constexpr (_Multi) {
return _Equal_range(_Keyval, _Hashval)._Distance;
Expand All @@ -1252,6 +1268,19 @@ public:
}
}

#if _HAS_CXX20
template <class _KeyTy>
requires _Traits::_Has_transparent_overloads
_NODISCARD size_type count(const _KeyTy& _Keyval) const {
const size_t _Hashval = _Traitsobj(_Keyval);
if constexpr (_Multi) {
return _Equal_range(_Keyval, _Hashval)._Distance;
} else {
return static_cast<bool>(_Find_last(_Keyval, _Hashval)._Duplicate);
}
}
#endif // _HAS_CXX20

private:
struct _Equal_range_result {
_Unchecked_const_iterator _First;
Expand Down Expand Up @@ -1307,19 +1336,34 @@ private:
}

public:
template <class _Keyty = void>
_NODISCARD pair<iterator, iterator> equal_range(typename _Traits::template _Deduce_key<_Keyty> _Keyval) {
_NODISCARD pair<iterator, iterator> equal_range(const key_type& _Keyval) {
const auto _Result = _Equal_range(_Keyval, _Traitsobj(_Keyval));
return {_List._Make_iter(_Result._First._Ptr), _List._Make_iter(_Result._Last._Ptr)};
}

#if _HAS_CXX20
template <class _KeyTy>
requires _Traits::_Has_transparent_overloads
_NODISCARD pair<iterator, iterator> equal_range(const _KeyTy& _Keyval) {
const auto _Result = _Equal_range(_Keyval, _Traitsobj(_Keyval));
return {_List._Make_iter(_Result._First._Ptr), _List._Make_iter(_Result._Last._Ptr)};
}
#endif // _HAS_CXX20

template <class _Keyty = void>
_NODISCARD pair<const_iterator, const_iterator> equal_range(
typename _Traits::template _Deduce_key<_Keyty> _Keyval) const {
_NODISCARD pair<const_iterator, const_iterator> equal_range(const key_type& _Keyval) const {
const auto _Result = _Equal_range(_Keyval, _Traitsobj(_Keyval));
return {_List._Make_const_iter(_Result._First._Ptr), _List._Make_const_iter(_Result._Last._Ptr)};
}

#if _HAS_CXX20
template <class _KeyTy>
requires _Traits::_Has_transparent_overloads
_NODISCARD pair<const_iterator, const_iterator> equal_range(const _KeyTy& _Keyval) const {
const auto _Result = _Equal_range(_Keyval, _Traitsobj(_Keyval));
return {_List._Make_const_iter(_Result._First._Ptr), _List._Make_const_iter(_Result._Last._Ptr)};
}
#endif // _HAS_CXX20

void swap(_Hash& _Right) noexcept(noexcept(_Traitsobj.swap(_Right._Traitsobj))) /* strengthened */ {
if (this != _STD addressof(_Right)) {
_Traitsobj.swap(_Right._Traitsobj);
Expand Down
30 changes: 30 additions & 0 deletions tests/std/tests/P0919R3_heterogeneous_unordered_lookup/test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,9 @@ void assert_unique() {
emplace_test_strings(cRawToExtract);
#endif // _HAS_CXX23

// GH-5207 "<xhash>: Some member functions of transparent hash containers fail to work with initializer lists"
typename Container::key_type testNotInStringListSrc(testNotInString.data(), testNotInString.size());

// Test that transparent containers pass through the string_view; non-transparent containers
// are only passed in here with string_view value_type, so they also don't allocate.
[[maybe_unused]] prohibit_allocations prohibitor(true);
Expand All @@ -281,6 +284,18 @@ void assert_unique() {
assert(c.count(testNotInString) == 0);
assert_range_empty(c.equal_range(testNotInString));

assert(c.find({testNotInStringListSrc}) == c.end());
assert(c.contains({testNotInStringListSrc}) == false);
assert(c.count({testNotInStringListSrc}) == 0);
assert_range_empty(c.equal_range({testNotInStringListSrc}));

// Test non-const overloads.
assert(cRaw.find(testNotInString) == cRaw.end());
assert_range_empty(cRaw.equal_range(testNotInString));

assert(cRaw.find({testNotInStringListSrc}) == cRaw.end());
assert_range_empty(cRaw.equal_range({testNotInStringListSrc}));

for (const auto& example : testStrings) {
const auto target = c.find(example);
assert(target != c.end());
Expand Down Expand Up @@ -328,6 +343,9 @@ void assert_multi() {
#endif // _HAS_CXX23
}

// GH-5207 "<xhash>: Some member functions of transparent hash containers fail to work with initializer lists"
typename Container::key_type testNotInStringListSrc(testNotInString.data(), testNotInString.size());

// Test that transparent containers pass through the string_view; non-transparent containers
// are only passed in here with string_view value_type, so they also don't allocate.
[[maybe_unused]] prohibit_allocations prohibitor(true);
Expand All @@ -337,6 +355,18 @@ void assert_multi() {
assert(c.count(testNotInString) == 0);
assert_range_empty(c.equal_range(testNotInString));

assert(c.find({testNotInStringListSrc}) == c.end());
assert(c.contains({testNotInStringListSrc}) == false);
assert(c.count({testNotInStringListSrc}) == 0);
assert_range_empty(c.equal_range({testNotInStringListSrc}));

// Test non-const overloads.
assert(cRaw.find(testNotInString) == cRaw.end());
assert_range_empty(cRaw.equal_range(testNotInString));

assert(cRaw.find({testNotInStringListSrc}) == cRaw.end());
assert_range_empty(cRaw.equal_range({testNotInStringListSrc}));

for (const auto& example : testStrings) {
const auto target = c.find(example);
assert(target != c.end());
Expand Down