This is an automated email from the ASF dual-hosted git repository.
wwbmmm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/brpc.git
The following commit(s) were added to refs/heads/master by this push:
new f97cbe5c Fix FlatMap assign operator bug (#2622)
f97cbe5c is described below
commit f97cbe5c65de314dd4b7db18b0900c68cf499f8a
Author: Bright Chen <[email protected]>
AuthorDate: Mon Apr 29 10:17:44 2024 +0800
Fix FlatMap assign operator bug (#2622)
---
src/butil/containers/flat_map.h | 16 ++++++++--------
src/butil/containers/flat_map_inl.h | 28 +++++++++++++++++++---------
test/flat_map_unittest.cpp | 15 +++++++++++++++
3 files changed, 42 insertions(+), 17 deletions(-)
diff --git a/src/butil/containers/flat_map.h b/src/butil/containers/flat_map.h
index 9bfd8ec1..50f44540 100644
--- a/src/butil/containers/flat_map.h
+++ b/src/butil/containers/flat_map.h
@@ -151,12 +151,12 @@ public:
key_type key;
};
- FlatMap(const hasher& hashfn = hasher(),
- const key_equal& eql = key_equal(),
- const allocator_type& alloc = allocator_type());
+ explicit FlatMap(const hasher& hashfn = hasher(),
+ const key_equal& eql = key_equal(),
+ const allocator_type& alloc = allocator_type());
~FlatMap();
- FlatMap(const FlatMap& rhs);
- void operator=(const FlatMap& rhs);
+ FlatMap(const FlatMap& rhs);
+ FlatMap& operator=(const FlatMap& rhs);
void swap(FlatMap & rhs);
// Must be called to initialize this map, otherwise insert/operator[]
@@ -307,9 +307,9 @@ public:
typedef typename Map::key_equal key_equal;
typedef typename Map::allocator_type allocator_type;
- FlatSet(const hasher& hashfn = hasher(),
- const key_equal& eql = key_equal(),
- const allocator_type& alloc = allocator_type())
+ explicit FlatSet(const hasher& hashfn = hasher(),
+ const key_equal& eql = key_equal(),
+ const allocator_type& alloc = allocator_type())
: _map(hashfn, eql, alloc) {}
void swap(FlatSet & rhs) { _map.swap(rhs._map); }
diff --git a/src/butil/containers/flat_map_inl.h
b/src/butil/containers/flat_map_inl.h
index 264d28c7..65830368 100644
--- a/src/butil/containers/flat_map_inl.h
+++ b/src/butil/containers/flat_map_inl.h
@@ -261,20 +261,19 @@ FlatMap<_K, _T, _H, _E, _S, _A>::FlatMap(const FlatMap&
rhs)
}
template <typename _K, typename _T, typename _H, typename _E, bool _S,
typename _A>
-void
+FlatMap<_K, _T, _H, _E, _S, _A>&
FlatMap<_K, _T, _H, _E, _S, _A>::operator=(const FlatMap<_K, _T, _H, _E, _S,
_A>& rhs) {
if (this == &rhs) {
- return;
+ return *this;
}
// NOTE: assignment does not change _load_factor/_hashfn/_eql if |this| is
// initialized
clear();
- if (rhs.empty()) {
- return;
- }
- if (!initialized()) {
- _load_factor = rhs._load_factor;
+ if (!rhs.initialized()) {
+ return *this;
}
+ bool need_copy = !rhs.empty();
+ _load_factor = rhs._load_factor;
if (_buckets == NULL || is_too_crowded(rhs._size)) {
get_allocator().Free(_buckets);
_nbucket = rhs._nbucket;
@@ -282,18 +281,28 @@ FlatMap<_K, _T, _H, _E, _S, _A>::operator=(const
FlatMap<_K, _T, _H, _E, _S, _A>
_buckets = (Bucket*)get_allocator().Alloc(sizeof(Bucket) * (_nbucket +
1/*note*/));
if (NULL == _buckets) {
LOG(ERROR) << "Fail to new _buckets";
- return;
+ return *this;
+ }
+ // If no need to copy, set buckets invalid.
+ if (!need_copy) {
+ for (size_t i = 0; i < _nbucket; ++i) {
+ _buckets[i].set_invalid();
+ }
+ _buckets[_nbucket].next = NULL;
}
if (_S) {
free(_thumbnail);
_thumbnail = bit_array_malloc(_nbucket);
if (NULL == _thumbnail) {
LOG(ERROR) << "Fail to new _thumbnail";
- return;
+ return *this;
}
bit_array_clear(_thumbnail, _nbucket);
}
}
+ if (!need_copy) {
+ return *this;
+ }
if (_nbucket == rhs._nbucket) {
// For equivalent _nbucket, walking through _buckets instead of using
// iterators is more efficient.
@@ -322,6 +331,7 @@ FlatMap<_K, _T, _H, _E, _S, _A>::operator=(const
FlatMap<_K, _T, _H, _E, _S, _A>
Element::second_ref_from_value(*it);
}
}
+ return *this;
}
template <typename _K, typename _T, typename _H, typename _E, bool _S,
typename _A>
diff --git a/test/flat_map_unittest.cpp b/test/flat_map_unittest.cpp
index 6ba7b3ec..d689bdf1 100644
--- a/test/flat_map_unittest.cpp
+++ b/test/flat_map_unittest.cpp
@@ -1332,6 +1332,21 @@ TEST_F(FlatMapTest, copy) {
m2 = m1;
ASSERT_FALSE(m1.is_too_crowded(m1.size()));
ASSERT_FALSE(m2.is_too_crowded(m1.size()));
+
+ butil::FlatMap<int, int> m3;
+ ASSERT_FALSE(m3.initialized());
+ m1 = m3;
+ ASSERT_TRUE(m1.empty());
+ ASSERT_TRUE(m1.initialized());
+
+ m3 = m2;
+ ASSERT_TRUE(m3.initialized());
+ m3.clear();
+ ASSERT_TRUE(m3.initialized());
+ ASSERT_TRUE(m3.empty());
+ butil::FlatMap<int, int> m4 = m3;
+ ASSERT_TRUE(m4.initialized());
+ ASSERT_TRUE(m4.empty());
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
