Skip to content

Commit 2f270c4

Browse files
Refactor RuntimeAssertInBounds to remove repeated logic and make Get/Mutable easier to read.
PiperOrigin-RevId: 804640388
1 parent 89041ca commit 2f270c4

File tree

3 files changed

+10
-27
lines changed

3 files changed

+10
-27
lines changed

src/google/protobuf/no_field_presence_map_test.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ TEST(NoFieldPresenceTest, GenCodeMapReflectionMissingKeyDeathTest) {
133133

134134
// Trying to get an unset map entry would crash with a DCHECK in debug mode.
135135
EXPECT_DEATH(r->GetRepeatedMessage(message, field_map_int32_bytes, 0),
136-
"index < current_size_");
136+
"index < size");
137137
}
138138
#endif
139139

src/google/protobuf/repeated_field.h

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -830,13 +830,7 @@ inline void RepeatedField<Element>::Resize(int new_size, const Element& value) {
830830
template <typename Element>
831831
inline const Element& RepeatedField<Element>::Get(int index) const
832832
ABSL_ATTRIBUTE_LIFETIME_BOUND {
833-
if constexpr (internal::GetBoundsCheckMode() ==
834-
internal::BoundsCheckMode::kAbort) {
835833
internal::RuntimeAssertInBounds(index, size());
836-
} else {
837-
ABSL_DCHECK_GE(index, 0);
838-
ABSL_DCHECK_LT(index, size());
839-
}
840834
return elements(is_soo())[index];
841835
}
842836

@@ -859,13 +853,7 @@ inline Element& RepeatedField<Element>::at(int index)
859853
template <typename Element>
860854
inline Element* RepeatedField<Element>::Mutable(int index)
861855
ABSL_ATTRIBUTE_LIFETIME_BOUND {
862-
if constexpr (internal::GetBoundsCheckMode() ==
863-
internal::BoundsCheckMode::kAbort) {
864856
internal::RuntimeAssertInBounds(index, size());
865-
} else {
866-
ABSL_DCHECK_GE(index, 0);
867-
ABSL_DCHECK_LT(index, size());
868-
}
869857
return &elements(is_soo())[index];
870858
}
871859

src/google/protobuf/repeated_ptr_field.h

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -107,9 +107,13 @@ PROTOBUF_EXPORT void LogIndexOutOfBounds(int index, int size);
107107
PROTOBUF_PRESERVE_ALL PROTOBUF_EXPORT void LogIndexOutOfBoundsAndAbort(
108108
int index, int size);
109109
PROTOBUF_EXPORT inline void RuntimeAssertInBounds(int index, int size) {
110-
if (ABSL_PREDICT_FALSE(index < 0 || index >= size)) {
111-
LogIndexOutOfBoundsAndAbort(index, size);
110+
if constexpr (GetBoundsCheckMode() == BoundsCheckMode::kAbort) {
111+
if (ABSL_PREDICT_FALSE(index < 0 || index >= size)) {
112+
LogIndexOutOfBoundsAndAbort(index, size);
113+
}
112114
}
115+
ABSL_DCHECK_GE(index, 0);
116+
ABSL_DCHECK_LT(index, size);
113117
}
114118

115119
// Defined further below.
@@ -196,12 +200,7 @@ class PROTOBUF_EXPORT RepeatedPtrFieldBase {
196200

197201
template <typename TypeHandler>
198202
Value<TypeHandler>* Mutable(int index) {
199-
if constexpr (GetBoundsCheckMode() == BoundsCheckMode::kAbort) {
200203
RuntimeAssertInBounds(index, current_size_);
201-
} else {
202-
ABSL_DCHECK_GE(index, 0);
203-
ABSL_DCHECK_LT(index, current_size_);
204-
}
205204
return cast<TypeHandler>(element_at(index));
206205
}
207206

@@ -271,14 +270,10 @@ class PROTOBUF_EXPORT RepeatedPtrFieldBase {
271270
return TypeHandler::default_instance();
272271
}
273272
}
274-
} else if constexpr (GetBoundsCheckMode() == BoundsCheckMode::kAbort) {
275-
// We refactor this to a separate function instead of inlining it so we
276-
// can measure the performance impact more easily.
277-
RuntimeAssertInBounds(index, current_size_);
278-
} else {
279-
ABSL_DCHECK_GE(index, 0);
280-
ABSL_DCHECK_LT(index, current_size_);
281273
}
274+
// We refactor this to a separate function instead of inlining it so we
275+
// can measure the performance impact more easily.
276+
RuntimeAssertInBounds(index, current_size_);
282277
return *cast<TypeHandler>(element_at(index));
283278
}
284279

0 commit comments

Comments
 (0)