Fix a bug in handling of implicit-presence string_view fields. (#20403)

mkruskal-google · web-flow · commit 81196acc7e3e · 2025-02-19T14:52:33.000-08:00
PiperOrigin-RevId: 728814630
diff --git a/src/google/protobuf/compiler/cpp/helpers.h b/src/google/protobuf/compiler/cpp/helpers.h
@@ -358,6 +358,11 @@ inline bool IsString(const FieldDescriptor* field) {
 }
 
 
+inline bool IsArenaStringPtr(const FieldDescriptor* field) {
+  return field->cpp_string_type() == FieldDescriptor::CppStringType::kString ||
+         field->cpp_string_type() == FieldDescriptor::CppStringType::kView;
+}
+
 bool IsProfileDriven(const Options& options);
 
 // Returns true if `field` is unlikely to be present based on PDProto profile.
diff --git a/src/google/protobuf/compiler/cpp/message.cc b/src/google/protobuf/compiler/cpp/message.cc
@@ -292,8 +292,7 @@ void EmitNonDefaultCheckForString(io::Printer* p, absl::string_view prefix,
                                   const FieldDescriptor* field, bool split,
                                   absl::AnyInvocable<void()> emit_body) {
   ABSL_DCHECK(field->cpp_type() == FieldDescriptor::CPPTYPE_STRING);
-  ABSL_DCHECK(field->cpp_string_type() ==
-              FieldDescriptor::CppStringType::kString);
+  ABSL_DCHECK(IsArenaStringPtr(field));
   p->Emit(
       {
           {"condition", [&] { EmitNonDefaultCheck(p, prefix, field); }},
@@ -392,7 +391,7 @@ void MayEmitMutableIfNonDefaultCheck(io::Printer* p, absl::string_view prefix,
                                      bool with_enclosing_braces_always) {
   if (ShouldEmitNonDefaultCheck(field)) {
     if (field->cpp_type() == FieldDescriptor::CPPTYPE_STRING &&
-        field->cpp_string_type() == FieldDescriptor::CppStringType::kString) {
+        IsArenaStringPtr(field)) {
       // If a field is backed by std::string, when default initialized it will
       // point to a global empty std::string instance. We prefer to spend some
       // extra cycles here to create a local string instance in the else branch,
diff --git a/src/google/protobuf/compiler/cpp/tracker.cc b/src/google/protobuf/compiler/cpp/tracker.cc
@@ -13,7 +13,6 @@
 
 #include "absl/log/absl_check.h"
 #include "absl/strings/str_cat.h"
-#include "absl/strings/str_format.h"
 #include "absl/strings/string_view.h"
 #include "absl/strings/substitute.h"
 #include "absl/types/optional.h"
diff --git a/src/google/protobuf/string_view_test.cc b/src/google/protobuf/string_view_test.cc
@@ -311,6 +311,13 @@ TEST(StringViewFieldTest, RepeatedSetAndGetByReflection) {
               StrEq("222222222222"));
 }
 
+TEST(StringViewFieldTest, MergeAndClearEmptyImplicitPresence) {
+  TestStringView message, other;
+  other.set_implicit_presence("");
+  message.MergeFrom(other);
+  message.Clear();
+}
+
 }  // namespace
 }  // namespace protobuf
 }  // namespace google
diff --git a/src/google/protobuf/unittest_string_view.proto b/src/google/protobuf/unittest_string_view.proto
@@ -8,10 +8,11 @@ option java_multiple_files = true;
 option optimize_for = SPEED;
 option features.(pb.cpp).string_type = VIEW;
 
-// NEXT_TAG = 5;
+// NEXT_TAG = 6;
 message TestStringView {
   string singular_string = 1;
   bytes singular_bytes = 2;
+  string implicit_presence = 5 [features.field_presence = IMPLICIT];
 
   repeated string repeated_string = 3;
   repeated bytes repeated_bytes = 4;

Original file line number	Diff line number	Diff line change
`@@ -358,6 +358,11 @@ inline bool IsString(const FieldDescriptor* field) {`
`358`	`358`	`}`
`359`	`359`
`360`	`360`
	`361`	`+inline bool IsArenaStringPtr(const FieldDescriptor* field) {`
	`362`	`+ return field->cpp_string_type() == FieldDescriptor::CppStringType::kString \|\|`
	`363`	`+ field->cpp_string_type() == FieldDescriptor::CppStringType::kView;`
	`364`	`+}`
	`365`	`+`
`361`	`366`	`bool IsProfileDriven(const Options& options);`
`362`	`367`
`363`	`368`	// Returns true if `field` is unlikely to be present based on PDProto profile.