Modify GetEmitRepeatedFieldMutableSub to take in a FieldDescriptor in…

…stead of a boolean and ensure split is enabled for string piece. PiperOrigin-RevId: 718036356
protocolbuffers · Jan 24, 2025 · 1c2ea90 · 1c2ea90
1 parent b9ec7e0
commit 1c2ea90
Show file tree

Hide file tree

Showing 4 changed files with 14 additions and 9 deletions.
diff --git a/src/google/protobuf/compiler/cpp/field_generators/message_field.cc b/src/google/protobuf/compiler/cpp/field_generators/message_field.cc
@@ -783,7 +783,7 @@ void RepeatedMessage::GenerateAccessorDeclarations(io::Printer* p) const {
 
 void RepeatedMessage::GenerateInlineAccessorDefinitions(io::Printer* p) const {
   // TODO: move insertion points
-  p->Emit({GetEmitRepeatedFieldMutableSub(*opts_, p)},
+  p->Emit({GetEmitRepeatedFieldMutableSub(*opts_, p, field_)},
           R"cc(
             inline $Submsg$* $nonnull$ $Msg$::mutable_$name$(int index)
                 ABSL_ATTRIBUTE_LIFETIME_BOUND {

diff --git a/src/google/protobuf/compiler/cpp/field_generators/string_field.cc b/src/google/protobuf/compiler/cpp/field_generators/string_field.cc
@@ -858,7 +858,7 @@ void RepeatedString::GenerateInlineAccessorDefinitions(io::Printer* p) const {
                p->Emit(", $pbi$::BytesTag{}");
              }
            }},
-          GetEmitRepeatedFieldMutableSub(*opts_, p),
+          GetEmitRepeatedFieldMutableSub(*opts_, p, field_),
       },
       R"cc(
         inline std::string* $nonnull$ $Msg$::add_$name$()

diff --git a/src/google/protobuf/compiler/cpp/field_generators/string_view_field.cc b/src/google/protobuf/compiler/cpp/field_generators/string_view_field.cc
@@ -669,7 +669,7 @@ void RepeatedStringView::GenerateInlineAccessorDefinitions(
   p->Emit(
       {
           {GetEmitRepeatedFieldGetterSub(*opts_, p)},
-          {GetEmitRepeatedFieldMutableSub(*opts_, p)},
+          {GetEmitRepeatedFieldMutableSub(*opts_, p, field_)},
       },
       R"cc(
         inline absl::string_view $Msg$::$name$(int index) const

diff --git a/src/google/protobuf/compiler/cpp/helpers.h b/src/google/protobuf/compiler/cpp/helpers.h
@@ -1216,26 +1216,31 @@ inline auto GetEmitRepeatedFieldGetterSub(const Options& options,
 // Emit the code for getting a mutable element from a repeated field. This will
 // generate different code depending on the `bounds_check_mode` specified in the
 // options.
-// TODO: b/347304492 Harden this function by taking in the field and checking
-// if splitting is supported.
 inline auto GetEmitRepeatedFieldMutableSub(const Options& options,
                                            io::Printer* p,
-                                           bool use_stringpiecefield = false) {
+                                           const FieldDescriptor* field) {
+  bool is_stringpiecefield =
+      field->cpp_type() == FieldDescriptor::CPPTYPE_STRING &&
+      field->options().ctype() == FieldOptions::STRING;
+
+  if (is_stringpiecefield) {
+    // ABSL_CHECK(!IsSplitEnabledForField(field, options));
+  }
   return io::Printer::Sub{
       "mutable",
-      [&options, p, use_stringpiecefield] {
+      [&options, p, is_stringpiecefield] {
         switch (options.bounds_check_mode) {
           case BoundsCheckMode::kNoEnforcement:
           case BoundsCheckMode::kReturnDefaultValue:
-            if (use_stringpiecefield) {
+            if (is_stringpiecefield) {
               p->Emit("$field$.Mutable(index)");
             } else {
               p->Emit(
                   R"cc(_internal_mutable_$name_internal$()->Mutable(index))cc");
             }
             break;
           case BoundsCheckMode::kAbort:
-            if (use_stringpiecefield) {
+            if (is_stringpiecefield) {
               p->Emit("$pbi$::CheckedMutableOrAbort(&$field$, index)");
             } else {
               p->Emit(R"cc(