From 0c95d4846f8a2c3727a576af5676d0a628fe6d54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ion=20Gazta=C3=B1aga?= Date: Sat, 22 Jun 2019 10:26:36 +0200 Subject: [PATCH] Fix "count" with heterogeneous lookups in flat_map and flat_set --- doc/container.qbk | 1 + include/boost/container/detail/flat_tree.hpp | 2 +- include/boost/container/flat_map.hpp | 8 +++++++- include/boost/container/flat_set.hpp | 8 +++++++- test/flat_map_test.cpp | 7 ++++++- test/flat_set_test.cpp | 6 +++++- 6 files changed, 27 insertions(+), 5 deletions(-) diff --git a/doc/container.qbk b/doc/container.qbk index a6e19a8..644f510 100644 --- a/doc/container.qbk +++ b/doc/container.qbk @@ -1333,6 +1333,7 @@ use [*Boost.Container]? There are several reasons for that: * [@https://github.com/boostorg/container/issues/117 GitHub #117: ['"flat_map/map::insert_or_assign with hint has wrong return types"]]. * [@https://github.com/boostorg/container/issues/118 GitHub #118: ['"Non-unique inplace_set_difference used in in flat_tree_merge_unique and iterator invalidation in insert_unique"]]. * [@https://github.com/boostorg/container/issues/122 GitHub #122: ['"Fix has_trivial_destructor_after_move"]]. + * [@https://github.com/boostorg/container/issues/123 GitHub #123: ['"With heterogeneous lookup, `equal_range` can result in a range with length greater than 1"]]. * [classref boost::container::deque deque] can now have options, using [classref boost::container::deque_options deque_options]. The block size/bytes can be be specified. diff --git a/include/boost/container/detail/flat_tree.hpp b/include/boost/container/detail/flat_tree.hpp index 5d96a21..c346360 100644 --- a/include/boost/container/detail/flat_tree.hpp +++ b/include/boost/container/detail/flat_tree.hpp @@ -1605,7 +1605,7 @@ class flat_tree const Compare &key_cmp = this->m_data.get_comp(); KeyOfValue key_extract; RanIt lb(this->priv_lower_bound(first, last, k)), ub(lb); - if(lb != last && static_cast(!key_cmp(k, key_extract(*lb)))){ + if(lb != last && !key_cmp(k, key_extract(*lb))){ ++ub; } return std::pair(lb, ub); diff --git a/include/boost/container/flat_map.hpp b/include/boost/container/flat_map.hpp index 3a21ea9..a3bcf47 100644 --- a/include/boost/container/flat_map.hpp +++ b/include/boost/container/flat_map.hpp @@ -1343,7 +1343,9 @@ class flat_map //! //! Complexity: log(size())+count(k) BOOST_CONTAINER_FORCEINLINE size_type count(const key_type& x) const - { return static_cast(m_flat_tree.find(x) != m_flat_tree.end()); } + //Don't use find() != end optimization here as transparent comparators with key K might + //return a different range than key_type (which can only return a single element range) + { return m_flat_tree.count(x); } //! Requires: This overload is available only if //! key_compare::is_transparent exists. @@ -1465,6 +1467,8 @@ class flat_map //! Complexity: Logarithmic. template BOOST_CONTAINER_FORCEINLINE std::pair equal_range(const K& x) + //Don't use lower_bound_range optimization here as transparent comparators with key K might + //return a different range than key_type (which can only return a single element range) { return dtl::force_copy >(m_flat_tree.equal_range(x)); } //! Requires: This overload is available only if @@ -1475,6 +1479,8 @@ class flat_map //! Complexity: Logarithmic. template BOOST_CONTAINER_FORCEINLINE std::pair equal_range(const K& x) const + //Don't use lower_bound_range optimization here as transparent comparators with key K might + //return a different range than key_type (which can only return a single element range) { return dtl::force_copy >(m_flat_tree.equal_range(x)); } //! Effects: Extracts the internal sequence container. diff --git a/include/boost/container/flat_set.hpp b/include/boost/container/flat_set.hpp index 2b69e1a..7cb1d5c 100644 --- a/include/boost/container/flat_set.hpp +++ b/include/boost/container/flat_set.hpp @@ -925,7 +925,9 @@ class flat_set //! Complexity: log(size())+count(k) template BOOST_CONTAINER_FORCEINLINE size_type count(const K& x) const - { return static_cast(this->tree_t::find(x) != this->tree_t::cend()); } + //Don't use find() != end optimization here as transparent comparators with key K might + //return a different range than key_type (which can only return a single element range) + { return this->tree_t::count(x); } #if defined(BOOST_CONTAINER_DOXYGEN_INVOKED) @@ -1031,6 +1033,8 @@ class flat_set //! Complexity: Logarithmic template std::pair equal_range(const K& x) + //Don't use lower_bound_range optimization here as transparent comparators with key K might + //return a different range than key_type (which can only return a single element range) { return this->tree_t::equal_range(x); } //! Requires: This overload is available only if @@ -1041,6 +1045,8 @@ class flat_set //! Complexity: Logarithmic template std::pair equal_range(const K& x) const + //Don't use lower_bound_range optimization here as transparent comparators with key K might + //return a different range than key_type (which can only return a single element range) { return this->tree_t::equal_range(x); } #if defined(BOOST_CONTAINER_DOXYGEN_INVOKED) diff --git a/test/flat_map_test.cpp b/test/flat_map_test.cpp index d01598b..f5caff7 100644 --- a/test/flat_map_test.cpp +++ b/test/flat_map_test.cpp @@ -589,7 +589,12 @@ bool test_heterogeneous_lookup_by_partial_key() std::pair const first_0_range = map1.equal_range(0); - return 2 == first_0_range.second - first_0_range.first; + if(2 != (first_0_range.second - first_0_range.first)) + return false; + + if(2 != map1.count(0)) + return false; + return true; } }}} //namespace boost::container::test diff --git a/test/flat_set_test.cpp b/test/flat_set_test.cpp index 693305f..7927b99 100644 --- a/test/flat_set_test.cpp +++ b/test/flat_set_test.cpp @@ -604,8 +604,12 @@ bool test_heterogeneous_lookup_by_partial_key() set1.insert(std::pair(0, 2)); std::pair const first_0_range = set1.equal_range(0); + if(2 != (first_0_range.second - first_0_range.first)) + return false; - return 2 == first_0_range.second - first_0_range.first; + if(2 != set1.count(0)) + return false; + return true; } }}}