From 1c2ea908abd8944ac306a3af0a3a643633615b38 Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Tue, 21 Jan 2025 12:58:46 -0800 Subject: [PATCH] Modify GetEmitRepeatedFieldMutableSub to take in a FieldDescriptor instead of a boolean and ensure split is enabled for string piece. PiperOrigin-RevId: 718036356 --- .../cpp/field_generators/message_field.cc | 2 +- .../cpp/field_generators/string_field.cc | 2 +- .../cpp/field_generators/string_view_field.cc | 2 +- src/google/protobuf/compiler/cpp/helpers.h | 17 +++++++++++------ 4 files changed, 14 insertions(+), 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 index 30496c9522ac4..b6f9bd169c0fa 100644 --- 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 index f135cd2f76462..071c71be5461f 100644 --- 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 index 7c85ca9fa7974..27fa4dac2fa5b 100644 --- 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 index d7092c51e3751..c7eb7632d8a3c 100644 --- a/src/google/protobuf/compiler/cpp/helpers.h +++ b/src/google/protobuf/compiler/cpp/helpers.h @@ -1216,18 +1216,23 @@ 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( @@ -1235,7 +1240,7 @@ inline auto GetEmitRepeatedFieldMutableSub(const Options& options, } break; case BoundsCheckMode::kAbort: - if (use_stringpiecefield) { + if (is_stringpiecefield) { p->Emit("$pbi$::CheckedMutableOrAbort(&$field$, index)"); } else { p->Emit(R"cc(