Refactor RuntimeAssertInBounds to remove repeated logic and make Get/Mutable easier to read.

protobuf-github-bot · copybara-github · commit 2f270c49a288 · 2025-09-08T16:59:03.000-07:00
PiperOrigin-RevId: 804640388
diff --git a/src/google/protobuf/no_field_presence_map_test.cc b/src/google/protobuf/no_field_presence_map_test.cc
@@ -133,7 +133,7 @@ TEST(NoFieldPresenceTest, GenCodeMapReflectionMissingKeyDeathTest) {
 
   // Trying to get an unset map entry would crash with a DCHECK in debug mode.
   EXPECT_DEATH(r->GetRepeatedMessage(message, field_map_int32_bytes, 0),
-               "index < current_size_");
+               "index < size");
 }
 #endif
 
diff --git a/src/google/protobuf/repeated_field.h b/src/google/protobuf/repeated_field.h
@@ -830,13 +830,7 @@ inline void RepeatedField<Element>::Resize(int new_size, const Element& value) {
 template <typename Element>
 inline const Element& RepeatedField<Element>::Get(int index) const
     ABSL_ATTRIBUTE_LIFETIME_BOUND {
-  if constexpr (internal::GetBoundsCheckMode() ==
-                internal::BoundsCheckMode::kAbort) {
     internal::RuntimeAssertInBounds(index, size());
-  } else {
-    ABSL_DCHECK_GE(index, 0);
-    ABSL_DCHECK_LT(index, size());
-  }
   return elements(is_soo())[index];
 }
 
@@ -859,13 +853,7 @@ inline Element& RepeatedField<Element>::at(int index)
 template <typename Element>
 inline Element* RepeatedField<Element>::Mutable(int index)
     ABSL_ATTRIBUTE_LIFETIME_BOUND {
-  if constexpr (internal::GetBoundsCheckMode() ==
-                internal::BoundsCheckMode::kAbort) {
     internal::RuntimeAssertInBounds(index, size());
-  } else {
-    ABSL_DCHECK_GE(index, 0);
-    ABSL_DCHECK_LT(index, size());
-  }
   return &elements(is_soo())[index];
 }
 
diff --git a/src/google/protobuf/repeated_ptr_field.h b/src/google/protobuf/repeated_ptr_field.h
@@ -107,9 +107,13 @@ PROTOBUF_EXPORT void LogIndexOutOfBounds(int index, int size);
 PROTOBUF_PRESERVE_ALL PROTOBUF_EXPORT void LogIndexOutOfBoundsAndAbort(
     int index, int size);
 PROTOBUF_EXPORT inline void RuntimeAssertInBounds(int index, int size) {
-  if (ABSL_PREDICT_FALSE(index < 0 || index >= size)) {
-    LogIndexOutOfBoundsAndAbort(index, size);
+  if constexpr (GetBoundsCheckMode() == BoundsCheckMode::kAbort) {
+    if (ABSL_PREDICT_FALSE(index < 0 || index >= size)) {
+      LogIndexOutOfBoundsAndAbort(index, size);
+    }
   }
+  ABSL_DCHECK_GE(index, 0);
+  ABSL_DCHECK_LT(index, size);
 }
 
 // Defined further below.
@@ -196,12 +200,7 @@ class PROTOBUF_EXPORT RepeatedPtrFieldBase {
 
   template <typename TypeHandler>
   Value<TypeHandler>* Mutable(int index) {
-    if constexpr (GetBoundsCheckMode() == BoundsCheckMode::kAbort) {
       RuntimeAssertInBounds(index, current_size_);
-    } else {
-      ABSL_DCHECK_GE(index, 0);
-      ABSL_DCHECK_LT(index, current_size_);
-    }
     return cast<TypeHandler>(element_at(index));
   }
 
@@ -271,14 +270,10 @@ class PROTOBUF_EXPORT RepeatedPtrFieldBase {
           return TypeHandler::default_instance();
         }
       }
-    } else if constexpr (GetBoundsCheckMode() == BoundsCheckMode::kAbort) {
-      // We refactor this to a separate function instead of inlining it so we
-      // can measure the performance impact more easily.
-      RuntimeAssertInBounds(index, current_size_);
-    } else {
-      ABSL_DCHECK_GE(index, 0);
-      ABSL_DCHECK_LT(index, current_size_);
     }
+    // We refactor this to a separate function instead of inlining it so we
+    // can measure the performance impact more easily.
+    RuntimeAssertInBounds(index, current_size_);
     return *cast<TypeHandler>(element_at(index));
   }
 
