diff options
author | Abseil Team <absl-team@google.com> | 2020-06-16 12:36:35 -0700 |
---|---|---|
committer | Andy Getz <durandal@google.com> | 2020-06-17 10:29:28 -0400 |
commit | 4a851046a0102cd986a5714a1af8deef28a544c4 (patch) | |
tree | 08cfadacd9a087dfff8fe95e7accc33b33190c52 /absl | |
parent | ccdbb5941f992fabda7eae3ce72f55efc17c826a (diff) |
Export of internal Abseil changes
--
1f80d4f1cb8847545627a2d0b18f697c4a763292 by Samuel Benzaquen <sbenza@google.com>:
Add a hint message to the assertions for easier debugging.
Also, move it out of the template to avoid printing the whole template
instantiation as part of the assertion. It is not relevant and can
significantly bloat the error message and hide the important bits.
PiperOrigin-RevId: 316736140
--
223e97a90150de5b7206be2e45c62a9b70ac02df by Tom Manshreck <shreck@google.com>:
Update Span description to remove "view" terminology, in light of recent clarification of the differences between a view and a span type.
PiperOrigin-RevId: 316734382
--
361b87d55b1809a5da3f72a996686f27b3d630c2 by Evan Brown <ezb@google.com>:
Allow for explicit conversion of values in btree range constructor/insert.
PiperOrigin-RevId: 316725951
GitOrigin-RevId: 1f80d4f1cb8847545627a2d0b18f697c4a763292
Change-Id: I74e2b095bc24710b27ed63ed94a50ef8f0fc897f
Diffstat (limited to 'absl')
-rw-r--r-- | absl/container/btree_test.cc | 54 | ||||
-rw-r--r-- | absl/container/internal/btree.h | 52 | ||||
-rw-r--r-- | absl/container/internal/btree_container.h | 4 | ||||
-rw-r--r-- | absl/container/internal/raw_hash_set.h | 31 | ||||
-rw-r--r-- | absl/container/internal/raw_hash_set_test.cc | 2 | ||||
-rw-r--r-- | absl/types/span.h | 22 |
6 files changed, 126 insertions, 39 deletions
diff --git a/absl/container/btree_test.cc b/absl/container/btree_test.cc index bbdb5f42..8234b017 100644 --- a/absl/container/btree_test.cc +++ b/absl/container/btree_test.cc @@ -52,6 +52,7 @@ using ::absl::test_internal::MovableOnlyInstance; using ::testing::ElementsAre; using ::testing::ElementsAreArray; using ::testing::IsEmpty; +using ::testing::IsNull; using ::testing::Pair; template <typename T, typename U> @@ -2404,6 +2405,59 @@ TEST(Btree, BitfieldArgument) { m[n]; } +TEST(Btree, SetRangeConstructorAndInsertSupportExplicitConversionComparable) { + const absl::string_view names[] = {"n1", "n2"}; + + absl::btree_set<std::string> name_set1{std::begin(names), std::end(names)}; + EXPECT_THAT(name_set1, ElementsAreArray(names)); + + absl::btree_set<std::string> name_set2; + name_set2.insert(std::begin(names), std::end(names)); + EXPECT_THAT(name_set2, ElementsAreArray(names)); +} + +TEST(Btree, + SetRangeConstructorAndInsertSupportExplicitConversionNonComparable) { + const int i[] = {0, 1}; + + absl::btree_set<std::vector<void *>> s1{std::begin(i), std::end(i)}; + EXPECT_THAT(s1, ElementsAre(IsEmpty(), ElementsAre(IsNull()))); + + absl::btree_set<std::vector<void *>> s2; + s2.insert(std::begin(i), std::end(i)); + EXPECT_THAT(s2, ElementsAre(IsEmpty(), ElementsAre(IsNull()))); +} + +// GCC 4.9 has a bug in the std::pair constructors that prevents explicit +// conversions between pair types. +#if defined(__clang__) || !defined(__GNUC__) || __GNUC__ >= 5 +TEST(Btree, MapRangeConstructorAndInsertSupportExplicitConversionComparable) { + const std::pair<absl::string_view, int> names[] = {{"n1", 1}, {"n2", 2}}; + + absl::btree_map<std::string, int> name_map1{std::begin(names), + std::end(names)}; + EXPECT_THAT(name_map1, ElementsAre(Pair("n1", 1), Pair("n2", 2))); + + absl::btree_map<std::string, int> name_map2; + name_map2.insert(std::begin(names), std::end(names)); + EXPECT_THAT(name_map2, ElementsAre(Pair("n1", 1), Pair("n2", 2))); +} + +TEST(Btree, + MapRangeConstructorAndInsertSupportExplicitConversionNonComparable) { + const std::pair<int, int> i[] = {{0, 1}, {1, 2}}; + + absl::btree_map<std::vector<void *>, int> m1{std::begin(i), std::end(i)}; + EXPECT_THAT(m1, + ElementsAre(Pair(IsEmpty(), 1), Pair(ElementsAre(IsNull()), 2))); + + absl::btree_map<std::vector<void *>, int> m2; + m2.insert(std::begin(i), std::end(i)); + EXPECT_THAT(m2, + ElementsAre(Pair(IsEmpty(), 1), Pair(ElementsAre(IsNull()), 2))); +} +#endif + } // namespace } // namespace container_internal ABSL_NAMESPACE_END diff --git a/absl/container/internal/btree.h b/absl/container/internal/btree.h index f52fe235..9fe1b27f 100644 --- a/absl/container/internal/btree.h +++ b/absl/container/internal/btree.h @@ -290,9 +290,12 @@ struct map_params : common_params<Key, Compare, Alloc, TargetNodeSize, Multi, }; using is_map_container = std::true_type; - static const Key &key(const value_type &value) { return value.first; } - static const Key &key(const init_type &init) { return init.first; } + template <typename V> + static auto key(const V &value) -> decltype(value.first) { + return value.first; + } static const Key &key(const slot_type *s) { return slot_policy::key(s); } + static const Key &key(slot_type *s) { return slot_policy::key(s); } static mapped_type &value(value_type *value) { return value->second; } }; @@ -353,8 +356,10 @@ struct set_params : common_params<Key, Compare, Alloc, TargetNodeSize, Multi, using value_compare = typename set_params::common_params::key_compare; using is_map_container = std::false_type; - static const Key &key(const value_type &value) { return value; } + template <typename V> + static const V &key(const V &value) { return value; } static const Key &key(const slot_type *slot) { return *slot; } + static const Key &key(slot_type *slot) { return *slot; } }; // An adapter class that converts a lower-bound compare into an upper-bound @@ -1020,6 +1025,7 @@ template <typename Params> class btree { using node_type = btree_node<Params>; using is_key_compare_to = typename Params::is_key_compare_to; + using init_type = typename Params::init_type; // We use a static empty node for the root/leftmost/rightmost of empty btrees // in order to avoid branching in begin()/end(). @@ -1184,8 +1190,8 @@ class btree { // boolean return value indicates whether insertion succeeded or failed. // Requirement: if `key` already exists in the btree, does not consume `args`. // Requirement: `key` is never referenced after consuming `args`. - template <typename... Args> - std::pair<iterator, bool> insert_unique(const key_type &key, Args &&... args); + template <typename K, typename... Args> + std::pair<iterator, bool> insert_unique(const K &key, Args &&... args); // Inserts with hint. Checks to see if the value should be placed immediately // before `position` in the tree. If so, then the insertion will take @@ -1193,14 +1199,23 @@ class btree { // logarithmic time as if a call to insert_unique() were made. // Requirement: if `key` already exists in the btree, does not consume `args`. // Requirement: `key` is never referenced after consuming `args`. - template <typename... Args> + template <typename K, typename... Args> std::pair<iterator, bool> insert_hint_unique(iterator position, - const key_type &key, + const K &key, Args &&... args); // Insert a range of values into the btree. + // Note: the first overload avoids constructing a value_type if the key + // already exists in the btree. + template <typename InputIterator, + typename = decltype( + compare_keys(params_type::key(*std::declval<InputIterator>()), + std::declval<const key_type &>()))> + void insert_iterator_unique(InputIterator b, InputIterator e, int); + // We need the second overload for cases in which we need to construct a + // value_type in order to compare it with the keys already in the btree. template <typename InputIterator> - void insert_iterator_unique(InputIterator b, InputIterator e); + void insert_iterator_unique(InputIterator b, InputIterator e, char); // Inserts a value into the btree. template <typename ValueType> @@ -1855,8 +1870,8 @@ btree<P>::btree(const btree &other) } template <typename P> -template <typename... Args> -auto btree<P>::insert_unique(const key_type &key, Args &&... args) +template <typename K, typename... Args> +auto btree<P>::insert_unique(const K &key, Args &&... args) -> std::pair<iterator, bool> { if (empty()) { mutable_root() = rightmost_ = new_leaf_root_node(1); @@ -1881,8 +1896,8 @@ auto btree<P>::insert_unique(const key_type &key, Args &&... args) } template <typename P> -template <typename... Args> -inline auto btree<P>::insert_hint_unique(iterator position, const key_type &key, +template <typename K, typename... Args> +inline auto btree<P>::insert_hint_unique(iterator position, const K &key, Args &&... args) -> std::pair<iterator, bool> { if (!empty()) { @@ -1906,14 +1921,23 @@ inline auto btree<P>::insert_hint_unique(iterator position, const key_type &key, } template <typename P> -template <typename InputIterator> -void btree<P>::insert_iterator_unique(InputIterator b, InputIterator e) { +template <typename InputIterator, typename> +void btree<P>::insert_iterator_unique(InputIterator b, InputIterator e, int) { for (; b != e; ++b) { insert_hint_unique(end(), params_type::key(*b), *b); } } template <typename P> +template <typename InputIterator> +void btree<P>::insert_iterator_unique(InputIterator b, InputIterator e, char) { + for (; b != e; ++b) { + init_type value(*b); + insert_hint_unique(end(), params_type::key(value), std::move(value)); + } +} + +template <typename P> template <typename ValueType> auto btree<P>::insert_multi(const key_type &key, ValueType &&v) -> iterator { if (empty()) { diff --git a/absl/container/internal/btree_container.h b/absl/container/internal/btree_container.h index 734c90ef..d0fc6451 100644 --- a/absl/container/internal/btree_container.h +++ b/absl/container/internal/btree_container.h @@ -289,10 +289,10 @@ class btree_set_container : public btree_container<Tree> { } template <typename InputIterator> void insert(InputIterator b, InputIterator e) { - this->tree_.insert_iterator_unique(b, e); + this->tree_.insert_iterator_unique(b, e, 0); } void insert(std::initializer_list<init_type> init) { - this->tree_.insert_iterator_unique(init.begin(), init.end()); + this->tree_.insert_iterator_unique(init.begin(), init.end(), 0); } insert_return_type insert(node_type &&node) { if (!node) return {this->end(), false, node_type()}; diff --git a/absl/container/internal/raw_hash_set.h b/absl/container/internal/raw_hash_set.h index df0f2b2b..1f304b61 100644 --- a/absl/container/internal/raw_hash_set.h +++ b/absl/container/internal/raw_hash_set.h @@ -497,6 +497,18 @@ inline size_t GrowthToLowerboundCapacity(size_t growth) { return growth + static_cast<size_t>((static_cast<int64_t>(growth) - 1) / 7); } +inline void AssertIsFull(ctrl_t* ctrl) { + ABSL_HARDENING_ASSERT((ctrl != nullptr && IsFull(*ctrl)) && + "Invalid operation on iterator. The element might have " + "been erased, or the table might have rehashed."); +} + +inline void AssertIsValid(ctrl_t* ctrl) { + ABSL_HARDENING_ASSERT((ctrl == nullptr || IsFull(*ctrl)) && + "Invalid operation on iterator. The element might have " + "been erased, or the table might have rehashed."); +} + // Policy: a policy defines how to perform different operations on // the slots of the hashtable (see hash_policy_traits.h for the full interface // of policy). @@ -617,7 +629,7 @@ class raw_hash_set { // PRECONDITION: not an end() iterator. reference operator*() const { - assert_is_full(); + AssertIsFull(ctrl_); return PolicyTraits::element(slot_); } @@ -626,7 +638,7 @@ class raw_hash_set { // PRECONDITION: not an end() iterator. iterator& operator++() { - assert_is_full(); + AssertIsFull(ctrl_); ++ctrl_; ++slot_; skip_empty_or_deleted(); @@ -640,8 +652,8 @@ class raw_hash_set { } friend bool operator==(const iterator& a, const iterator& b) { - a.assert_is_valid(); - b.assert_is_valid(); + AssertIsValid(a.ctrl_); + AssertIsValid(b.ctrl_); return a.ctrl_ == b.ctrl_; } friend bool operator!=(const iterator& a, const iterator& b) { @@ -655,13 +667,6 @@ class raw_hash_set { ABSL_INTERNAL_ASSUME(ctrl != nullptr); } - void assert_is_full() const { - ABSL_HARDENING_ASSERT(ctrl_ != nullptr && IsFull(*ctrl_)); - } - void assert_is_valid() const { - ABSL_HARDENING_ASSERT(ctrl_ == nullptr || IsFull(*ctrl_)); - } - void skip_empty_or_deleted() { while (IsEmptyOrDeleted(*ctrl_)) { uint32_t shift = Group{ctrl_}.CountLeadingEmptyOrDeleted(); @@ -1174,7 +1179,7 @@ class raw_hash_set { // This overload is necessary because otherwise erase<K>(const K&) would be // a better match if non-const iterator is passed as an argument. void erase(iterator it) { - it.assert_is_full(); + AssertIsFull(it.ctrl_); PolicyTraits::destroy(&alloc_ref(), it.slot_); erase_meta_only(it); } @@ -1208,7 +1213,7 @@ class raw_hash_set { } node_type extract(const_iterator position) { - position.inner_.assert_is_full(); + AssertIsFull(position.inner_.ctrl_); auto node = CommonAccess::Transfer<node_type>(alloc_ref(), position.inner_.slot_); erase_meta_only(position); diff --git a/absl/container/internal/raw_hash_set_test.cc b/absl/container/internal/raw_hash_set_test.cc index 2fc85591..6befa960 100644 --- a/absl/container/internal/raw_hash_set_test.cc +++ b/absl/container/internal/raw_hash_set_test.cc @@ -1791,7 +1791,7 @@ TEST(TableDeathTest, EraseOfEndAsserts) { IntTable t; // Extra simple "regexp" as regexp support is highly varied across platforms. - constexpr char kDeathMsg[] = "IsFull"; + constexpr char kDeathMsg[] = "Invalid operation on iterator"; EXPECT_DEATH_IF_SUPPORTED(t.erase(t.end()), kDeathMsg); } diff --git a/absl/types/span.h b/absl/types/span.h index 4e450fc9..95fe7926 100644 --- a/absl/types/span.h +++ b/absl/types/span.h @@ -17,10 +17,13 @@ // span.h // ----------------------------------------------------------------------------- // -// This header file defines a `Span<T>` type for holding a view of an existing -// array of data. The `Span` object, much like the `absl::string_view` object, -// does not own such data itself. A span provides a lightweight way to pass -// around view of such data. +// This header file defines a `Span<T>` type for holding a reference to existing +// array data. The `Span` object, much like the `absl::string_view` object, +// does not own such data itself, and the data being referenced by the span must +// outlive the span itself. Unlike `view` type references, a span can hold a +// reference to mutable data (and can mutate it for underlying types of +// non-const T.) A span provides a lightweight way to pass a reference to such +// data. // // Additionally, this header file defines `MakeSpan()` and `MakeConstSpan()` // factory functions, for clearly creating spans of type `Span<T>` or read-only @@ -72,9 +75,9 @@ ABSL_NAMESPACE_BEGIN // Span //------------------------------------------------------------------------------ // -// A `Span` is an "array view" type for holding a view of a contiguous data -// array; the `Span` object does not and cannot own such data itself. A span -// provides an easy way to provide overloads for anything operating on +// A `Span` is an "array reference" type for holding a reference of contiguous +// array data; the `Span` object does not and cannot own such data itself. A +// span provides an easy way to provide overloads for anything operating on // contiguous sequences without needing to manage pointers and array lengths // manually. @@ -92,7 +95,8 @@ ABSL_NAMESPACE_BEGIN // constructors. // // A `Span<T>` is somewhat analogous to an `absl::string_view`, but for an array -// of elements of type `T`. A user of `Span` must ensure that the data being +// of elements of type `T`, and unlike an `absl::string_view`, a span can hold a +// reference to mutable data. A user of `Span` must ensure that the data being // pointed to outlives the `Span` itself. // // You can construct a `Span<T>` in several ways: @@ -122,7 +126,7 @@ ABSL_NAMESPACE_BEGIN // Note that `Span` objects, in addition to requiring that the memory they // point to remains alive, must also ensure that such memory does not get // reallocated. Therefore, to avoid undefined behavior, containers with -// associated span views should not invoke operations that may reallocate memory +// associated spans should not invoke operations that may reallocate memory // (such as resizing) or invalidate iterators into the container. // // One common use for a `Span` is when passing arguments to a routine that can |