Skip to content

<unordered_set>, <unordered_map>: operator== is incorrect with custom equivalence functor #4388

@sibecker

Description

@sibecker

Describe the bug

According to the C++ standard, two unordered_sets should only compare equal if they have elements that compare equal per value_type’s operator==, regardless of the key_equal equivalence functor. When using unordered_set with a custom key_equal equivalence functor (and compatible hasher functor), operator== erroneously returns true when the sets contain values that are equivalent but not equal.

The same issue occurs for unordered_map, but not for unordered_multimap, unordered_multiset or any of the associative containers.

See also https://godbolt.org/z/99Mnf9hqv

Command-line test case

C:\temp>type unordered.cpp
#include <algorithm>
#include <cassert>
#include <map>
#include <set>
#include <unordered_map>
#include <unordered_set>


namespace modulo
{
// Functors for comparison and hashing
// based on modular arithmetic equivalence classes
template<int N>
struct compare
{
    bool operator()(int lhs, int rhs) const
    {
        return (lhs % N) < (rhs % N);
    }
};

template<int N>
struct equals
{
    bool operator()(int lhs, int rhs) const
    {
        return (lhs % N) == (rhs % N);
    }
};

template<int N>
struct hash
{
    std::size_t operator()(int value) const
    {
        return value % N;
    }
};
}

// Overloaded function to get a key from a set / map's value_type
int const& get_key(int const& value)
{
    return value;
}
int const& get_key(std::pair<int const, int> const& pair)
{
    return pair.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(Container const& lhs, Container const& rhs)
{
    if (lhs.size() != rhs.size())
        return false;

    for(auto it = lhs.cbegin(); it != lhs.cend(); ) {
        auto const& key = get_key(*it);
        auto l_range = lhs.equal_range(key);
        auto r_range = rhs.equal_range(key);

        if (!std::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>
bool test_maps()
{
    // In all cases, these sets should compare not equal, even though their elements are equivalent
    Map map1 = {{1, 1}, {2, 2}};
    Map map2 = {{21, 1}, {12, 2}};

    return (map1 == map2) == std_equal(map1, map2);
}

template<typename Set>
bool test_sets()
{
    // In all cases, these sets should compare not equal, even though their elements are equivalent
    Set set1 = {1, 2};
    Set set2 = {21, 12};

    return (set1 == set2) == std_equal(set1, set2);
}

int main()
{
    using Set = std::set<int, modulo::compare<10>>;
    using MultiSet = std::multiset<int, modulo::compare<10>>;
    using UnorderedSet = std::unordered_set<int, modulo::hash<10>, modulo::equals<10>>;
    using UnorderedMultiSet = std::unordered_multiset<int, modulo::hash<10>, modulo::equals<10>>;

    assert(test_sets<Set>()); // ok
    assert(test_sets<MultiSet>()); // ok
    assert(test_sets<UnorderedSet>()); // FAILURE!
    assert(test_sets<UnorderedMultiSet>()); // ok

    using Map = std::map<int, int, modulo::compare<10>>;
    using MultiMap = std::multimap<int, int, modulo::compare<10>>;
    using UnorderedMap = std::unordered_map<int, int, modulo::hash<10>, modulo::equals<10>>;
    using UnorderedMultiMap = std::unordered_multimap<int, int, modulo::hash<10>, modulo::equals<10>>;

    assert(test_maps<Map>()); // ok
    assert(test_maps<MultiMap>()); // ok
    assert(test_maps<UnorderedMap>()); // FAILURE!
    assert(test_maps<UnorderedMultiMap>()); // ok

    return 0;
}

C:\temp>cl /EHsc /W4 /WX /std:c++latest .\unordered.cpp
Microsoft (R) C/C++ Optimizing Compiler Version 19.38.33134 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

/std:c++latest is provided as a preview of language features from the latest C++
working draft, and we're eager to hear about bugs and suggestions for improvements.
However, note that these features are provided as-is without support, and subject
to changes or removal as the working draft evolves. See
https://go.microsoft.com/fwlink/?linkid=2045807 for details.

unordered.cpp
Microsoft (R) Incremental Linker Version 14.38.33134.0
Copyright (C) Microsoft Corporation.  All rights reserved.

/out:unordered.exe
unordered.obj

C:\temp>.\unordered.exe
Assertion failed: test_sets<UnorderedSet>(), file .\unordered.cpp, line 105

Expected behavior

Equality for unordered_set and unordered_map should be based on equality of elements, not just equivalence, just as it is for the other containers.

STL version

  • Option 1: Visual Studio version
    Microsoft Visual Studio Community 2022 (64-bit) - Current
    Version 17.8.4

Additional context

I have previously raised this issue at DevCom-1392782 (internal VSO-1307889 / AB#1307889 ), but didn't seem to get much traction there. See also https://godbolt.org/z/99Mnf9hqv

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingfixedSomething works now, yay!

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions