Infer string type feature from ctype pre-editions.

This will allow internal code to simply check the feature value instead of checking both ctype and string_type. PiperOrigin-RevId: 625897380
protocolbuffers · Apr 18, 2024 · 1d6fdc1 · 1d6fdc1
1 parent 89118ba
commit 1d6fdc1
Show file tree

Hide file tree

Showing 2 changed files with 82 additions and 1 deletion.
diff --git a/src/google/protobuf/descriptor.cc b/src/google/protobuf/descriptor.cc
@@ -5446,6 +5446,16 @@ static void InferLegacyProtoFeatures(const ProtoT& proto,
 static void InferLegacyProtoFeatures(const FieldDescriptorProto& proto,
                                      const FieldOptions& options,
                                      Edition edition, FeatureSet& features) {
+  if (!features.MutableExtension(pb::cpp)->has_string_type()) {
+    if (options.ctype() == FieldOptions::CORD) {
+      features.MutableExtension(pb::cpp)->set_string_type(
+          pb::CppFeatures::CORD);
+    }
+  }
+
+  // Everything below is specifically for proto2/proto.
+  if (!IsLegacyEdition(edition)) return;
+
   if (proto.label() == FieldDescriptorProto::LABEL_REQUIRED) {
     features.set_field_presence(FeatureSet::LEGACY_REQUIRED);
   }
@@ -5491,8 +5501,8 @@ void DescriptorBuilder::ResolveFeaturesImpl(
       AddError(descriptor->name(), proto, error_location,
                "Features are only valid under editions.");
     }
-    InferLegacyProtoFeatures(proto, *options, edition, base_features);
   }
+  InferLegacyProtoFeatures(proto, *options, edition, base_features);
 
   if (base_features.ByteSizeLong() == 0 && !force_merge) {
     // Nothing to merge, and we aren't forcing it.

diff --git a/src/google/protobuf/descriptor_unittest.cc b/src/google/protobuf/descriptor_unittest.cc
@@ -7481,6 +7481,20 @@ TEST_F(FeaturesTest, Proto2Features) {
       }
       field { name: "utf8" number: 6 label: LABEL_REPEATED type: TYPE_STRING }
       field { name: "req" number: 7 label: LABEL_REQUIRED type: TYPE_INT32 }
+      field {
+        name: "cord"
+        number: 8
+        label: LABEL_OPTIONAL
+        type: TYPE_STRING
+        options { ctype: CORD }
+      }
+      field {
+        name: "piece"
+        number: 9
+        label: LABEL_OPTIONAL
+        type: TYPE_STRING
+        options { ctype: STRING_PIECE }
+      }
     }
     enum_type {
       name: "Foo2"
@@ -7531,6 +7545,10 @@ TEST_F(FeaturesTest, Proto2Features) {
       Utf8CheckMode::kVerify);
   EXPECT_EQ(GetUtf8CheckMode(message->FindFieldByName("str"), /*is_lite=*/true),
             Utf8CheckMode::kNone);
+  EXPECT_EQ(GetCoreFeatures(message->FindFieldByName("cord"))
+                .GetExtension(pb::cpp)
+                .string_type(),
+            pb::CppFeatures::CORD);
   EXPECT_FALSE(field->is_packed());
   EXPECT_FALSE(field->legacy_enum_field_treated_as_closed());
   EXPECT_FALSE(HasPreservingUnknownEnumSemantics(field));
@@ -7783,6 +7801,59 @@ TEST_F(FeaturesTest, Edition2023Defaults) {
             pb::VALUE3);
 }
 
+TEST_F(FeaturesTest, Edition2023InferredFeatures) {
+  FileDescriptorProto file_proto = ParseTextOrDie(R"pb(
+    name: "foo.proto"
+    syntax: "editions"
+    edition: EDITION_2023
+    message_type {
+      name: "Foo"
+      field { name: "str" number: 1 label: LABEL_OPTIONAL type: TYPE_STRING }
+      field {
+        name: "cord"
+        number: 2
+        label: LABEL_OPTIONAL
+        type: TYPE_STRING
+        options { ctype: CORD }
+      }
+      field {
+        name: "piece"
+        number: 3
+        label: LABEL_OPTIONAL
+        type: TYPE_STRING
+        options { ctype: STRING_PIECE }
+      }
+      field {
+        name: "ctype_and_string_type"
+        number: 4
+        label: LABEL_OPTIONAL
+        type: TYPE_STRING
+        options {
+          ctype: CORD
+          features {
+            [pb.cpp] { string_type: VIEW }
+          }
+        }
+      }
+    }
+  )pb");
+
+  BuildDescriptorMessagesInTestPool();
+  BuildFileInTestPool(pb::CppFeatures::GetDescriptor()->file());
+  const FileDescriptor* file = ABSL_DIE_IF_NULL(pool_.BuildFile(file_proto));
+  const Descriptor* message = file->message_type(0);
+
+  EXPECT_EQ(
+      GetCoreFeatures(message->field(0)).GetExtension(pb::cpp).string_type(),
+      pb::CppFeatures::STRING);
+  EXPECT_EQ(
+      GetCoreFeatures(message->field(1)).GetExtension(pb::cpp).string_type(),
+      pb::CppFeatures::CORD);
+  EXPECT_EQ(
+      GetCoreFeatures(message->field(3)).GetExtension(pb::cpp).string_type(),
+      pb::CppFeatures::VIEW);
+}
+
 TEST_F(FeaturesTest, Edition2024Defaults) {
   FileDescriptorProto file_proto = ParseTextOrDie(R"pb(
     name: "foo.proto"