diff --git a/src/google/protobuf/repeated_field.h b/src/google/protobuf/repeated_field.h index 151c0b6cfc..66c2351fae 100644 --- a/src/google/protobuf/repeated_field.h +++ b/src/google/protobuf/repeated_field.h @@ -387,9 +387,9 @@ class RepeatedField final // Reserves space to expand the field to at least the given size. // If the array is grown, it will always be at least doubled in size. // If `annotate_size` is true (the default), then this function will annotate - // the old container from `old_size` to `capacity_` (unpoison memory) + // the old container from `old_size` to `Capacity()` (unpoison memory) // directly before it is being released, and annotate the new container from - // `capacity_` to `old_size` (poison unused memory). + // `Capacity()` to `old_size` (poison unused memory). void Grow(int old_size, int new_size); void GrowNoAnnotate(int old_size, int new_size); @@ -410,10 +410,10 @@ class RepeatedField final } } - // Replaces size_ with new_size and returns the previous value of - // size_. This function is intended to be the only place where - // size_ is modified, with the exception of `AddInputIterator()` - // where the size of added items is not known in advance. + // Replaces size with new_size and returns the previous value of size. This + // function is intended to be the only place where size is modified, with the + // exception of `AddInputIterator()` where the size of added items is not + // known in advance. inline int ExchangeCurrentSize(int new_size) { const int prev_size = size(); AnnotateSize(prev_size, new_size); @@ -422,7 +422,7 @@ class RepeatedField final } // Returns a pointer to elements array. - // pre-condition: the array must have been allocated. + // pre-condition: Capacity() > 0. Element* elements() const { ABSL_DCHECK_GT(Capacity(), 0); // Because of above pre-condition this cast is safe. @@ -612,12 +612,14 @@ inline Element* RepeatedField::AddNAlreadyReserved(int n) template inline void RepeatedField::Resize(int new_size, const Element& value) { ABSL_DCHECK_GE(new_size, 0); - if (new_size > size()) { - if (new_size > Capacity()) Grow(size(), new_size); + const int old_size = size(); + if (new_size > old_size) { + if (new_size > Capacity()) Grow(old_size, new_size); Element* first = elements() + ExchangeCurrentSize(new_size); - std::uninitialized_fill(first, elements() + size(), value); - } else if (new_size < size()) { - Destroy(unsafe_elements() + new_size, unsafe_elements() + size()); + std::uninitialized_fill(first, elements() + new_size, value); + } else if (new_size < old_size) { + Element* elem = unsafe_elements(); + Destroy(elem + new_size, elem + old_size); ExchangeCurrentSize(new_size); } } @@ -663,14 +665,15 @@ inline void RepeatedField::Set(int index, const Element& value) { template inline void RepeatedField::Add(Element value) { + const int old_size = size(); int capacity = Capacity(); Element* elem = unsafe_elements(); - if (ABSL_PREDICT_FALSE(size() == capacity)) { - Grow(size(), size() + 1); + if (ABSL_PREDICT_FALSE(old_size == capacity)) { + Grow(old_size, old_size + 1); capacity = Capacity(); elem = unsafe_elements(); } - int new_size = size() + 1; + int new_size = old_size + 1; void* p = elem + ExchangeCurrentSize(new_size); ::new (p) Element(std::move(value)); @@ -682,21 +685,23 @@ inline void RepeatedField::Add(Element value) { template inline Element* RepeatedField::Add() ABSL_ATTRIBUTE_LIFETIME_BOUND { - if (ABSL_PREDICT_FALSE(size() == Capacity())) { - Grow(size(), size() + 1); + const int old_size = size(); + if (ABSL_PREDICT_FALSE(old_size == Capacity())) { + Grow(old_size, old_size + 1); } - void* p = unsafe_elements() + ExchangeCurrentSize(size() + 1); + void* p = unsafe_elements() + ExchangeCurrentSize(old_size + 1); return ::new (p) Element; } template template inline void RepeatedField::AddForwardIterator(Iter begin, Iter end) { + const int old_size = size(); int capacity = Capacity(); Element* elem = unsafe_elements(); - int new_size = size() + static_cast(std::distance(begin, end)); + int new_size = old_size + static_cast(std::distance(begin, end)); if (ABSL_PREDICT_FALSE(new_size > capacity)) { - Grow(size(), new_size); + Grow(old_size, new_size); elem = unsafe_elements(); capacity = Capacity(); } @@ -711,24 +716,27 @@ inline void RepeatedField::AddForwardIterator(Iter begin, Iter end) { template template inline void RepeatedField::AddInputIterator(Iter begin, Iter end) { - Element* first = unsafe_elements() + size(); - Element* last = unsafe_elements() + Capacity(); + Element* elem = unsafe_elements(); + Element* first = elem + size(); + Element* last = elem + Capacity(); AnnotateSize(size(), Capacity()); while (begin != end) { if (ABSL_PREDICT_FALSE(first == last)) { - int size = first - unsafe_elements(); + int size = first - elem; GrowNoAnnotate(size, size + 1); - first = unsafe_elements() + size; - last = unsafe_elements() + Capacity(); + elem = unsafe_elements(); + first = elem + size; + last = elem + Capacity(); } ::new (static_cast(first)) Element(*begin); ++begin; ++first; } - set_size(first - unsafe_elements()); - AnnotateSize(Capacity(), size()); + const int new_size = first - elem; + set_size(new_size); + AnnotateSize(Capacity(), new_size); } template @@ -745,9 +753,10 @@ inline void RepeatedField::Add(Iter begin, Iter end) { template inline void RepeatedField::RemoveLast() { - ABSL_DCHECK_GT(size(), 0); - elements()[size() - 1].~Element(); - ExchangeCurrentSize(size() - 1); + const int old_size = size(); + ABSL_DCHECK_GT(old_size, 0); + elements()[old_size - 1].~Element(); + ExchangeCurrentSize(old_size - 1); } template @@ -755,7 +764,8 @@ void RepeatedField::ExtractSubrange(int start, int num, Element* elements) { ABSL_DCHECK_GE(start, 0); ABSL_DCHECK_GE(num, 0); - ABSL_DCHECK_LE(start + num, size()); + const int old_size = size(); + ABSL_DCHECK_LE(start + num, old_size); // Save the values of the removed elements if requested. if (elements != nullptr) { @@ -764,24 +774,26 @@ void RepeatedField::ExtractSubrange(int start, int num, // Slide remaining elements down to fill the gap. if (num > 0) { - for (int i = start + num; i < size(); ++i) Set(i - num, Get(i)); - Truncate(size() - num); + for (int i = start + num; i < old_size; ++i) Set(i - num, Get(i)); + Truncate(old_size - num); } } template inline void RepeatedField::Clear() { - Destroy(unsafe_elements(), unsafe_elements() + size()); + Element* elem = unsafe_elements(); + Destroy(elem, elem + size()); ExchangeCurrentSize(0); } template inline void RepeatedField::MergeFrom(const RepeatedField& other) { ABSL_DCHECK_NE(&other, this); - if (auto size = other.size()) { - Reserve(this->size() + size); - Element* dst = elements() + ExchangeCurrentSize(this->size() + size); - UninitializedCopyN(other.elements(), size, dst); + if (auto other_size = other.size()) { + const int old_size = size(); + Reserve(old_size + other_size); + Element* dst = elements() + ExchangeCurrentSize(old_size + other_size); + UninitializedCopyN(other.elements(), other_size, dst); } } @@ -905,8 +917,8 @@ RepeatedField::cend() const ABSL_ATTRIBUTE_LIFETIME_BOUND { template inline size_t RepeatedField::SpaceUsedExcludingSelfLong() const { - return Capacity() > 0 ? (Capacity() * sizeof(Element) + kHeapRepHeaderSize) - : 0; + const int capacity = Capacity(); + return capacity > 0 ? capacity * sizeof(Element) + kHeapRepHeaderSize : 0; } namespace internal { @@ -1016,9 +1028,11 @@ PROTOBUF_NOINLINE void RepeatedField::Grow(int old_size, template inline void RepeatedField::Truncate(int new_size) { - ABSL_DCHECK_LE(new_size, size()); - if (new_size < size()) { - Destroy(unsafe_elements() + new_size, unsafe_elements() + size()); + const int old_size = size(); + ABSL_DCHECK_LE(new_size, old_size); + if (new_size < old_size) { + Element* elem = unsafe_elements(); + Destroy(elem + new_size, elem + old_size); ExchangeCurrentSize(new_size); } } diff --git a/src/google/protobuf/repeated_field_unittest.cc b/src/google/protobuf/repeated_field_unittest.cc index 4b4d328111..50e43f594e 100644 --- a/src/google/protobuf/repeated_field_unittest.cc +++ b/src/google/protobuf/repeated_field_unittest.cc @@ -1241,8 +1241,8 @@ TEST(RepeatedField, HardenAgainstBadTruncate) { for (int size = 0; size < 10; ++size) { field.Truncate(size); #if GTEST_HAS_DEATH_TEST - EXPECT_DEBUG_DEATH(field.Truncate(size + 1), "new_size <= size"); - EXPECT_DEBUG_DEATH(field.Truncate(size + 2), "new_size <= size"); + EXPECT_DEBUG_DEATH(field.Truncate(size + 1), "new_size <= old_size"); + EXPECT_DEBUG_DEATH(field.Truncate(size + 2), "new_size <= old_size"); #elif defined(NDEBUG) field.Truncate(size + 1); field.Truncate(size + 1);