Skip to content

Commit

Permalink
Prepare the code for migrating return types from const std::string& to
Browse files Browse the repository at this point in the history
`absl::string_view`.

Added temporary macro `PROTOBUF_TEMPORARY_ENABLE_STRING_VIEW_RETURN_TYPE` to turn on the new return type before the breaking change happens. It allows users to test and update their code ahead of time.

PiperOrigin-RevId: 652517294
  • Loading branch information
protobuf-github-bot authored and copybara-github committed Jul 15, 2024
1 parent 8e60d9f commit e13b8e9
Show file tree
Hide file tree
Showing 22 changed files with 268 additions and 200 deletions.
2 changes: 1 addition & 1 deletion conformance/binary_json_conformance_suite.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1810,7 +1810,7 @@ void BinaryAndJsonConformanceSuiteImpl<MessageType>::
const std::string type_name =
UpperCase(absl::StrCat(".", FieldDescriptor::TypeName(type)));
const FieldDescriptor* field = GetFieldForType(type, true, Packed::kFalse);
std::string field_name = field->name();
const absl::string_view field_name = field->name();

std::string message_field =
absl::StrCat("\"", field_name, "\": [", field_value, "]");
Expand Down
1 change: 1 addition & 0 deletions src/google/protobuf/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -1144,6 +1144,7 @@ cc_library(
"//src/google/protobuf/testing:file",
"@com_google_absl//absl/container:flat_hash_map",
"@com_google_absl//absl/log:absl_check",
"@com_google_absl//absl/strings",
"@com_google_absl//absl/time",
"@com_google_googletest//:gtest",
],
Expand Down
192 changes: 97 additions & 95 deletions src/google/protobuf/descriptor.cc

Large diffs are not rendered by default.

