Skip to content

Commit d0e49df

Browse files
mkruskal-googlecopybara-github
authored andcommitted
Introduce FieldDescriptor::cpp_string_type() API to replace direct ctype inspection which will be removed in the next breaking change
This should provide the roughly same result as ctype, except that it reflects actual behavior rather than the specification in the proto file. VIEW will now be visible, and some subtleties around CORD and PIECE will change. PiperOrigin-RevId: 653677655
1 parent b4a7757 commit d0e49df

File tree

4 files changed

+115
-8
lines changed

4 files changed

+115
-8
lines changed

src/google/protobuf/descriptor.cc

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3939,6 +3939,29 @@ bool FieldDescriptor::has_optional_keyword() const {
39393939
is_optional() && !containing_oneof());
39403940
}
39413941

3942+
FieldDescriptor::CppStringType FieldDescriptor::cpp_string_type() const {
3943+
ABSL_DCHECK(cpp_type() == FieldDescriptor::CPPTYPE_STRING);
3944+
switch (features().GetExtension(pb::cpp).string_type()) {
3945+
case pb::CppFeatures::VIEW:
3946+
return CppStringType::kView;
3947+
case pb::CppFeatures::CORD:
3948+
// In open-source, protobuf CORD is only supported for singular bytes
3949+
// fields.
3950+
if (type() != FieldDescriptor::TYPE_BYTES || is_repeated() ||
3951+
is_extension()) {
3952+
return CppStringType::kString;
3953+
}
3954+
return CppStringType::kCord;
3955+
case pb::CppFeatures::STRING:
3956+
return CppStringType::kString;
3957+
default:
3958+
// If features haven't been resolved, this is a dynamic build not for C++
3959+
// codegen. Just use string type.
3960+
ABSL_DCHECK(!features().GetExtension(pb::cpp).has_string_type());
3961+
return CppStringType::kString;
3962+
}
3963+
}
3964+
39423965
// Location methods ===============================================
39433966

