diff --git a/doc/changes.qbk b/doc/changes.qbk index 8b2bd44c..204b427b 100644 --- a/doc/changes.qbk +++ b/doc/changes.qbk @@ -45,5 +45,21 @@ First official release. [h2 Boost 1.38.0] * Use [@../../libs/utility/swap.html `boost::swap`]. +* [@https://svn.boost.org/trac/boost/ticket/2237 Ticket 2237]: + Document that the equality and inequality operators are undefined for two + objects if their equality predicates aren't equivalent. Thanks to Daniel + Krügler. +* [@https://svn.boost.org/trac/boost/ticket/1710 Ticket 1710]: + Use a larger prime number list. Thanks to Thorsten Ottosen and Hervé + Brönnimann. +* Use + [@../../libs/type_traits/doc/html/boost_typetraits/category/alignment.html + aligned storage] to store the types. This changes the way the allocator is + used to construct nodes. It used to construct the node with two calls to + the allocator's `construct` method - once for the pointers and once for the + value. It now constructs the node with a single call to construct and + then constructs the value using in place construction. +* Add support for C++0x initializer lists where they're available (currently + only g++ 4.4 in C++0x mode). [endsect] diff --git a/doc/hash_equality.qbk b/doc/hash_equality.qbk index 7ca4396f..74004b49 100644 --- a/doc/hash_equality.qbk +++ b/doc/hash_equality.qbk @@ -41,6 +41,16 @@ This is a simplified version of the example at [@../../libs/unordered/examples/case_insensitive.hpp /libs/unordered/examples/case_insensitive.hpp] which supports other locales and string types. +[caution +Be careful when using the equality (`==`) operator with custom equality +predicates, especially if you're using a function pointer. If you compare two +containers with different equality predicates then the result is undefined. +For most stateless function objects this is impossible - since you can only +compare objects with the same equality predicate you know the equality +predicates must be equal. But if you're using function pointers or a stateful +equality predicate (e.g. boost::function) then you can get into trouble. +] + [h2 Custom Types] Similarly, a custom hash function can be used for custom types: diff --git a/doc/ref.xml b/doc/ref.xml index 67ef3640..2b045219 100644 --- a/doc/ref.xml +++ b/doc/ref.xml @@ -678,6 +678,8 @@ file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) bool This is a boost extension. + Behavior is undefined if the two containers don't have + equivalent equality predicates. @@ -700,6 +702,8 @@ file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) bool This is a boost extension. + Behavior is undefined if the two containers don't have + equivalent equality predicates. @@ -1406,6 +1410,8 @@ file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) bool This is a boost extension. + Behavior is undefined if the two containers don't have + equivalent equality predicates. @@ -1428,6 +1434,8 @@ file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) bool This is a boost extension. + Behavior is undefined if the two containers don't have + equivalent equality predicates. @@ -2185,6 +2193,8 @@ file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) bool This is a boost extension. + Behavior is undefined if the two containers don't have + equivalent equality predicates. @@ -2209,6 +2219,8 @@ file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) bool This is a boost extension. + Behavior is undefined if the two containers don't have + equivalent equality predicates. @@ -2927,6 +2939,8 @@ file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) bool This is a boost extension. + Behavior is undefined if the two containers don't have + equivalent equality predicates. @@ -2951,6 +2965,8 @@ file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) bool This is a boost extension. + Behavior is undefined if the two containers don't have + equivalent equality predicates. diff --git a/include/boost/unordered/detail/hash_table.hpp b/include/boost/unordered/detail/hash_table.hpp index 7e649a3f..7601c8ff 100644 --- a/include/boost/unordered/detail/hash_table.hpp +++ b/include/boost/unordered/detail/hash_table.hpp @@ -26,6 +26,8 @@ #include #include #include +#include +#include #include #include #include @@ -76,8 +78,9 @@ namespace boost { template std::size_t const prime_list_template::value[] = { - 53ul, 97ul, 193ul, 389ul, 769ul, - 1543ul, 3079ul, 6151ul, 12289ul, 24593ul, + 5ul, 11ul, 17ul, 29ul, 37ul, 53ul, 67ul, 79ul, + 97ul, 131ul, 193ul, 257ul, 389ul, 521ul, 769ul, + 1031ul, 1543ul, 2053ul, 3079ul, 6151ul, 12289ul, 24593ul, 49157ul, 98317ul, 196613ul, 393241ul, 786433ul, 1572869ul, 3145739ul, 6291469ul, 12582917ul, 25165843ul, 50331653ul, 100663319ul, 201326611ul, 402653189ul, 805306457ul, @@ -220,7 +223,11 @@ namespace boost { functions func2_; functions_ptr func_; // The currently active functions. }; - + + template + void destroy(T* x) { + x->~T(); + } } } diff --git a/include/boost/unordered/detail/hash_table_impl.hpp b/include/boost/unordered/detail/hash_table_impl.hpp index a75d30d5..bdee0614 100644 --- a/include/boost/unordered/detail/hash_table_impl.hpp +++ b/include/boost/unordered/detail/hash_table_impl.hpp @@ -34,7 +34,6 @@ namespace boost { public: typedef BOOST_UNORDERED_TABLE_DATA data; - struct node_base; struct node; struct bucket; typedef std::size_t size_type; @@ -45,9 +44,6 @@ namespace boost { typedef BOOST_DEDUCED_TYPENAME boost::unordered_detail::rebind_wrap::type node_allocator; - typedef BOOST_DEDUCED_TYPENAME - boost::unordered_detail::rebind_wrap::type - node_base_allocator; typedef BOOST_DEDUCED_TYPENAME boost::unordered_detail::rebind_wrap::type bucket_allocator; @@ -88,39 +84,36 @@ namespace boost { } }; + // Value Base + + struct value_base { + typename boost::aligned_storage< + sizeof(value_type), + boost::alignment_of::value>::type data_; + + void* address() { return this; } + }; + // Hash Node // // all no throw - struct node_base : bucket - { + struct node : value_base, bucket { #if BOOST_UNORDERED_EQUIVALENT_KEYS public: - node_base() : group_prev_() + node() : group_prev_() { BOOST_UNORDERED_MSVC_RESET_PTR(group_prev_); } link_ptr group_prev_; #endif + + value_type& value() { + return *static_cast(this->address()); + } }; - struct node : node_base - { - public: -#if defined(BOOST_HAS_RVALUE_REFS) && defined(BOOST_HAS_VARIADIC_TMPL) - template - node(Args&&... args) - : node_base(), value_(std::forward(args)...) {} -#else - node(value_type const& v) : node_base(), value_(v) {} -#endif - - value_type value_; - }; - -#if defined(BOOST_HAS_RVALUE_REFS) && defined(BOOST_HAS_VARIADIC_TMPL) - // allocators // // Stores all the allocators that we're going to need. @@ -136,7 +129,9 @@ namespace boost { void destroy(link_ptr ptr) { - node_ptr n(node_alloc_.address(*static_cast(&*ptr))); + node* raw_ptr = static_cast(&*ptr); + boost::unordered_detail::destroy(&raw_ptr->value()); + node_ptr n(node_alloc_.address(*raw_ptr)); node_alloc_.destroy(n); node_alloc_.deallocate(n, 1); } @@ -163,146 +158,62 @@ namespace boost { node_ptr node_; bool node_constructed_; + bool value_constructed_; public: node_constructor(allocators& a) : allocators_(a), - node_(), node_constructed_(false) + node_(), node_constructed_(false), value_constructed_(false) { } ~node_constructor() { if (node_) { + if (value_constructed_) { + boost::unordered_detail::destroy(&node_->value()); + } + if (node_constructed_) allocators_.node_alloc_.destroy(node_); allocators_.node_alloc_.deallocate(node_, 1); } } +#if defined(BOOST_HAS_RVALUE_REFS) && defined(BOOST_HAS_VARIADIC_TMPL) template void construct(Args&&... args) { BOOST_ASSERT(!node_); node_constructed_ = false; + value_constructed_ = false; node_ = allocators_.node_alloc_.allocate(1); - allocators_.node_alloc_.construct(node_, std::forward(args)...); + + allocators_.node_alloc_.construct(node_, node()); node_constructed_ = true; - } - node_ptr get() const - { - BOOST_ASSERT(node_); - return node_; + new(node_->address()) value_type(std::forward(args)...); + value_constructed_ = true; } - - // no throw - link_ptr release() - { - node_ptr p = node_; - unordered_detail::reset(node_); - return link_ptr(allocators_.bucket_alloc_.address(*p)); - } - - private: - node_constructor(node_constructor const&); - node_constructor& operator=(node_constructor const&); - }; #else - - // allocators - // - // Stores all the allocators that we're going to need. - - struct allocators - { - node_allocator node_alloc_; - bucket_allocator bucket_alloc_; - value_allocator value_alloc_; - node_base_allocator node_base_alloc_; - - allocators(value_allocator const& a) - : node_alloc_(a), bucket_alloc_(a), - value_alloc_(a), node_base_alloc_(a) - {} - - void destroy(link_ptr ptr) - { - node_ptr n(node_alloc_.address(*static_cast(&*ptr))); - value_alloc_.destroy(value_alloc_.address(n->value_)); - node_base_alloc_.destroy(node_base_alloc_.address(*n)); - node_alloc_.deallocate(n, 1); - } - - void swap(allocators& x) - { - boost::swap(node_alloc_, x.node_alloc_); - boost::swap(bucket_alloc_, x.bucket_alloc_); - boost::swap(value_alloc_, x.value_alloc_); - boost::swap(node_base_alloc_, x.node_base_alloc_); - } - - bool operator==(allocators const& x) - { - return value_alloc_ == x.value_alloc_; - } - }; - - // node_constructor - // - // Used to construct nodes in an exception safe manner. - - class node_constructor - { - allocators& allocators_; - - node_ptr node_; - bool value_constructed_; - bool node_base_constructed_; - - public: - - node_constructor(allocators& a) - : allocators_(a), - node_(), value_constructed_(false), node_base_constructed_(false) - { - BOOST_UNORDERED_MSVC_RESET_PTR(node_); - } - - ~node_constructor() - { - if (node_) { - if (value_constructed_) - allocators_.value_alloc_.destroy( - allocators_.value_alloc_.address(node_->value_)); - if (node_base_constructed_) - allocators_.node_base_alloc_.destroy( - allocators_.node_base_alloc_.address(*node_)); - - allocators_.node_alloc_.deallocate(node_, 1); - } - } - template void construct(V const& v) { BOOST_ASSERT(!node_); + node_constructed_ = false; value_constructed_ = false; - node_base_constructed_ = false; node_ = allocators_.node_alloc_.allocate(1); - allocators_.node_base_alloc_.construct( - allocators_.node_base_alloc_.address(*node_), - node_base()); - node_base_constructed_ = true; + allocators_.node_alloc_.construct(node_, node()); + node_constructed_ = true; - allocators_.value_alloc_.construct( - allocators_.value_alloc_.address(node_->value_), v); + new(node_->address()) value_type(v); value_constructed_ = true; } +#endif node_ptr get() const { @@ -322,7 +233,6 @@ namespace boost { node_constructor(node_constructor const&); node_constructor& operator=(node_constructor const&); }; -#endif // Methods for navigating groups of elements with equal keys. @@ -351,8 +261,7 @@ namespace boost { // pre: Must be pointing to a node static inline reference get_value(link_ptr p) { - BOOST_ASSERT(p); - return static_cast(&*p)->value_; + return get_node(p).value(); } class iterator_base @@ -1357,17 +1266,10 @@ namespace boost { // accessors // no throw -#if defined(BOOST_HAS_RVALUE_REFS) && defined(BOOST_HAS_VARIADIC_TMPL) node_allocator get_allocator() const { return data_.allocators_.node_alloc_; } -#else - value_allocator get_allocator() const - { - return data_.allocators_.value_alloc_; - } -#endif // no throw hasher const& hash_function() const @@ -1469,6 +1371,21 @@ namespace boost { return need_to_reserve; } + // basic exception safety + bool reserve_for_insert(size_type n) + { + bool need_to_reserve = n >= max_load_; + // throws - basic: + if (need_to_reserve) { + size_type s = size(); + s = s + (s >> 1); + s = s > n ? s : n; + rehash_impl(min_buckets_for_size(s)); + } + BOOST_ASSERT(n < max_load_ || n > max_size()); + return need_to_reserve; + } + public: // no throw @@ -1713,14 +1630,14 @@ namespace boost { iterator_base insert_impl(node_constructor& a) { - key_type const& k = extract_key(a.get()->value_); + key_type const& k = extract_key(a.get()->value()); size_type hash_value = hash_function()(k); bucket_ptr bucket = data_.bucket_ptr_from_hash(hash_value); link_ptr position = find_iterator(bucket, k); // reserve has basic exception safety if the hash function // throws, strong otherwise. - if(reserve(size() + 1)) + if(reserve_for_insert(size() + 1)) bucket = data_.bucket_ptr_from_hash(hash_value); // I'm relying on link_ptr not being invalidated by @@ -1735,7 +1652,7 @@ namespace boost { iterator_base insert_hint_impl(iterator_base const& it, node_constructor& a) { // equal can throw, but with no effects - if (it == data_.end() || !equal(extract_key(a.get()->value_), *it)) { + if (it == data_.end() || !equal(extract_key(a.get()->value()), *it)) { // Use the standard insert if the iterator doesn't point // to a matching key. return insert_impl(a); @@ -1750,8 +1667,8 @@ namespace boost { // reserve has basic exception safety if the hash function // throws, strong otherwise. - bucket_ptr base = reserve(size() + 1) ? - get_bucket(extract_key(a.get()->value_)) : it.bucket_; + bucket_ptr base = reserve_for_insert(size() + 1) ? + get_bucket(extract_key(a.get()->value())) : it.bucket_; // Nothing after this point can throw @@ -1775,13 +1692,13 @@ namespace boost { } else { // Only require basic exception safety here - reserve(size() + distance); + reserve_for_insert(size() + distance); node_constructor a(data_.allocators_); for (; i != j; ++i) { a.construct(*i); - key_type const& k = extract_key(a.get()->value_); + key_type const& k = extract_key(a.get()->value()); bucket_ptr bucket = get_bucket(k); link_ptr position = find_iterator(bucket, k); @@ -1841,7 +1758,7 @@ namespace boost { // reserve has basic exception safety if the hash function // throws, strong otherwise. - if(reserve(size() + 1)) + if(reserve_for_insert(size() + 1)) bucket = data_.bucket_ptr_from_hash(hash_value); // Nothing after this point can throw. @@ -1880,7 +1797,7 @@ namespace boost { // reserve has basic exception safety if the hash function // throws, strong otherwise. - if(reserve(size() + 1)) + if(reserve_for_insert(size() + 1)) bucket = data_.bucket_ptr_from_hash(hash_value); // Nothing after this point can throw. @@ -1947,7 +1864,7 @@ namespace boost { // reserve has basic exception safety if the hash function // throws, strong otherwise. - if(reserve(size() + 1)) + if(reserve_for_insert(size() + 1)) bucket = data_.bucket_ptr_from_hash(hash_value); // Nothing after this point can throw. @@ -1966,7 +1883,7 @@ namespace boost { a.construct(std::forward(args)...); // No side effects in this initial code - key_type const& k = extract_key(a.get()->value_); + key_type const& k = extract_key(a.get()->value()); size_type hash_value = hash_function()(k); bucket_ptr bucket = data_.bucket_ptr_from_hash(hash_value); link_ptr pos = find_iterator(bucket, k); @@ -1978,7 +1895,7 @@ namespace boost { } else { // reserve has basic exception safety if the hash function // throws, strong otherwise. - if(reserve(size() + 1)) + if(reserve_for_insert(size() + 1)) bucket = data_.bucket_ptr_from_hash(hash_value); // Nothing after this point can throw. @@ -2047,7 +1964,7 @@ namespace boost { // reserve has basic exception safety if the hash function // throws, strong otherwise. if(size() + 1 >= max_load_) { - reserve(size() + insert_size(i, j)); + reserve_for_insert(size() + insert_size(i, j)); bucket = data_.bucket_ptr_from_hash(hash_value); } diff --git a/include/boost/unordered/unordered_map.hpp b/include/boost/unordered/unordered_map.hpp index edbee93a..69e5522e 100644 --- a/include/boost/unordered/unordered_map.hpp +++ b/include/boost/unordered/unordered_map.hpp @@ -135,6 +135,24 @@ namespace boost #endif #endif +#if !defined(BOOST_NO_INITIALIZER_LISTS) + unordered_map(std::initializer_list list, + size_type n = boost::unordered_detail::default_initial_bucket_count, + const hasher &hf = hasher(), + const key_equal &eql = key_equal(), + const allocator_type &a = allocator_type()) + : base(list.begin(), list.end(), n, hf, eql, a) + { + } + + unordered_map& operator=(std::initializer_list list) + { + base.data_.clear(); + base.insert_range(list.begin(), list.end()); + return *this; + } +#endif + private: BOOST_DEDUCED_TYPENAME implementation::iterator_base const& @@ -523,6 +541,24 @@ namespace boost #endif #endif +#if !defined(BOOST_NO_INITIALIZER_LISTS) + unordered_multimap(std::initializer_list list, + size_type n = boost::unordered_detail::default_initial_bucket_count, + const hasher &hf = hasher(), + const key_equal &eql = key_equal(), + const allocator_type &a = allocator_type()) + : base(list.begin(), list.end(), n, hf, eql, a) + { + } + + unordered_multimap& operator=(std::initializer_list list) + { + base.data_.clear(); + base.insert_range(list.begin(), list.end()); + return *this; + } +#endif + private: diff --git a/include/boost/unordered/unordered_set.hpp b/include/boost/unordered/unordered_set.hpp index c8d0d1e1..8f39c87e 100644 --- a/include/boost/unordered/unordered_set.hpp +++ b/include/boost/unordered/unordered_set.hpp @@ -133,6 +133,24 @@ namespace boost #endif #endif +#if !defined(BOOST_NO_INITIALIZER_LISTS) + unordered_set(std::initializer_list list, + size_type n = boost::unordered_detail::default_initial_bucket_count, + const hasher &hf = hasher(), + const key_equal &eql = key_equal(), + const allocator_type &a = allocator_type()) + : base(list.begin(), list.end(), n, hf, eql, a) + { + } + + unordered_set& operator=(std::initializer_list list) + { + base.data_.clear(); + base.insert_range(list.begin(), list.end()); + return *this; + } +#endif + private: BOOST_DEDUCED_TYPENAME implementation::iterator_base const& @@ -493,6 +511,24 @@ namespace boost #endif #endif +#if !defined(BOOST_NO_INITIALIZER_LISTS) + unordered_multiset(std::initializer_list list, + size_type n = boost::unordered_detail::default_initial_bucket_count, + const hasher &hf = hasher(), + const key_equal &eql = key_equal(), + const allocator_type &a = allocator_type()) + : base(list.begin(), list.end(), n, hf, eql, a) + { + } + + unordered_multiset& operator=(std::initializer_list list) + { + base.data_.clear(); + base.insert_range(list.begin(), list.end()); + return *this; + } +#endif + private: BOOST_DEDUCED_TYPENAME implementation::iterator_base const& diff --git a/test/unordered/assign_tests.cpp b/test/unordered/assign_tests.cpp index bb8daff8..76d81ec5 100644 --- a/test/unordered/assign_tests.cpp +++ b/test/unordered/assign_tests.cpp @@ -103,6 +103,22 @@ UNORDERED_TEST(assign_tests2, ((default_generator)(generate_collisions)) ) +#if !defined(BOOST_NO_INITIALIZER_LISTS) + +UNORDERED_AUTO_TEST(assign_initializer_list) +{ + std::cerr<<"Initializer List Tests\n"; + + boost::unordered_set x; + x.insert(10); + x.insert(20); + x = { 1, 2, -10 }; + BOOST_CHECK(x.find(10) == x.end()); + BOOST_CHECK(x.find(-10) != x.end()); +} + +#endif + } RUN_TESTS() diff --git a/test/unordered/constructor_tests.cpp b/test/unordered/constructor_tests.cpp index bdf5cd10..a56f7905 100644 --- a/test/unordered/constructor_tests.cpp +++ b/test/unordered/constructor_tests.cpp @@ -288,6 +288,17 @@ UNORDERED_TEST(map_constructor_test, ((test_map)(test_multimap)) ) +#if !defined(BOOST_NO_INITIALIZER_LISTS) + +UNORDERED_AUTO_TEST(test_initializer_list) { + std::cerr<<"Initializer List Tests\n"; + boost::unordered_set x1 = { 2, 10, 45, -5 }; + BOOST_CHECK(x1.find(10) != x1.end()); + BOOST_CHECK(x1.find(46) == x1.end()); +} + +#endif + } RUN_TESTS() diff --git a/test/unordered/equality_tests.cpp b/test/unordered/equality_tests.cpp index ff0e6b57..997e609d 100644 --- a/test/unordered/equality_tests.cpp +++ b/test/unordered/equality_tests.cpp @@ -13,6 +13,10 @@ namespace equality_tests { struct mod_compare { + bool alt_hash_; + + explicit mod_compare(bool alt_hash = false) : alt_hash_(alt_hash) {} + bool operator()(int x, int y) const { return x % 1000 == y % 1000; @@ -20,7 +24,7 @@ namespace equality_tests int operator()(int x) const { - return x % 250; + return alt_hash_ ? x % 250 : (x + 5) % 250; } }; @@ -138,6 +142,25 @@ namespace equality_tests ((1)(2))((1001)(1)), ==, ((1001)(2))((1)(1))); } + // Test that equality still works when the two containers have + // different hash functions but the same equality predicate. + + UNORDERED_AUTO_TEST(equality_different_hash_test) + { + typedef boost::unordered_set set; + set set1(0, mod_compare(false), mod_compare(false)); + set set2(0, mod_compare(true), mod_compare(true)); + BOOST_CHECK(set1 == set2); + set1.insert(1); set2.insert(2); + BOOST_CHECK(set1 != set2); + set1.insert(2); set2.insert(1); + BOOST_CHECK(set1 == set2); + set1.insert(10); set2.insert(20); + BOOST_CHECK(set1 != set2); + set1.insert(20); set2.insert(10); + BOOST_CHECK(set1 == set2); + } + } RUN_TESTS()