Equality for unordered_set and unordered_map should be based on equality of elements (#4406)

Co-authored-by: Matt Stephanson <68978048+MattStephanson@users.noreply.github.com>
Co-authored-by: Stephan T. Lavavej <stl@nuwen.net>

stl/inc/hash_map

@@ -85,12 +85,6 @@ namespace stdext {
             return _Val.first;
         }
 
-        template <class _Ty1, class _Ty2>
-        _NODISCARD static const _Ty2& _Nonkfn(const pair<_Ty1, _Ty2>& _Val) noexcept {
-            // extract non-key from element value
-            return _Val.second;
-        }
-
         _NODISCARD float& _Get_max_bucket_size() noexcept {
             return _Max_buckets;
         }

stl/inc/hash_set

@@ -63,11 +63,6 @@ namespace stdext {
             return _Val;
         }
 
-        _NODISCARD static int _Nonkfn(const value_type&) noexcept {
-            // extract "non-key" from element value (for container equality)
-            return 0;
-        }
-
         _NODISCARD float& _Get_max_bucket_size() noexcept {
             return _Max_buckets;
         }

stl/inc/unordered_map

@@ -54,11 +54,6 @@ public:
     static const _Kty& _Kfn(const pair<_Ty1, _Ty2>& _Val) noexcept { // extract key from element value
         return _Val.first;
     }
-
-    template <class _Ty1, class _Ty2>
-    static const _Ty2& _Nonkfn(const pair<_Ty1, _Ty2>& _Val) noexcept { // extract non-key from element value
-        return _Val.second;
-    }
 };
 
 _EXPORT_STD template <class _Kty, class _Ty, class _Hasher = hash<_Kty>, class _Keyeq = equal_to<_Kty>,

stl/inc/unordered_set

@@ -52,10 +52,6 @@ public:
     static const _Kty& _Kfn(const value_type& _Val) noexcept {
         return _Val;
     }
-
-    static int _Nonkfn(const value_type&) noexcept { // extract "non-key" from element value (for container equality)
-        return 0;
-    }
 };
 
 _EXPORT_STD template <class _Kty, class _Hasher = hash<_Kty>, class _Keyeq = equal_to<_Kty>,

stl/inc/xhash

@@ -2015,7 +2015,7 @@ _NODISCARD bool _Hash_equal(const _Hash<_Traits>& _Left, const _Hash<_Traits>& _
             // look for element with equivalent key
             const auto& _Keyval = _Traits::_Kfn(_LVal);
             const auto _Next2   = _Right._Find_last(_Keyval, _Right._Traitsobj(_Keyval))._Duplicate;
-            if (!(static_cast<bool>(_Next2) && _Traits::_Nonkfn(_LVal) == _Traits::_Nonkfn(_Next2->_Myval))) {
+            if (!(static_cast<bool>(_Next2) && _LVal == _Next2->_Myval)) {
                 return false;
             }
         }

tests/std/test.lst

@@ -239,6 +239,7 @@ tests\GH_004040_container_nonmember_functions
 tests\GH_004109_iter_value_t_direct_initialization
 tests\GH_004201_chrono_formatter
 tests\GH_004275_seeking_fancy_iterators
+tests\GH_004388_unordered_meow_operator_equal
 tests\LWG2381_num_get_floating_point
 tests\LWG2597_complex_branch_cut
 tests\LWG3018_shared_ptr_function

tests/std/tests/GH_004388_unordered_meow_operator_equal/env.lst

@@ -0,0 +1,4 @@
+# Copyright (c) Microsoft Corporation.
+# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+RUNALL_INCLUDE ..\usual_matrix.lst

tests/std/tests/GH_004388_unordered_meow_operator_equal/test.cpp

@@ -0,0 +1,122 @@
+// Copyright (c) Microsoft Corporation.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+#include <algorithm>
+#include <cassert>
+#include <cstddef>
+#include <map>
+#include <set>
+#include <unordered_map>
+#include <unordered_set>
+#include <utility>
+
+using namespace std;
+
+// Functors for comparison and hashing
+// based on modular arithmetic equivalence classes
+template <int N>
+struct ModLess {
+    bool operator()(int lhs, int rhs) const {
+        return (lhs % N) < (rhs % N);
+    }
+};
+
+template <int N>
+struct ModEqual {
+    bool operator()(int lhs, int rhs) const {
+        return (lhs % N) == (rhs % N);
+    }
+};
+
+template <int N>
+struct ModHash {
+    size_t operator()(int value) const {
+        return static_cast<size_t>(value % N);
+    }
+};
+
+// Overloaded function to get a key from a set / map's value_type
+const int& get_key(const int& value) {
+    return value;
+}
+const int& get_key(const pair<const int, int>& p) {
+    return p.first;
+}
+
+// Equality comparison for unordered sets and maps as per the C++ standard
+// It also gives the correct result for associative containers (though is sub-optimal)
+template <typename Container>
+bool std_equal(const Container& lhs, const Container& rhs) {
+    if (lhs.size() != rhs.size()) {
+        return false;
+    }
+
+    for (auto it = lhs.cbegin(); it != lhs.cend();) {
+        const auto& key    = get_key(*it);
+        const auto l_range = lhs.equal_range(key);
+        const auto r_range = rhs.equal_range(key);
+
+        if (!is_permutation(l_range.first, l_range.second, r_range.first, r_range.second)) {
+            return false;
+        }
+        it = l_range.second;
+    }
+    return true;
+}
+
+template <typename Map>
+void test_maps() {
+    // These maps should compare not equal, even though their elements are equivalent
+    Map map1 = {{1, 1}, {2, 2}};
+    Map map2 = {{21, 1}, {12, 2}};
+
+    assert(map1 != map2);
+    assert(!std_equal(map1, map2));
+
+    // Test a case that should compare equal
+    Map map3 = {{12, 2}, {21, 1}};
+    assert(map2 == map3);
+    assert(std_equal(map2, map3));
+}
+
+template <typename Set>
+void test_sets() {
+    // These sets should compare not equal, even though their elements are equivalent
+    Set set1 = {1, 2};
+    Set set2 = {21, 12};
+
+    assert(set1 != set2);
+    assert(!std_equal(set1, set2));
+
+    // Test a case that should compare equal
+    Set set3 = {12, 21};
+    assert(set2 == set3);
+    assert(std_equal(set2, set3));
+}
+
+// GH-4388: <unordered_set>, <unordered_map>: operator== is incorrect with custom equivalence functor
+void test_gh_4388() {
+    using Set               = set<int, ModLess<10>>;
+    using MultiSet          = multiset<int, ModLess<10>>;
+    using UnorderedSet      = unordered_set<int, ModHash<10>, ModEqual<10>>;
+    using UnorderedMultiSet = unordered_multiset<int, ModHash<10>, ModEqual<10>>;
+
+    test_sets<Set>();
+    test_sets<MultiSet>();
+    test_sets<UnorderedSet>();
+    test_sets<UnorderedMultiSet>();
+
+    using Map               = map<int, int, ModLess<10>>;
+    using MultiMap          = multimap<int, int, ModLess<10>>;
+    using UnorderedMap      = unordered_map<int, int, ModHash<10>, ModEqual<10>>;
+    using UnorderedMultiMap = unordered_multimap<int, int, ModHash<10>, ModEqual<10>>;
+
+    test_maps<Map>();
+    test_maps<MultiMap>();
+    test_maps<UnorderedMap>();
+    test_maps<UnorderedMultiMap>();
+}
+
+int main() {
+    test_gh_4388();
+}