Skip to content

Commit

Permalink
Introduce FieldDescriptor::cpp_string_type() API to replace direct ct…
Browse files Browse the repository at this point in the history
…ype 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
  • Loading branch information
mkruskal-google authored and zhangskz committed Aug 12, 2024
1 parent 49dc63b commit 72b0b7a
Show file tree
Hide file tree
Showing 4 changed files with 115 additions and 8 deletions.
23 changes: 23 additions & 0 deletions src/google/protobuf/descriptor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3941,6 +3941,29 @@ bool FieldDescriptor::has_optional_keyword() const {
is_optional() && !containing_oneof());
}

FieldDescriptor::CppStringType FieldDescriptor::cpp_string_type() const {
ABSL_DCHECK(cpp_type() == FieldDescriptor::CPPTYPE_STRING);
switch (features().GetExtension(pb::cpp).string_type()) {
case pb::CppFeatures::VIEW:
return CppStringType::kView;
case pb::CppFeatures::CORD:
// In open-source, protobuf CORD is only supported for singular bytes
// fields.
if (type() != FieldDescriptor::TYPE_BYTES || is_repeated() ||
is_extension()) {
return CppStringType::kString;
}
return CppStringType::kCord;
case pb::CppFeatures::STRING:
return CppStringType::kString;
default:
// If features haven't been resolved, this is a dynamic build not for C++
// codegen. Just use string type.
ABSL_DCHECK(!features().GetExtension(pb::cpp).has_string_type());
return CppStringType::kString;
}
}

// Location methods ===============================================

bool FileDescriptor::GetSourceLocation(const std::vector<int>& path,
Expand Down
19 changes: 11 additions & 8 deletions src/google/protobuf/descriptor.h
Original file line number Diff line number Diff line change
Expand Up @@ -859,6 +859,10 @@ class PROTOBUF_EXPORT FieldDescriptor : private internal::SymbolBase,
const char* cpp_type_name() const; // Name of the C++ type.
Label label() const; // optional/required/repeated

#ifndef SWIG
CppStringType cpp_string_type() const; // The C++ string type of this field.
#endif

bool is_required() const; // shorthand for label() == LABEL_REQUIRED
bool is_optional() const; // shorthand for label() == LABEL_OPTIONAL
bool is_repeated() const; // shorthand for label() == LABEL_REPEATED
Expand Down Expand Up @@ -2892,22 +2896,21 @@ PROTOBUF_EXPORT bool HasPreservingUnknownEnumSemantics(

PROTOBUF_EXPORT bool HasHasbit(const FieldDescriptor* field);

#ifndef SWIG
// For a string field, returns the effective ctype. If the actual ctype is
// not supported, returns the default of STRING.
template <typename FieldDesc = FieldDescriptor,
typename FieldOpts = FieldOptions>
typename FieldOpts::CType EffectiveStringCType(const FieldDesc* field) {
ABSL_DCHECK(field->cpp_type() == FieldDescriptor::CPPTYPE_STRING);
// Open-source protobuf release only supports STRING ctype and CORD for
// sinuglar bytes.
if (field->type() == FieldDescriptor::TYPE_BYTES && !field->is_repeated() &&
field->options().ctype() == FieldOpts::CORD && !field->is_extension()) {
return FieldOpts::CORD;
// TODO Replace this function with FieldDescriptor::string_type;
switch (field->cpp_string_type()) {
case FieldDescriptor::CppStringType::kCord:
return FieldOpts::CORD;
default:
return FieldOpts::STRING;
}
return FieldOpts::STRING;
}

#ifndef SWIG
enum class Utf8CheckMode : uint8_t {
kStrict = 0, // Parsing will fail if non UTF-8 data is in string fields.
kVerify = 1, // Only log an error but parsing will succeed.
Expand Down
11 changes: 11 additions & 0 deletions src/google/protobuf/descriptor_lite.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,17 @@ class FieldDescriptorLite {
MAX_LABEL = 3, // Constant useful for defining lookup tables
// indexed by Label.
};

// Identifies the storage type of a C++ string field. This corresponds to
// pb.CppFeatures.StringType, but is compatible with ctype prior to Edition
// 2024. 0 is reserved for errors.
#ifndef SWIG
enum class CppStringType {
kView = 1,
kCord = 2,
kString = 3,
};
#endif
};

} // namespace internal
Expand Down
70 changes: 70 additions & 0 deletions src/google/protobuf/descriptor_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7593,6 +7593,9 @@ TEST_F(FeaturesTest, Proto2Features) {
EXPECT_TRUE(message->FindFieldByName("req")->is_required());
EXPECT_TRUE(file->enum_type(0)->is_closed());

EXPECT_EQ(message->FindFieldByName("str")->cpp_string_type(),
FieldDescriptor::CppStringType::kString);

// Check round-trip consistency.
FileDescriptorProto proto;
file->CopyTo(&proto);
Expand Down Expand Up @@ -9709,6 +9712,73 @@ TEST_F(FeaturesTest, EnumFeatureHelpers) {
EXPECT_FALSE(HasPreservingUnknownEnumSemantics(field_legacy_closed));
}

TEST_F(FeaturesTest, FieldCppStringType) {
BuildDescriptorMessagesInTestPool();
const std::string file_contents = absl::Substitute(
R"pb(
name: "foo.proto"
syntax: "editions"
edition: EDITION_2024
message_type {
name: "Foo"
field {
name: "view"
number: 1
label: LABEL_OPTIONAL
type: TYPE_STRING
}
field {
name: "str"
number: 2
label: LABEL_OPTIONAL
type: TYPE_STRING
options {
features {
[pb.cpp] { string_type: STRING }
}
}
}
field {
name: "cord"
number: 3
label: LABEL_OPTIONAL
type: TYPE_STRING
options {
features {
[pb.cpp] { string_type: CORD }
}
}
}
field {
name: "cord_bytes"
number: 4
label: LABEL_OPTIONAL
type: TYPE_BYTES
options {
features {
[pb.cpp] { string_type: CORD }
}
}
} $0
}
)pb",
""
);
const FileDescriptor* file = BuildFile(file_contents);
const Descriptor* message = file->message_type(0);
const FieldDescriptor* view = message->field(0);
const FieldDescriptor* str = message->field(1);
const FieldDescriptor* cord = message->field(2);
const FieldDescriptor* cord_bytes = message->field(3);

EXPECT_EQ(view->cpp_string_type(), FieldDescriptor::CppStringType::kView);
EXPECT_EQ(str->cpp_string_type(), FieldDescriptor::CppStringType::kString);
EXPECT_EQ(cord_bytes->cpp_string_type(),
FieldDescriptor::CppStringType::kCord);
EXPECT_EQ(cord->cpp_string_type(), FieldDescriptor::CppStringType::kString);

}

TEST_F(FeaturesTest, MergeFeatureValidationFailed) {
BuildDescriptorMessagesInTestPool();
BuildFileInTestPool(pb::TestFeatures::descriptor()->file());
Expand Down

0 comments on commit 72b0b7a

Please sign in to comment.