From 1ad2b26ee42dcd88e7967575f72563ad67a74390 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ion=20Gazta=C3=B1aga?= Date: Tue, 7 Nov 2017 00:46:23 +0100 Subject: [PATCH 1/5] Avoid using external reference in loop to avoid aliasing pesimization. --- include/boost/intrusive/hashtable.hpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/include/boost/intrusive/hashtable.hpp b/include/boost/intrusive/hashtable.hpp index 6b1d5dd..58ccfa9 100644 --- a/include/boost/intrusive/hashtable.hpp +++ b/include/boost/intrusive/hashtable.hpp @@ -3370,7 +3370,7 @@ class hashtable_impl , size_type &found_bucket , size_type &cnt) const { - cnt = 0; + size_type internal_cnt = 0; //Let's see if the element is present siterator prev; @@ -3386,20 +3386,21 @@ class hashtable_impl //the same bucket bucket_type &b = this->priv_bucket_pointer()[n_bucket]; siterator it = to_return.first; - ++cnt; //At least one is found + ++internal_cnt; //At least one is found if(optimize_multikey){ to_return.second = ++(priv_last_in_group)(it); - cnt += boost::intrusive::iterator_distance(++it, to_return.second); + internal_cnt += boost::intrusive::iterator_distance(++it, to_return.second); } else{ siterator const bend = b.end(); while(++it != bend && this->priv_is_value_equal_to_key(this->priv_value_from_slist_node(it.pointed_node()), h, key, equal_func)){ - ++cnt; + ++internal_cnt; } to_return.second = it; } } + cnt = internal_cnt; return to_return; } From 4f9e77acfe1d6c78a35032016f7ecb214db56ef4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ion=20Gazta=C3=B1aga?= Date: Tue, 7 Nov 2017 00:46:44 +0100 Subject: [PATCH 2/5] Use intrusive's type traits instead of container's traits --- test/iterator_test.hpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/iterator_test.hpp b/test/iterator_test.hpp index 02f1ddc..486cdd8 100644 --- a/test/iterator_test.hpp +++ b/test/iterator_test.hpp @@ -120,8 +120,8 @@ void test_iterator_compatible(C &c) test_iterator_operations(get_reverse_iterator::begin(c), get_reverse_iterator::end(c)); test_iterator_operations(get_const_reverse_iterator::begin(c), get_const_reverse_iterator::end(c)); //Make sure dangeous conversions are not possible - BOOST_STATIC_ASSERT((!boost::container::container_detail::is_convertible::value)); - BOOST_STATIC_ASSERT((!boost::container::container_detail::is_convertible::value)); + BOOST_STATIC_ASSERT((!boost::intrusive::detail::is_convertible::value)); + BOOST_STATIC_ASSERT((!boost::intrusive::detail::is_convertible::value)); //Test iterator conversions { const_iterator ci; @@ -318,7 +318,7 @@ void test_iterator_forward(C &c) typedef iterator_traits rnit_traits; typedef iterator_traits crit_traits; - using boost::container::container_detail::is_same; + using boost::intrusive::detail::is_same; //iterator_category BOOST_STATIC_ASSERT((is_same::value)); BOOST_STATIC_ASSERT((is_same::value)); @@ -340,7 +340,7 @@ void test_iterator_bidirectional(C &c) typedef iterator_traits rnit_traits; typedef iterator_traits crit_traits; - using boost::container::container_detail::is_same; + using boost::intrusive::detail::is_same; //iterator_category BOOST_STATIC_ASSERT((is_same::value)); BOOST_STATIC_ASSERT((is_same::value)); @@ -362,7 +362,7 @@ void test_iterator_random(C &c) typedef iterator_traits rnit_traits; typedef iterator_traits crit_traits; - using boost::container::container_detail::is_same; + using boost::intrusive::detail::is_same; //iterator_category BOOST_STATIC_ASSERT((is_same::value)); BOOST_STATIC_ASSERT((is_same::value)); From 71317671eccb460ab33dee42f75c776310f7c3bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ion=20Gazta=C3=B1aga?= Date: Sat, 9 Dec 2017 13:01:02 +0100 Subject: [PATCH 3/5] Add GCC's "-Wuninitialized" workaround. When initializing the pointer of flags part of an uninitialized pointer plus bit object, the other part is used uninitialized. It's harmless warning but annoying for the user. --- doc/intrusive.qbk | 7 +++++++ include/boost/intrusive/pointer_plus_bits.hpp | 15 +++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/doc/intrusive.qbk b/doc/intrusive.qbk index fbd6a93..cb76699 100644 --- a/doc/intrusive.qbk +++ b/doc/intrusive.qbk @@ -3870,6 +3870,13 @@ to be inserted in intrusive containers are allocated using `std::vector` or `std [section:release_notes Release Notes] +[section:release_notes_boost_1_67_00 Boost 1.67 Release] + +* Fixed bugs: + * [@https://github.com/boostorg/intrusive/issues/29 GitHub Issues #29: ['Uninitialized variable warning pointer_plus_bits.hpp]] + +[endsect] + [section:release_notes_boost_1_65_00 Boost 1.65 Release] * Fixed bugs: diff --git a/include/boost/intrusive/pointer_plus_bits.hpp b/include/boost/intrusive/pointer_plus_bits.hpp index dfde66b..a39da32 100644 --- a/include/boost/intrusive/pointer_plus_bits.hpp +++ b/include/boost/intrusive/pointer_plus_bits.hpp @@ -22,6 +22,17 @@ # pragma once #endif + +//GCC reports uninitialized values when an uninitialized pointer plus bits type +//is asigned some bits or some pointer value, but that's ok, because we don't want +//to default initialize parts that are not being updated. +#if defined(BOOST_GCC) +# if (BOOST_GCC >= 40600) +# pragma GCC diagnostic push +# pragma GCC diagnostic ignored "-Wuninitialized" +# endif +#endif + namespace boost { namespace intrusive { @@ -89,6 +100,10 @@ struct pointer_plus_bits } //namespace intrusive } //namespace boost +#if defined(BOOST_GCC) && (BOOST_GCC >= 40600) +# pragma GCC diagnostic pop +#endif + #include #endif //BOOST_INTRUSIVE_POINTER_PLUS_BITS_HPP From 53b1aef2da71406a2041d716a15af9f67a940de5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ion=20Gazta=C3=B1aga?= Date: Sat, 9 Dec 2017 13:01:49 +0100 Subject: [PATCH 4/5] Refactor suggested_upper_bucket_count/suggested_lower_bucket_count to be independent of the hashtable. --- include/boost/intrusive/hashtable.hpp | 84 +++++++++++++++++++++++---- 1 file changed, 72 insertions(+), 12 deletions(-) diff --git a/include/boost/intrusive/hashtable.hpp b/include/boost/intrusive/hashtable.hpp index 58ccfa9..258e601 100644 --- a/include/boost/intrusive/hashtable.hpp +++ b/include/boost/intrusive/hashtable.hpp @@ -115,26 +115,89 @@ bool priv_algo_is_permutation(ForwardIterator1 first1, ForwardIterator1 last1, F template struct prime_list_holder { + private: + + template // sizeof(SizeType) < sizeof(std::size_t) + static BOOST_INTRUSIVE_FORCEINLINE SizeType truncate_size_type(std::size_t n, detail::true_) + { + return n < std::size_t(SizeType(-1)) ? static_cast(n) : SizeType(-1); + } + + template // sizeof(SizeType) == sizeof(std::size_t) + static BOOST_INTRUSIVE_FORCEINLINE SizeType truncate_size_type(std::size_t n, detail::false_) + { + return static_cast(n); + } + + template //sizeof(SizeType) > sizeof(std::size_t) + static BOOST_INTRUSIVE_FORCEINLINE SizeType suggested_upper_bucket_count_dispatch(SizeType n, detail::true_) + { + std::size_t const c = n > std::size_t(-1) + ? std::size_t(-1) + : suggested_upper_bucket_count_impl(static_cast(n)); + return static_cast(c); + } + + template //sizeof(SizeType) > sizeof(std::size_t) + static BOOST_INTRUSIVE_FORCEINLINE SizeType suggested_lower_bucket_count_dispatch(SizeType n, detail::true_) + { + std::size_t const c = n > std::size_t(-1) + ? std::size_t(-1) + : suggested_lower_bucket_count_impl(static_cast(n)); + return static_cast(c); + } + + template + static BOOST_INTRUSIVE_FORCEINLINE SizeType suggested_upper_bucket_count_dispatch(SizeType n, detail::false_) + { + std::size_t const c = suggested_upper_bucket_count_impl(static_cast(n)); + return truncate_size_type(c, detail::bool_<(sizeof(SizeType) < sizeof(std::size_t))>()); + + } + + template + static BOOST_INTRUSIVE_FORCEINLINE SizeType suggested_lower_bucket_count_dispatch(SizeType n, detail::false_) + { + std::size_t const c = suggested_lower_bucket_count_impl(static_cast(n)); + return truncate_size_type(c, detail::bool_<(sizeof(SizeType) < sizeof(std::size_t))>()); + } + static const std::size_t prime_list[]; static const std::size_t prime_list_size; - static std::size_t suggested_upper_bucket_count(std::size_t n) + static std::size_t suggested_lower_bucket_count_impl(std::size_t n) { const std::size_t *primes = &prime_list_holder<0>::prime_list[0]; const std::size_t *primes_end = primes + prime_list_holder<0>::prime_list_size; std::size_t const* bound = std::lower_bound(primes, primes_end, n); - bound -= (bound == primes_end); + //Tables have upper SIZE_MAX, so we must always found an entry + BOOST_INTRUSIVE_INVARIANT_ASSERT(bound != primes_end); + bound -= std::size_t(bound != primes); return *bound; } - static std::size_t suggested_lower_bucket_count(std::size_t n) + static std::size_t suggested_upper_bucket_count_impl(std::size_t n) { const std::size_t *primes = &prime_list_holder<0>::prime_list[0]; const std::size_t *primes_end = primes + prime_list_holder<0>::prime_list_size; std::size_t const* bound = std::upper_bound(primes, primes_end, n); - bound -= (bound != primes); + bound -= std::size_t(bound == primes_end); return *bound; } + + public: + + template + static BOOST_INTRUSIVE_FORCEINLINE SizeType suggested_upper_bucket_count(SizeType n) + { + return (suggested_upper_bucket_count_dispatch)(n, detail::bool_<(sizeof(SizeType) > sizeof(std::size_t))>()); + } + + template + static BOOST_INTRUSIVE_FORCEINLINE SizeType suggested_lower_bucket_count(SizeType n) + { + return (suggested_lower_bucket_count_dispatch)(n, detail::bool_<(sizeof(SizeType) > sizeof(std::size_t))>()); + } }; #if !defined(BOOST_INTRUSIVE_DOXYGEN_INVOKED) @@ -184,9 +247,9 @@ const std::size_t prime_list_holder::prime_list[] = { BOOST_INTRUSIVE_PRIME_C(432345564227567621), BOOST_INTRUSIVE_PRIME_C(864691128455135207), BOOST_INTRUSIVE_PRIME_C(1729382256910270481), BOOST_INTRUSIVE_PRIME_C(3458764513820540933), BOOST_INTRUSIVE_PRIME_C(6917529027641081903), BOOST_INTRUSIVE_PRIME_C(13835058055282163729), - BOOST_INTRUSIVE_PRIME_C(18446744073709551557) + BOOST_INTRUSIVE_PRIME_C(18446744073709551557), BOOST_INTRUSIVE_PRIME_C(18446744073709551615) //Upper limit, just in case #else - BOOST_INTRUSIVE_PRIME_C(4294967291) + BOOST_INTRUSIVE_PRIME_C(4294967291), BOOST_INTRUSIVE_PRIME_C(4294967295) //Upper limit, just in case #endif }; @@ -1476,16 +1539,12 @@ struct hashdata_internal static BOOST_INTRUSIVE_FORCEINLINE size_type suggested_upper_bucket_count(size_type n) { - std::size_t c = prime_list_holder<0>::suggested_upper_bucket_count - (sizeof(size_type) > sizeof(std::size_t) && n > std::size_t(-1) ? std::size_t(-1) : static_cast(n)); - return sizeof(size_type) < sizeof(std::size_t) && c > size_type(-1) ? size_type(-1) : static_cast(c); + return prime_list_holder<0>::suggested_upper_bucket_count(n); } static BOOST_INTRUSIVE_FORCEINLINE size_type suggested_lower_bucket_count(size_type n) { - std::size_t c = prime_list_holder<0>::suggested_lower_bucket_count - (sizeof(size_type) > sizeof(std::size_t) && n > std::size_t(-1) ? std::size_t(-1) : static_cast(n)); - return sizeof(size_type) < sizeof(std::size_t) && c > size_type(-1) ? size_type(-1) : static_cast(c); + return prime_list_holder<0>::suggested_lower_bucket_count(n); } const_local_iterator cbegin(size_type n) const @@ -1497,6 +1556,7 @@ struct hashdata_internal using internal_type::end; using internal_type::cend; + local_iterator end(size_type n) { return local_iterator(this->priv_bucket_pointer()[n].end(), this->priv_value_traits_ptr()); } From 00efc251445280ff60c0a378fde776d1944a7309 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ion=20Gazta=C3=B1aga?= Date: Sun, 10 Dec 2017 23:34:48 +0100 Subject: [PATCH 5/5] Reorder disabled warnings for clarity --- .../boost/intrusive/detail/config_begin.hpp | 27 +++++-------------- 1 file changed, 7 insertions(+), 20 deletions(-) diff --git a/include/boost/intrusive/detail/config_begin.hpp b/include/boost/intrusive/detail/config_begin.hpp index cef8616..8bd57a3 100644 --- a/include/boost/intrusive/detail/config_begin.hpp +++ b/include/boost/intrusive/detail/config_begin.hpp @@ -17,33 +17,20 @@ #ifdef BOOST_MSVC #pragma warning (push) - // - //'function' : resolved overload was found by argument-dependent lookup - //A function found by argument-dependent lookup (Koenig lookup) was eventually - //chosen by overload resolution. - // - //In Visual C++ .NET and earlier compilers, a different function would have - //been called. To pick the original function, use an explicitly qualified name. - // - - //warning C4275: non dll-interface class 'x' used as base for - //dll-interface class 'Y' - #pragma warning (disable : 4275) - //warning C4251: 'x' : class 'y' needs to have dll-interface to - //be used by clients of class 'z' - #pragma warning (disable : 4251) - #pragma warning (disable : 4675) - #pragma warning (disable : 4996) - #pragma warning (disable : 4503) + #pragma warning (disable : 4275) // non DLL-interface classkey "identifier" used as base for DLL-interface classkey "identifier" + #pragma warning (disable : 4251) // "identifier" : class "type" needs to have dll-interface to be used by clients of class "type2" + #pragma warning (disable : 4675) // "method" should be declared "static" and have exactly one parameter + #pragma warning (disable : 4996) // "function": was declared deprecated + #pragma warning (disable : 4503) // "identifier" : decorated name length exceeded, name was truncated #pragma warning (disable : 4284) // odd return type for operator-> #pragma warning (disable : 4244) // possible loss of data #pragma warning (disable : 4521) ////Disable "multiple copy constructors specified" #pragma warning (disable : 4127) //conditional expression is constant - #pragma warning (disable : 4146) + #pragma warning (disable : 4146) // unary minus operator applied to unsigned type, result still unsigned #pragma warning (disable : 4267) //conversion from 'X' to 'Y', possible loss of data #pragma warning (disable : 4541) //'typeid' used on polymorphic type 'boost::exception' with /GR- #pragma warning (disable : 4512) //'typeid' used on polymorphic type 'boost::exception' with /GR- - #pragma warning (disable : 4522) + #pragma warning (disable : 4522) // "class" : multiple assignment operators specified #pragma warning (disable : 4706) //assignment within conditional expression #pragma warning (disable : 4710) // function not inlined #pragma warning (disable : 4714) // "function": marked as __forceinline not inlined