39443967
bool FileDescriptor::GetSourceLocation(const std::vector<int>& path,

src/google/protobuf/descriptor.h

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -875,6 +875,10 @@ class PROTOBUF_EXPORT FieldDescriptor : private internal::SymbolBase,
875875
const char* cpp_type_name() const; // Name of the C++ type.
876876
Label label() const; // optional/required/repeated
877877

878+
#ifndef SWIG
879+
CppStringType cpp_string_type() const; // The C++ string type of this field.
880+
#endif
881+
878882
bool is_required() const; // shorthand for label() == LABEL_REQUIRED
879883
bool is_optional() const; // shorthand for label() == LABEL_OPTIONAL
880884
bool is_repeated() const; // shorthand for label() == LABEL_REPEATED
@@ -2932,22 +2936,21 @@ PROTOBUF_EXPORT bool HasPreservingUnknownEnumSemantics(
29322936

29332937
PROTOBUF_EXPORT bool HasHasbit(const FieldDescriptor* field);
29342938

2939+
#ifndef SWIG
29352940
// For a string field, returns the effective ctype. If the actual ctype is
29362941
// not supported, returns the default of STRING.
29372942
template <typename FieldDesc = FieldDescriptor,
29382943
typename FieldOpts = FieldOptions>
29392944
typename FieldOpts::CType EffectiveStringCType(const FieldDesc* field) {
2940-
ABSL_DCHECK(field->cpp_type() == FieldDescriptor::CPPTYPE_STRING);
2941-
// Open-source protobuf release only supports STRING ctype and CORD for
2942-
// sinuglar bytes.
2943-
if (field->type() == FieldDescriptor::TYPE_BYTES && !field->is_repeated() &&
2944-
field->options().ctype() == FieldOpts::CORD && !field->is_extension()) {
2945-
return FieldOpts::CORD;
2945+
// TODO Replace this function with FieldDescriptor::string_type;
2946+
switch (field->cpp_string_type()) {
2947+
case FieldDescriptor::CppStringType::kCord:
2948+
return FieldOpts::CORD;
2949+
default:
2950+
return FieldOpts::STRING;
29462951
}
2947-
return FieldOpts::STRING;
29482952
}
29492953

2950-
#ifndef SWIG
29512954
enum class Utf8CheckMode : uint8_t {
29522955
kStrict = 0, // Parsing will fail if non UTF-8 data is in string fields.
29532956
kVerify = 1, // Only log an error but parsing will succeed.

src/google/protobuf/descriptor_lite.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,17 @@ class FieldDescriptorLite {
7878
MAX_LABEL = 3, // Constant useful for defining lookup tables
7979
// indexed by Label.
8080
};
81+
82+
// Identifies the storage type of a C++ string field. This corresponds to
83+
// pb.CppFeatures.StringType, but is compatible with ctype prior to Edition
84+
// 2024. 0 is reserved for errors.
85+
#ifndef SWIG
86+
enum class CppStringType {
87+
kView = 1,
88+
kCord = 2,
89+
kString = 3,
90+
};
91+
#endif
8192
};
8293

8394
} // namespace internal

src/google/protobuf/descriptor_unittest.cc

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7594,6 +7594,9 @@ TEST_F(FeaturesTest, Proto2Features) {
75947594
EXPECT_TRUE(message->FindFieldByName("req")->is_required());
75957595
EXPECT_TRUE(file->enum_type(0)->is_closed());
75967596

7597+
EXPECT_EQ(message->FindFieldByName("str")->cpp_string_type(),
7598+
FieldDescriptor::CppStringType::kString);
7599+
75977600
// Check round-trip consistency.
75987601
FileDescriptorProto proto;
75997602
file->CopyTo(&proto);
@@ -9710,6 +9713,73 @@ TEST_F(FeaturesTest, EnumFeatureHelpers) {
97109713
EXPECT_FALSE(HasPreservingUnknownEnumSemantics(field_legacy_closed));
97119714
}
97129715

9716+
TEST_F(FeaturesTest, FieldCppStringType) {
9717+
BuildDescriptorMessagesInTestPool();
9718+
const std::string file_contents = absl::Substitute(
9719+
R"pb(
9720+
name: "foo.proto"
9721+
syntax: "editions"
9722+
edition: EDITION_2024
9723+
message_type {
9724+
name: "Foo"
9725+
field {
9726+
name: "view"
9727+
number: 1
9728+
label: LABEL_OPTIONAL
9729+
type: TYPE_STRING
9730+
}
9731+
field {
9732+
name: "str"
9733+
number: 2
9734+
label: LABEL_OPTIONAL
9735+
type: TYPE_STRING
9736+
options {
9737+
features {
9738+
[pb.cpp] { string_type: STRING }
9739+
}
9740+
}
9741+
}
9742+
field {
9743+
name: "cord"
9744+
number: 3
9745+
label: LABEL_OPTIONAL
9746+
type: TYPE_STRING
9747+
options {
9748+
features {
9749+
[pb.cpp] { string_type: CORD }
9750+
}
9751+
}
9752+
}
9753+
field {
9754+
name: "cord_bytes"
9755+
number: 4
9756+
label: LABEL_OPTIONAL
9757+
type: TYPE_BYTES
9758+
options {
9759+
features {
9760+
[pb.cpp] { string_type: CORD }
9761+
}
9762+
}
9763+
} $0
9764+
}
9765+
)pb",
9766+
""
9767+
);
9768+
const FileDescriptor* file = BuildFile(file_contents);
9769+
const Descriptor* message = file->message_type(0);
9770+
const FieldDescriptor* view = message->field(0);
9771+
const FieldDescriptor* str = message->field(1);
9772+
const FieldDescriptor* cord = message->field(2);
9773+
const FieldDescriptor* cord_bytes = message->field(3);
9774+
9775+
EXPECT_EQ(view->cpp_string_type(), FieldDescriptor::CppStringType::kView);
9776+
EXPECT_EQ(str->cpp_string_type(), FieldDescriptor::CppStringType::kString);
9777+
EXPECT_EQ(cord_bytes->cpp_string_type(),
9778+
FieldDescriptor::CppStringType::kCord);
9779+
EXPECT_EQ(cord->cpp_string_type(), FieldDescriptor::CppStringType::kString);
9780+
9781+
}
9782+
97139783
TEST_F(FeaturesTest, MergeFeatureValidationFailed) {
97149784
BuildDescriptorMessagesInTestPool();
97159785
BuildFileInTestPool(pb::TestFeatures::descriptor()->file());

0 commit comments

Comments
 (0)