Skip to content

Commit

Permalink
Add a comment explaining the extra comparison in raw_hash_set::operat…
Browse files Browse the repository at this point in the history
…or==. Also add a small optimization to avoid the extra comparison in sets that use hash_default_eq as the key_equal functor.

Note that removing the comparison entirely causes the Table.Equality2 test case to fail.

PiperOrigin-RevId: 693447075
Change-Id: I769f75cf1cb27114455f1854e330c899cee55fc2
  • Loading branch information
ezbr authored and copybara-github committed Nov 5, 2024
1 parent 8596c6e commit 4794821
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 1 deletion.
14 changes: 14 additions & 0 deletions absl/container/flat_hash_set_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include <cstddef>
#include <cstdint>
#include <memory>
#include <string>
#include <type_traits>
#include <utility>
#include <vector>
Expand Down Expand Up @@ -337,6 +338,19 @@ TEST(FlatHashSet, MovedFromCleared_EqMustBeValid) {
EXPECT_THAT(s1, UnorderedElementsAre(2));
}

TEST(FlatHashSet, Equality) {
{
flat_hash_set<int> s1 = {1, 2, 3};
flat_hash_set<int> s2 = {1, 2, 3};
EXPECT_EQ(s1, s2);
}
{
flat_hash_set<std::string> s1 = {"a", "b", "c"};
flat_hash_set<std::string> s2 = {"a", "b", "c"};
EXPECT_EQ(s1, s2);
}
}

} // namespace
} // namespace container_internal
ABSL_NAMESPACE_END
Expand Down
11 changes: 10 additions & 1 deletion absl/container/internal/raw_hash_set.h
Original file line number Diff line number Diff line change
Expand Up @@ -3471,7 +3471,16 @@ class raw_hash_set {
if (outer->capacity() > inner->capacity()) std::swap(outer, inner);
for (const value_type& elem : *outer) {
auto it = PolicyTraits::apply(FindElement{*inner}, elem);
if (it == inner->end() || !(*it == elem)) return false;
if (it == inner->end()) return false;
// Note: we used key_equal to check for key equality in FindElement, but
// we may need to do an additional comparison using
// value_type::operator==. E.g. the keys could be equal and the
// mapped_types could be unequal in a map or even in a set, key_equal
// could ignore some fields that aren't ignored by operator==.
static constexpr bool kKeyEqIsValueEq =
std::is_same<key_type, value_type>::value &&
std::is_same<key_equal, hash_default_eq<key_type>>::value;
if (!kKeyEqIsValueEq && !(*it == elem)) return false;
}
return true;
}
Expand Down

0 comments on commit 4794821

Please sign in to comment.