110 changes: 75 additions & 35 deletions src/google/protobuf/descriptor.h
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,13 @@ namespace internal {
#define PROTOBUF_INTERNAL_CHECK_CLASS_SIZE(t, expected)
#endif

// `typedef` instead of `using` for SWIG
#if defined(PROTOBUF_FUTURE_STRING_VIEW_RETURN_TYPE)
typedef absl::string_view DescriptorStringView;
#else
typedef const std::string& DescriptorStringView;
#endif

class FlatAllocator;

class PROTOBUF_EXPORT LazyDescriptor {
Expand Down Expand Up @@ -286,6 +293,9 @@ PROTOBUF_EXPORT absl::string_view ShortEditionName(Edition edition);

bool IsEnumFullySequential(const EnumDescriptor* enum_desc);

const std::string& DefaultValueStringAsString(const FieldDescriptor* field);
const std::string& NameOfEnumAsString(const EnumValueDescriptor* descriptor);

} // namespace internal

// Provide an Abseil formatter for edition names.
Expand All @@ -308,14 +318,14 @@ class PROTOBUF_EXPORT Descriptor : private internal::SymbolBase {
#endif

// The name of the message type, not including its scope.
const std::string& name() const;
internal::DescriptorStringView name() const;

// The fully-qualified name of the message type, scope delimited by
// periods. For example, message type "Foo" which is declared in package
// "bar" has full name "bar.Foo". If a type "Baz" is nested within
// Foo, Baz's full_name is "bar.Foo.Baz". To get only the part that
// comes after the last '.', use name().
const std::string& full_name() const;
internal::DescriptorStringView full_name() const;

// Index of this descriptor within the file or containing type's message
// type array.
Expand Down Expand Up @@ -493,10 +503,12 @@ class PROTOBUF_EXPORT Descriptor : private internal::SymbolBase {
const ExtensionRangeOptions& options() const { return *options_; }

// Returns the name of the containing type.
const std::string& name() const { return containing_type_->name(); }
internal::DescriptorStringView name() const {
return containing_type_->name();
}

// Returns the full name of the containing type.
const std::string& full_name() const {
internal::DescriptorStringView full_name() const {
return containing_type_->full_name();
}

Expand Down Expand Up @@ -615,7 +627,7 @@ class PROTOBUF_EXPORT Descriptor : private internal::SymbolBase {
int reserved_name_count() const;

// Gets a reserved name by index, where 0 <= index < reserved_name_count().
const std::string& reserved_name(int index) const;
internal::DescriptorStringView reserved_name(int index) const;

// Returns true if the field name is reserved.
bool IsReservedName(absl::string_view name) const;
Expand Down Expand Up @@ -827,9 +839,13 @@ class PROTOBUF_EXPORT FieldDescriptor : private internal::SymbolBase,
// Users may not declare fields that use reserved numbers.
static const int kLastReservedNumber = 19999;

const std::string& name() const; // Name of this field within the message.
const std::string& full_name() const; // Fully-qualified name of the field.
const std::string& json_name() const; // JSON name of this field.
// Name of this field within the message.
internal::DescriptorStringView name() const;
// Fully-qualified name of the field.
internal::DescriptorStringView full_name() const;
// JSON name of this field.
internal::DescriptorStringView json_name() const;

const FileDescriptor* file() const; // File in which this field was defined.
bool is_extension() const; // Is this an extension field?
int number() const; // Declared tag number.
Expand All @@ -840,7 +856,7 @@ class PROTOBUF_EXPORT FieldDescriptor : private internal::SymbolBase,
// field names should be lowercased anyway according to the protobuf style
// guide, so this only makes a difference when dealing with old .proto files
// which do not follow the guide.)
const std::string& lowercase_name() const;
internal::DescriptorStringView lowercase_name() const;

// Same as name() except converted to camel-case. In this conversion, any
// time an underscore appears in the name, it is removed and the next
Expand All @@ -851,7 +867,7 @@ class PROTOBUF_EXPORT FieldDescriptor : private internal::SymbolBase,
// fooBar -> fooBar
// This (and especially the FindFieldByCamelcaseName() method) can be useful
// when parsing formats which prefer to use camel-case naming style.
const std::string& camelcase_name() const;
internal::DescriptorStringView camelcase_name() const;

Type type() const; // Declared type of this field.
const char* type_name() const; // Name of the declared type.
Expand Down Expand Up @@ -947,7 +963,7 @@ class PROTOBUF_EXPORT FieldDescriptor : private internal::SymbolBase,
const EnumValueDescriptor* default_value_enum() const;
// Get the field default value if cpp_type() == CPPTYPE_STRING. If no
// explicit default was defined, the default is the empty string.
const std::string& default_value_string() const;
internal::DescriptorStringView default_value_string() const;

// The Descriptor for the message of which this is a field. For extensions,
// this is the extended type. Never nullptr.
Expand Down Expand Up @@ -1025,7 +1041,7 @@ class PROTOBUF_EXPORT FieldDescriptor : private internal::SymbolBase,
// its printable name) can be accomplished with
// message->file()->pool()->FindExtensionByPrintableName(message, name)
// where the extension extends "message".
const std::string& PrintableNameForExtension() const;
internal::DescriptorStringView PrintableNameForExtension() const;

// Source Location ---------------------------------------------------

Expand All @@ -1043,6 +1059,8 @@ class PROTOBUF_EXPORT FieldDescriptor : private internal::SymbolBase,
friend class compiler::cpp::Formatter;
friend class Reflection;
friend class FieldDescriptorLegacy;
friend const std::string& internal::DefaultValueStringAsString(
const FieldDescriptor* field);

// Returns true if this field was syntactically written with "optional" in the
// .proto file. Excludes singular proto3 fields that do not have a label.
Expand Down Expand Up @@ -1176,8 +1194,10 @@ class PROTOBUF_EXPORT OneofDescriptor : private internal::SymbolBase {
OneofDescriptor& operator=(const OneofDescriptor&) = delete;
#endif

const std::string& name() const; // Name of this oneof.
const std::string& full_name() const; // Fully-qualified name of the oneof.
// Name of this oneof.
internal::DescriptorStringView name() const;
// Fully-qualified name of the oneof.
internal::DescriptorStringView full_name() const;

// Index of this oneof within the message's oneof array.
int index() const;
Expand Down Expand Up @@ -1282,10 +1302,10 @@ class PROTOBUF_EXPORT EnumDescriptor : private internal::SymbolBase {
#endif

// The name of this enum type in the containing scope.
const std::string& name() const;
internal::DescriptorStringView name() const;

// The fully-qualified name of the enum type, scope delimited by periods.
const std::string& full_name() const;
internal::DescriptorStringView full_name() const;

// Index of this enum within the file or containing message's enum array.
int index() const;
Expand Down Expand Up @@ -1381,7 +1401,7 @@ class PROTOBUF_EXPORT EnumDescriptor : private internal::SymbolBase {
int reserved_name_count() const;

// Gets a reserved name by index, where 0 <= index < reserved_name_count().
const std::string& reserved_name(int index) const;
internal::DescriptorStringView reserved_name(int index) const;

// Returns true if the field name is reserved.
bool IsReservedName(absl::string_view name) const;
Expand Down Expand Up @@ -1494,7 +1514,7 @@ class PROTOBUF_EXPORT EnumValueDescriptor : private internal::SymbolBaseN<0>,
EnumValueDescriptor& operator=(const EnumValueDescriptor&) = delete;
#endif

const std::string& name() const; // Name of this enum constant.
internal::DescriptorStringView name() const; // Name of this enum constant.
int index() const; // Index within the enums's Descriptor.
int number() const; // Numeric value of this enum constant.

Expand All @@ -1503,7 +1523,7 @@ class PROTOBUF_EXPORT EnumValueDescriptor : private internal::SymbolBaseN<0>,
// "google.protobuf.FieldDescriptorProto.TYPE_INT32", NOT
// "google.protobuf.FieldDescriptorProto.Type.TYPE_INT32". This is to conform
// with C++ scoping rules for enums.
const std::string& full_name() const;
internal::DescriptorStringView full_name() const;

// The .proto file in which this value was defined. Never nullptr.
const FileDescriptor* file() const;
Expand Down Expand Up @@ -1545,6 +1565,8 @@ class PROTOBUF_EXPORT EnumValueDescriptor : private internal::SymbolBaseN<0>,
// Allows access to GetLocationPath for annotations.
friend class io::Printer;
friend class compiler::cpp::Formatter;
friend const std::string& internal::NameOfEnumAsString(
const EnumValueDescriptor* descriptor);

// Get the merged features that apply to this enum value. These are specified
// in the .proto file through the feature options in the message definition.
Expand Down Expand Up @@ -1595,9 +1617,9 @@ class PROTOBUF_EXPORT ServiceDescriptor : private internal::SymbolBase {
#endif

// The name of the service, not including its containing scope.
const std::string& name() const;
internal::DescriptorStringView name() const;
// The fully-qualified name of the service, scope delimited by periods.
const std::string& full_name() const;
internal::DescriptorStringView full_name() const;
// Index of this service within the file's services array.
int index() const;

Expand Down Expand Up @@ -1699,9 +1721,9 @@ class PROTOBUF_EXPORT MethodDescriptor : private internal::SymbolBase {
#endif

// Name of this method, not including containing scope.
const std::string& name() const;
internal::DescriptorStringView name() const;
// The fully-qualified name of the method, scope delimited by periods.
const std::string& full_name() const;
internal::DescriptorStringView full_name() const;
// Index within the service's Descriptor.
int index() const;

Expand Down Expand Up @@ -1807,10 +1829,10 @@ class PROTOBUF_EXPORT FileDescriptor : private internal::SymbolBase {

// The filename, relative to the source tree.
// e.g. "foo/bar/baz.proto"
const std::string& name() const;
internal::DescriptorStringView name() const;

// The package, e.g. "google.protobuf.compiler".
const std::string& package() const;
internal::DescriptorStringView package() const;

// The DescriptorPool in which this FileDescriptor and all its contents were
// allocated. Never nullptr.
Expand Down Expand Up @@ -2472,13 +2494,19 @@ class PROTOBUF_EXPORT DescriptorPool {
inline TYPE CLASS::FIELD() const { return FIELD##_; }

// Strings fields are stored as pointers but returned as const references.
#define PROTOBUF_DEFINE_STRING_ACCESSOR(CLASS, FIELD) \
inline const std::string& CLASS::FIELD() const { return *FIELD##_; }
#define PROTOBUF_DEFINE_STRING_ACCESSOR(CLASS, FIELD) \
inline internal::DescriptorStringView CLASS::FIELD() const { \
return *FIELD##_; \
}

// Name and full name are stored in a single array to save space.
#define PROTOBUF_DEFINE_NAME_ACCESSOR(CLASS) \
inline const std::string& CLASS::name() const { return all_names_[0]; } \
inline const std::string& CLASS::full_name() const { return all_names_[1]; }
#define PROTOBUF_DEFINE_NAME_ACCESSOR(CLASS) \
inline internal::DescriptorStringView CLASS::name() const { \
return all_names_[0]; \
} \
inline internal::DescriptorStringView CLASS::full_name() const { \
return all_names_[1]; \
}

// Arrays take an index parameter, obviously.
#define PROTOBUF_DEFINE_ARRAY_ACCESSOR(CLASS, FIELD, TYPE) \
Expand Down Expand Up @@ -2627,7 +2655,8 @@ inline bool Descriptor::IsReservedName(absl::string_view name) const {

// Can't use PROTOBUF_DEFINE_ARRAY_ACCESSOR because reserved_names_ is actually
// an array of pointers rather than the usual array of objects.
inline const std::string& Descriptor::reserved_name(int index) const {
inline internal::DescriptorStringView Descriptor::reserved_name(
int index) const {
return *reserved_names_[index];
}

Expand All @@ -2646,19 +2675,20 @@ inline bool EnumDescriptor::IsReservedName(absl::string_view name) const {

// Can't use PROTOBUF_DEFINE_ARRAY_ACCESSOR because reserved_names_ is actually
// an array of pointers rather than the usual array of objects.
inline const std::string& EnumDescriptor::reserved_name(int index) const {
inline internal::DescriptorStringView EnumDescriptor::reserved_name(
int index) const {
return *reserved_names_[index];
}

inline const std::string& FieldDescriptor::lowercase_name() const {
inline internal::DescriptorStringView FieldDescriptor::lowercase_name() const {
return all_names_[lowercase_name_index_];
}

inline const std::string& FieldDescriptor::camelcase_name() const {
inline internal::DescriptorStringView FieldDescriptor::camelcase_name() const {
return all_names_[camelcase_name_index_];
}

inline const std::string& FieldDescriptor::json_name() const {
inline internal::DescriptorStringView FieldDescriptor::json_name() const {
return all_names_[json_name_index_];
}

Expand Down Expand Up @@ -2822,6 +2852,16 @@ inline const FileDescriptor* FileDescriptor::weak_dependency(int index) const {

namespace internal {

inline const std::string& DefaultValueStringAsString(
const FieldDescriptor* field) {
return *field->default_value_string_;
}

inline const std::string& NameOfEnumAsString(
const EnumValueDescriptor* descriptor) {
return descriptor->all_names_[0];
}

inline bool IsEnumFullySequential(const EnumDescriptor* enum_desc) {
return enum_desc->sequential_value_limit_ == enum_desc->value_count() - 1;
}
Expand Down
27 changes: 14 additions & 13 deletions src/google/protobuf/descriptor_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -570,15 +570,15 @@ TEST_F(FileDescriptorTest, CopyHeadingTo) {
}

void ExtractDebugString(
const FileDescriptor* file, absl::flat_hash_set<std::string>* visited,
std::vector<std::pair<std::string, std::string>>* debug_strings) {
const FileDescriptor* file, absl::flat_hash_set<absl::string_view>* visited,
std::vector<std::pair<absl::string_view, std::string>>* debug_strings) {
if (!visited->insert(file->name()).second) {
return;
}
for (int i = 0; i < file->dependency_count(); ++i) {
ExtractDebugString(file->dependency(i), visited, debug_strings);
}
debug_strings->push_back(std::make_pair(file->name(), file->DebugString()));
debug_strings->push_back({file->name(), file->DebugString()});
}

class SimpleErrorCollector : public io::ErrorCollector {
Expand All @@ -596,8 +596,8 @@ class SimpleErrorCollector : public io::ErrorCollector {
// Test that the result of FileDescriptor::DebugString() can be used to create
// the original descriptors.
TEST_F(FileDescriptorTest, DebugStringRoundTrip) {
absl::flat_hash_set<std::string> visited;
std::vector<std::pair<std::string, std::string>> debug_strings;
absl::flat_hash_set<absl::string_view> visited;
std::vector<std::pair<absl::string_view, std::string>> debug_strings;
ExtractDebugString(protobuf_unittest::TestAllTypes::descriptor()->file(),
&visited, &debug_strings);
ExtractDebugString(
Expand All @@ -609,7 +609,7 @@ TEST_F(FileDescriptorTest, DebugStringRoundTrip) {

DescriptorPool pool;
for (size_t i = 0; i < debug_strings.size(); ++i) {
const std::string& name = debug_strings[i].first;
const absl::string_view name = debug_strings[i].first;
const std::string& content = debug_strings[i].second;
io::ArrayInputStream input_stream(content.data(), content.size());
SimpleErrorCollector error_collector;
Expand Down Expand Up @@ -850,16 +850,17 @@ TEST_F(DescriptorTest, ContainingType) {

TEST_F(DescriptorTest, FieldNamesDedup) {
const auto collect_unique_names = [](const FieldDescriptor* field) {
absl::btree_set<std::string> names{field->name(), field->lowercase_name(),
field->camelcase_name(),
field->json_name()};
absl::btree_set<absl::string_view> names{
field->name(), field->lowercase_name(), field->camelcase_name(),
field->json_name()};
// Verify that we have the same number of string objects as we have string
// values. That is, duplicate names use the same std::string object.
// This is for memory efficiency.
EXPECT_EQ(names.size(), (absl::flat_hash_set<const std::string*>{
&field->name(), &field->lowercase_name(),
&field->camelcase_name(), &field->json_name()}
.size()))
EXPECT_EQ(names.size(),
(absl::flat_hash_set<const void*>{
field->name().data(), field->lowercase_name().data(),
field->camelcase_name().data(), field->json_name().data()}
.size()))
<< testing::PrintToString(names);
return names;
};
Expand Down
Loading

0 comments on commit e13b8e9

Please sign in to comment.