From 1b3d226547c87d4ee744523528e12c8b3a56598f Mon Sep 17 00:00:00 2001 From: Jesse Li Date: Mon, 3 Oct 2022 11:08:08 -0700 Subject: [PATCH] Fix non-usage of key equal function (#606) Co-authored-by: Jesse --- include/etl/unordered_map.h | 2 +- include/etl/unordered_multimap.h | 6 +-- include/etl/unordered_multiset.h | 6 +-- include/etl/unordered_set.h | 2 +- test/test_unordered_map.cpp | 28 ++++++------- test/test_unordered_multimap.cpp | 57 ++++++++++++++++++++++++++- test/test_unordered_multiset.cpp | 57 ++++++++++++++++++++++++++- test/test_unordered_set.cpp | 67 ++++++++++++++++++++++++++++---- 8 files changed, 192 insertions(+), 33 deletions(-) diff --git a/include/etl/unordered_map.h b/include/etl/unordered_map.h index bc0ff25a..705dc4c9 100644 --- a/include/etl/unordered_map.h +++ b/include/etl/unordered_map.h @@ -967,7 +967,7 @@ namespace etl local_iterator icurrent = bucket.begin(); // Search for the key, if we have it. - while ((icurrent != bucket.end()) && (icurrent->key_value_pair.first != key)) + while ((icurrent != bucket.end()) && (!key_equal_function(icurrent->key_value_pair.first, key))) { ++iprevious; ++icurrent; diff --git a/include/etl/unordered_multimap.h b/include/etl/unordered_multimap.h index 26e9bf2e..9775db70 100644 --- a/include/etl/unordered_multimap.h +++ b/include/etl/unordered_multimap.h @@ -983,7 +983,7 @@ namespace etl ++l; ++n; - while ((l != end()) && (key == l->first)) + while ((l != end()) && key_equal_function(key, l->first)) { ++l; ++n; @@ -1078,7 +1078,7 @@ namespace etl { ++l; - while ((l != end()) && (key == l->first)) + while ((l != end()) && key_equal_function(key, l->first)) { ++l; } @@ -1104,7 +1104,7 @@ namespace etl { ++l; - while ((l != end()) && (key == l->first)) + while ((l != end()) && key_equal_function(key, l->first)) { ++l; } diff --git a/include/etl/unordered_multiset.h b/include/etl/unordered_multiset.h index e2470a18..1821d6e8 100644 --- a/include/etl/unordered_multiset.h +++ b/include/etl/unordered_multiset.h @@ -964,7 +964,7 @@ namespace etl ++l; ++n; - while ((l != end()) && (key == *l)) + while ((l != end()) && key_equal_function(key, *l)) { ++l; ++n; @@ -1059,7 +1059,7 @@ namespace etl { ++l; - while ((l != end()) && (key == *l)) + while ((l != end()) && key_equal_function(key, *l)) { ++l; } @@ -1085,7 +1085,7 @@ namespace etl { ++l; - while ((l != end()) && (key == *l)) + while ((l != end()) && key_equal_function(key, *l)) { ++l; } diff --git a/include/etl/unordered_set.h b/include/etl/unordered_set.h index ca917b0a..3a729c66 100644 --- a/include/etl/unordered_set.h +++ b/include/etl/unordered_set.h @@ -846,7 +846,7 @@ namespace etl local_iterator icurrent = bucket.begin(); // Search for the key, if we have it. - while ((icurrent != bucket.end()) && (icurrent->key != key)) + while ((icurrent != bucket.end()) && (!key_equal_function(icurrent->key, key))) { ++iprevious; ++icurrent; diff --git a/test/test_unordered_map.cpp b/test/test_unordered_map.cpp index 1787c4b6..4e9c347a 100644 --- a/test/test_unordered_map.cpp +++ b/test/test_unordered_map.cpp @@ -969,11 +969,11 @@ namespace {17, "17"}, {14, "14"}, { 3, "3"}, { 7, "7"}, { 2, "2"}, { 6, "6"}, { 9, "9"}, { 3, "3"}, {18, "18"}, {10, "10"}, { 8, "8"}, {11, "11"}, { 4, "4"}, { 1, "1"}, {12, "12"}, - {15, "15"}, {16, "16"}, { 0, "0"}, { 5, "5"}, {19, "19"} + {15, "15"}, {16, "16"}, { 0, "0"}, { 5, "5"}, {19, "19"} }; std::vector random_keys2 = - { + { { 3, "3"}, { 6, "6"}, { 5, "5"}, {17, "17"}, { 2, "2"}, { 7, "7"}, { 3, "3"}, {19, "19"}, { 8, "8"}, {15, "15"}, {14, "14"}, { 0, "0"}, {18, "18"}, { 4, "4"}, {10, "10"}, @@ -1051,7 +1051,7 @@ namespace } //************************************************************************* - TEST(test_ndc_hasher_and_key_eq) + TEST(test_ndc_hasher_and_key_eq) { typedef etl::unordered_map Map; ndc_hash hasher1(1); @@ -1093,7 +1093,7 @@ namespace } //************************************************************************* - TEST(test_parameterized_eq) + TEST(test_parameterized_eq) { constexpr std::size_t MODULO = 4; parameterized_hash hash{MODULO}; @@ -1108,12 +1108,14 @@ namespace CHECK_EQUAL(map.at(10), 3); CHECK_EQUAL(constmap.at(10), 3); - CHECK_FALSE(map.insert(etl::make_pair(6, 7)).second); + const std::pair keyvaluepair{6, 7}; + CHECK_FALSE(map.insert(keyvaluepair).second); + CHECK_FALSE(map.insert(std::move(keyvaluepair)).second); CHECK(map.find(14) != map.end()); CHECK(constmap.find(14) != constmap.end()); - map.erase(2); + map.erase(14); CHECK(map.find(6) == map.end()); } @@ -1155,13 +1157,13 @@ namespace CustomKeyEq ceq2(2); using value_type = etl::unordered_map::value_type; - std::array data = - { - value_type{1, 11}, - value_type{2, 22}, - value_type{3, 33}, - value_type{4, 44}, - value_type{5, 55} + std::array data = + { + value_type{1, 11}, + value_type{2, 22}, + value_type{3, 33}, + value_type{4, 44}, + value_type{5, 55} }; etl::unordered_map map1(data.begin(), data.end(), chf1, ceq2); diff --git a/test/test_unordered_multimap.cpp b/test/test_unordered_multimap.cpp index f05460b5..ac339e7d 100644 --- a/test/test_unordered_multimap.cpp +++ b/test/test_unordered_multimap.cpp @@ -133,6 +133,35 @@ namespace int id; }; + //************************************************************************* + // Hasher whose hash behaviour depends on provided data. + struct parameterized_hash + { + size_t modulus; + + parameterized_hash(size_t modulus_ = 2) : modulus(modulus_){} + + size_t operator()(size_t val) const + { + return val % modulus; + } + }; + + //************************************************************************* + // Equality checker whose behaviour depends on provided data. + struct parameterized_equal + { + size_t modulus; + + // Hasher whose hash behaviour depends on provided data. + parameterized_equal(size_t modulus_ = 2) : modulus(modulus_){} + + bool operator()(size_t lhs, size_t rhs) const + { + return (lhs % modulus) == (rhs % modulus); + } + }; + SUITE(test_unordered_multimap) { static const size_t SIZE = 10; @@ -362,9 +391,9 @@ namespace DataNDC data(initial_data.begin(), initial_data.end()); DataNDC other_data(data); -#include "etl/private/diagnostic_self_assign_overloaded_push.h" +#include "etl/private/diagnostic_self_assign_overloaded_push.h" other_data = other_data; -#include "etl/private/diagnostic_pop.h" +#include "etl/private/diagnostic_pop.h" CHECK(data == other_data); } @@ -999,5 +1028,29 @@ namespace using Map = etl::unordered_multimap; CHECK((!std::is_same_v)); } + + TEST(test_parameterized_eq) + { + constexpr std::size_t MODULO = 4; + parameterized_hash hash{MODULO}; + parameterized_equal eq{MODULO}; + // values are equal modulo 4 + etl::unordered_multimap map; + map.insert(etl::make_pair(2, 20)); + map.insert(etl::make_pair(6, 21)); + map.insert(etl::make_pair(10, 22)); + + const auto& constmap = map; + + CHECK_EQUAL(constmap.count(6), 3); + { + auto range = map.equal_range(6); + CHECK_EQUAL(std::distance(range.first, range.second), 3); + } + { + auto range = constmap.equal_range(6); + CHECK_EQUAL(std::distance(range.first, range.second), 3); + } + } }; } diff --git a/test/test_unordered_multiset.cpp b/test/test_unordered_multiset.cpp index 524427b5..5c9d6d23 100644 --- a/test/test_unordered_multiset.cpp +++ b/test/test_unordered_multiset.cpp @@ -106,6 +106,35 @@ namespace int id; }; + //************************************************************************* + // Hasher whose hash behaviour depends on provided data. + struct parameterized_hash + { + size_t modulus; + + parameterized_hash(size_t modulus_ = 2) : modulus(modulus_){} + + size_t operator()(size_t val) const + { + return val % modulus; + } + }; + + //************************************************************************* + // Equality checker whose behaviour depends on provided data. + struct parameterized_equal + { + size_t modulus; + + // Hasher whose hash behaviour depends on provided data. + parameterized_equal(size_t modulus_ = 2) : modulus(modulus_){} + + bool operator()(size_t lhs, size_t rhs) const + { + return (lhs % modulus) == (rhs % modulus); + } + }; + SUITE(test_unordered_multiset) { static const size_t SIZE = 10; @@ -306,9 +335,9 @@ namespace DataNDC data(initial_data.begin(), initial_data.end()); DataNDC other_data(data); -#include "etl/private/diagnostic_self_assign_overloaded_push.h" +#include "etl/private/diagnostic_self_assign_overloaded_push.h" other_data = other_data; -#include "etl/private/diagnostic_pop.h" +#include "etl/private/diagnostic_pop.h" CHECK(data == other_data); } @@ -875,5 +904,29 @@ namespace using Set = etl::unordered_multiset; CHECK((!std::is_same_v)); } + + TEST(test_parameterized_eq) + { + constexpr std::size_t MODULO = 4; + parameterized_hash hash{MODULO}; + parameterized_equal eq{MODULO}; + // values are equal modulo 4 + etl::unordered_multiset set; + set.insert(2); + set.insert(6); + set.insert(10); + + const auto& constset = set; + + CHECK_EQUAL(constset.count(6), 3); + { + auto range = set.equal_range(6); + CHECK_EQUAL(std::distance(range.first, range.second), 3); + } + { + auto range = constset.equal_range(6); + CHECK_EQUAL(std::distance(range.first, range.second), 3); + } + } }; } diff --git a/test/test_unordered_set.cpp b/test/test_unordered_set.cpp index b5a1a246..d560585a 100644 --- a/test/test_unordered_set.cpp +++ b/test/test_unordered_set.cpp @@ -106,6 +106,35 @@ namespace int id; }; + //************************************************************************* + // Hasher whose hash behaviour depends on provided data. + struct parameterized_hash + { + size_t modulus; + + parameterized_hash(size_t modulus_ = 2) : modulus(modulus_){} + + size_t operator()(size_t val) const + { + return val % modulus; + } + }; + + //************************************************************************* + // Equality checker whose behaviour depends on provided data. + struct parameterized_equal + { + size_t modulus; + + // Hasher whose hash behaviour depends on provided data. + parameterized_equal(size_t modulus_ = 2) : modulus(modulus_){} + + bool operator()(size_t lhs, size_t rhs) const + { + return (lhs % modulus) == (rhs % modulus); + } + }; + //*************************************************************************** SUITE(test_unordered_set) { @@ -290,9 +319,9 @@ namespace DataNDC data(initial_data.begin(), initial_data.end()); DataNDC other_data(data); -#include "etl/private/diagnostic_self_assign_overloaded_push.h" +#include "etl/private/diagnostic_self_assign_overloaded_push.h" other_data = other_data; -#include "etl/private/diagnostic_pop.h" +#include "etl/private/diagnostic_pop.h" CHECK(data == other_data); } @@ -696,18 +725,40 @@ namespace CHECK_EQUAL("set = 3", s[1]); } + TEST(test_parameterized_eq) + { + constexpr std::size_t MODULO = 4; + parameterized_hash hash{MODULO}; + parameterized_equal eq{MODULO}; + // values are equal modulo 4 + etl::unordered_set set; + set.insert(2); + + const auto& constset = set; + + const size_t key = 6; + CHECK_FALSE(set.insert(key).second); + CHECK_FALSE(set.insert(std::move(key)).second); + + CHECK(set.find(14) != set.end()); + CHECK(constset.find(14) != constset.end()); + + set.erase(14); + CHECK(set.find(6) == set.end()); + } + //************************************************************************* TEST(test_equality_comparison_fails_when_hash_collisions_occur_582) { struct bad_hash { // Force hash collisions - size_t operator()(int key) const - { - return key % 4; + size_t operator()(int key) const + { + return key % 4; } }; - + std::vector random_keys1 = { 17, 14, 3, 7, 2, 6, 9, 3, 18, 10, 8, 11, 4, 1, 12, 15, 16, 0, 5, 19 }; std::vector random_keys2 = { 3, 6, 5, 17, 2, 7, 3, 19, 8, 15, 14, 0, 18, 4, 10, 9, 16, 11, 12, 1 }; @@ -748,7 +799,7 @@ namespace //*************************************************** CHECK_EQUAL((stdset1 == stdset2), (etlset1 == etlset2)); } - + //************************************************************************* TEST(test_copying_of_hash_and_key_compare_with_copy_construct) { @@ -773,7 +824,7 @@ namespace etl::unordered_set set1(chf1, ceq2); etl::unordered_set set2(chf3, ceq4); - + set2.operator=(set1); CHECK_EQUAL(chf1.id, set2.hash_function().id);