Skip to content

Commit

Permalink
Export of internal Abseil changes
Browse files Browse the repository at this point in the history
--
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
  • Loading branch information
Abseil Team authored and suertreus committed Jun 17, 2020
1 parent ccdbb59 commit 4a85104
Show file tree
Hide file tree
Showing 6 changed files with 126 additions and 39 deletions.
54 changes: 54 additions & 0 deletions absl/container/btree_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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
Expand Down
52 changes: 38 additions & 14 deletions absl/container/internal/btree.h
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
};

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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().
Expand Down Expand Up @@ -1184,23 +1190,32 @@ 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
// amortized constant time. If not, the insertion will take amortized
// 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>
Expand Down Expand Up @@ -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);
Expand All @@ -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()) {
Expand All @@ -1906,13 +1921,22 @@ 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 {
Expand Down
4 changes: 2 additions & 2 deletions absl/container/internal/btree_container.h
Original file line number Diff line number Diff line change
Expand Up @@ -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()};
Expand Down
31 changes: 18 additions & 13 deletions absl/container/internal/raw_hash_set.h
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down Expand Up @@ -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_);
}

Expand All @@ -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();
Expand All @@ -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) {
Expand All @@ -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();
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion absl/container/internal/raw_hash_set_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
22 changes: 13 additions & 9 deletions absl/types/span.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.

Expand All @@ -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:
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 4a85104

Please sign in to comment.