diff --git a/doc/changes.qbk b/doc/changes.qbk index 5dc34cc4..7fb950a6 100644 --- a/doc/changes.qbk +++ b/doc/changes.qbk @@ -129,4 +129,9 @@ First official release. * Use Boost.Exception. * Stop using deprecated `BOOST_HAS_*` macros. +[h2 Boost 1.45.0] + +* Fix a bug when inserting into an `unordered_map` or `unordered_set` using + iterators which returns `value_type` by copy. + [endsect] diff --git a/include/boost/unordered/detail/unique.hpp b/include/boost/unordered/detail/unique.hpp index 7ad367fe..96fdfee6 100644 --- a/include/boost/unordered/detail/unique.hpp +++ b/include/boost/unordered/detail/unique.hpp @@ -1,6 +1,6 @@ // Copyright (C) 2003-2004 Jeremy B. Maitin-Shepard. -// Copyright (C) 2005-2009 Daniel James +// Copyright (C) 2005-2010 Daniel James // Distributed under the Boost Software License, Version 1.0. (See accompanying // file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) @@ -99,6 +99,8 @@ namespace boost { namespace unordered_detail { template void insert_range_impl(key_type const&, InputIt i, InputIt j); template + void insert_range_impl2(node_constructor&, key_type const&, InputIt i, InputIt j); + template void insert_range_impl(no_key, InputIt i, InputIt j); }; @@ -420,6 +422,36 @@ namespace boost { namespace unordered_detail { //////////////////////////////////////////////////////////////////////////// // Insert range methods + template + template + inline void hash_unique_table::insert_range_impl2( + node_constructor& a, key_type const& k, InputIt i, InputIt j) + { + // No side effects in this initial code + std::size_t hash_value = this->hash_function()(k); + bucket_ptr bucket = this->bucket_ptr_from_hash(hash_value); + node_ptr pos = this->find_iterator(bucket, k); + + if (!BOOST_UNORDERED_BORLAND_BOOL(pos)) { + // Doesn't already exist, add to bucket. + // Side effects only in this block. + + // Create the node before rehashing in case it throws an + // exception (need strong safety in such a case). + a.construct(*i); + + // reserve has basic exception safety if the hash function + // throws, strong otherwise. + if(this->size_ + 1 >= this->max_load_) { + this->reserve_for_insert(this->size_ + insert_size(i, j)); + bucket = this->bucket_ptr_from_hash(hash_value); + } + + // Nothing after this point can throw. + add_node(a, bucket); + } + } + template template inline void hash_unique_table::insert_range_impl( @@ -435,33 +467,14 @@ namespace boost { namespace unordered_detail { } do { - // No side effects in this initial code // Note: can't use get_key as '*i' might not be value_type - it // could be a pair with first_types as key_type without const or a // different second_type. - key_type const& k = extractor::extract(*i); - std::size_t hash_value = this->hash_function()(k); - bucket_ptr bucket = this->bucket_ptr_from_hash(hash_value); - node_ptr pos = this->find_iterator(bucket, k); - - if (!BOOST_UNORDERED_BORLAND_BOOL(pos)) { - // Doesn't already exist, add to bucket. - // Side effects only in this block. - - // Create the node before rehashing in case it throws an - // exception (need strong safety in such a case). - a.construct(*i); - - // reserve has basic exception safety if the hash function - // throws, strong otherwise. - if(this->size_ + 1 >= this->max_load_) { - this->reserve_for_insert(this->size_ + insert_size(i, j)); - bucket = this->bucket_ptr_from_hash(hash_value); - } - - // Nothing after this point can throw. - add_node(a, bucket); - } + // + // TODO: Might be worth storing the value_type instead of the key + // here. Could be more efficient if '*i' is expensive. Could be + // less efficient if copying the full value_type is expensive. + insert_range_impl2(a, extractor::extract(*i), i, j); } while(++i != j); } diff --git a/test/exception/constructor_exception_tests.cpp b/test/exception/constructor_exception_tests.cpp index 96d5c1f2..aa1e02a4 100644 --- a/test/exception/constructor_exception_tests.cpp +++ b/test/exception/constructor_exception_tests.cpp @@ -148,6 +148,19 @@ struct input_range_construct_test : public range, objects } }; +template +struct copy_range_construct_test : public range, objects +{ + copy_range_construct_test() : range(60) {} + + void run() const { + T x(test::copy_iterator(this->values.begin()), + test::copy_iterator(this->values.end()), + 0, hash, equal_to, allocator); + avoid_unused_warning(x); + } +}; + RUN_EXCEPTION_TESTS( (construct_test1) (construct_test2) @@ -160,5 +173,6 @@ RUN_EXCEPTION_TESTS( (range_construct_test3) (range_construct_test4) (range_construct_test5) - (input_range_construct_test), + (input_range_construct_test) + (copy_range_construct_test), CONTAINER_SEQ) diff --git a/test/helpers/input_iterator.hpp b/test/helpers/input_iterator.hpp index 873120cd..7bd2fdc7 100644 --- a/test/helpers/input_iterator.hpp +++ b/test/helpers/input_iterator.hpp @@ -1,5 +1,5 @@ -// Copyright 2005-2009 Daniel James. +// Copyright 2005-2010 Daniel James. // Distributed under the Boost Software License, Version 1.0. (See accompanying // file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) @@ -69,6 +69,65 @@ namespace test { return input_iterator_adaptor(it); } + + template + struct copy_iterator_adaptor + : public boost::iterator< + BOOST_DEDUCED_TYPENAME boost::iterator_category::type, + BOOST_DEDUCED_TYPENAME boost::iterator_value::type, + BOOST_DEDUCED_TYPENAME boost::iterator_difference::type, + BOOST_DEDUCED_TYPENAME boost::iterator_pointer::type, + proxy + > + { + typedef BOOST_DEDUCED_TYPENAME boost::iterator_value::type + value_type; + typedef BOOST_DEDUCED_TYPENAME boost::iterator_difference::type + difference_type; + + copy_iterator_adaptor() + : base_() {} + explicit copy_iterator_adaptor(Iterator const& it) + : base_(it) {} + value_type operator*() const { + return *base_; + } + value_type* operator->() const { + return &*base_; + } + copy_iterator_adaptor& operator++() { + ++base_; return *this; + } + copy_iterator_adaptor operator++(int) { + copy_iterator_adaptor tmp(*this); ++base_; return tmp; + } + bool operator==(copy_iterator_adaptor const& x) const { + return base_ == x.base_; + } + bool operator!=(copy_iterator_adaptor const& x) const { + return base_ != x.base_; + } + copy_iterator_adaptor operator+=(difference_type x) { + base_ += x; + return *this; + } + copy_iterator_adaptor operator-=(difference_type x) { + base_ -= x; + return *this; + } + difference_type operator-(copy_iterator_adaptor const& other) { + return base_-other.base_; + } + private: + Iterator base_; + }; + + template + copy_iterator_adaptor copy_iterator(Iterator const& it) + { + return copy_iterator_adaptor(it); + } + } #endif diff --git a/test/unordered/constructor_tests.cpp b/test/unordered/constructor_tests.cpp index e48cecde..489bf01a 100644 --- a/test/unordered/constructor_tests.cpp +++ b/test/unordered/constructor_tests.cpp @@ -1,5 +1,5 @@ -// Copyright 2006-2009 Daniel James. +// Copyright 2006-2010 Daniel James. // Distributed under the Boost Software License, Version 1.0. (See accompanying // file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) @@ -260,6 +260,19 @@ void constructor_tests2(T*, test::check_equivalent_keys(y); } + std::cerr<<"Construct 8.5 - from copy iterator\n"; + { + test::random_values v(100, generator); + T x(test::copy_iterator(v.begin()), + test::copy_iterator(v.end()), 0, hf1, eq1); + T y(test::copy_iterator(x.begin()), + test::copy_iterator(x.end()), 0, hf2, eq2); + test::check_container(x, v); + test::check_container(y, x); + test::check_equivalent_keys(x); + test::check_equivalent_keys(y); + } + std::cerr<<"Construct 9\n"; { test::random_values v(100, generator); diff --git a/test/unordered/insert_tests.cpp b/test/unordered/insert_tests.cpp index 5af8be66..4fee0771 100644 --- a/test/unordered/insert_tests.cpp +++ b/test/unordered/insert_tests.cpp @@ -1,5 +1,5 @@ -// Copyright 2006-2009 Daniel James. +// Copyright 2006-2010 Daniel James. // Distributed under the Boost Software License, Version 1.0. (See accompanying // file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) @@ -229,6 +229,18 @@ void insert_tests2(X*, test::check_equivalent_keys(x); } + + std::cerr<<"insert copy iterator range tests.\n"; + + { + X x; + + test::random_values v(1000, generator); + x.insert(test::copy_iterator(v.begin()), test::copy_iterator(v.end())); + test::check_container(x, v); + + test::check_equivalent_keys(x); + } } #if !defined(BOOST_NO_RVALUE_REFERENCES) && !defined(BOOST_NO_VARIADIC_TEMPLATES)