From e13b8e999b3922d0633802c7f90e39af50a31d76 Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Mon, 15 Jul 2024 10:05:03 -0700 Subject: [PATCH] Prepare the code for migrating return types from `const std::string&` to `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 --- conformance/binary_json_conformance_suite.cc | 2 +- src/google/protobuf/BUILD.bazel | 1 + src/google/protobuf/descriptor.cc | 192 +++++++++--------- src/google/protobuf/descriptor.h | 110 ++++++---- src/google/protobuf/descriptor_unittest.cc | 27 +-- .../protobuf/descriptor_visitor_test.cc | 16 +- .../protobuf/generated_message_reflection.cc | 28 +-- src/google/protobuf/io/printer.h | 16 +- .../json/internal/descriptor_traits.h | 2 +- src/google/protobuf/port_def.inc | 7 + src/google/protobuf/port_undef.inc | 1 + src/google/protobuf/reflection_ops.cc | 7 +- src/google/protobuf/reflection_tester.cc | 2 +- src/google/protobuf/test_util.h | 5 +- src/google/protobuf/text_format.cc | 5 +- src/google/protobuf/unknown_field_set.h | 12 +- .../protobuf/util/message_differencer.cc | 6 +- .../protobuf/util/type_resolver_util.cc | 4 +- src/google/protobuf/wire_format.cc | 12 +- src/google/protobuf/wire_format.h | 9 +- src/google/protobuf/wire_format_lite.cc | 2 +- src/google/protobuf/wire_format_lite.h | 2 +- 22 files changed, 268 insertions(+), 200 deletions(-) diff --git a/conformance/binary_json_conformance_suite.cc b/conformance/binary_json_conformance_suite.cc index 1d2c298006087..f631b9adade8d 100644 --- a/conformance/binary_json_conformance_suite.cc +++ b/conformance/binary_json_conformance_suite.cc @@ -1810,7 +1810,7 @@ void BinaryAndJsonConformanceSuiteImpl:: 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, "]"); diff --git a/src/google/protobuf/BUILD.bazel b/src/google/protobuf/BUILD.bazel index ce1d5e8dbee36..38c088e09273a 100644 --- a/src/google/protobuf/BUILD.bazel +++ b/src/google/protobuf/BUILD.bazel @@ -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", ], diff --git a/src/google/protobuf/descriptor.cc b/src/google/protobuf/descriptor.cc index fda5dc4d7a452..ed5f6af920f1f 100644 --- a/src/google/protobuf/descriptor.cc +++ b/src/google/protobuf/descriptor.cc @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -89,12 +90,11 @@ namespace google { namespace protobuf { namespace { -using ::google::protobuf::internal::DownCast; const int kPackageLimit = 100; -std::string ToCamelCase(const std::string& input, bool lower_first) { +std::string ToCamelCase(const absl::string_view input, bool lower_first) { bool capitalize_next = !lower_first; std::string result; result.reserve(input.size()); @@ -118,7 +118,7 @@ std::string ToCamelCase(const std::string& input, bool lower_first) { return result; } -std::string ToJsonName(const std::string& input) { +std::string ToJsonName(const absl::string_view input) { bool capitalize_next = false; std::string result; result.reserve(input.size()); @@ -465,13 +465,13 @@ class FlatAllocatorImpl { int camelcase_index; int json_index; }; - FieldNamesResult AllocateFieldNames(const std::string& name, - const std::string& scope, + FieldNamesResult AllocateFieldNames(const absl::string_view name, + const absl::string_view scope, const std::string* opt_json_name) { ABSL_CHECK(has_allocated()); std::string full_name = - scope.empty() ? name : absl::StrCat(scope, ".", name); + scope.empty() ? std::string(name) : absl::StrCat(scope, ".", name); // Fast path for snake_case names, which follow the style guide. if (opt_json_name == nullptr) { @@ -490,7 +490,7 @@ class FlatAllocatorImpl { } std::vector names; - names.push_back(name); + names.emplace_back(name); names.push_back(std::move(full_name)); const auto push_name = [&](std::string new_name) { @@ -507,7 +507,7 @@ class FlatAllocatorImpl { FieldNamesResult result{nullptr, 0, 0, 0}; - std::string lowercase_name = name; + std::string lowercase_name = std::string(name); absl::AsciiStrToLower(&lowercase_name); result.lowercase_index = push_name(std::move(lowercase_name)); result.camelcase_index = @@ -547,8 +547,8 @@ class FlatAllocatorImpl { static bool IsLowerOrDigit(char c) { return IsLower(c) || IsDigit(c); } enum class FieldNameCase { kAllLower, kSnakeCase, kOther }; - FieldNameCase GetFieldNameCase(const std::string& name) { - if (!IsLower(name[0])) return FieldNameCase::kOther; + FieldNameCase GetFieldNameCase(const absl::string_view name) { + if (!name.empty() && !IsLower(name[0])) return FieldNameCase::kOther; FieldNameCase best = FieldNameCase::kAllLower; for (char c : name) { if (IsLowerOrDigit(c)) { @@ -1082,7 +1082,7 @@ absl::flat_hash_set* NewAllowedProto3Extendee() { // Only extensions to descriptor options are allowed. We use name comparison // instead of comparing the descriptor directly because the extensions may be // defined in a different pool. -bool AllowedExtendeeInProto3(const std::string& name) { +bool AllowedExtendeeInProto3(const absl::string_view name) { static auto allowed_proto3_extendees = internal::OnShutdownDelete(NewAllowedProto3Extendee()); return allowed_proto3_extendees->find(name) != @@ -1768,8 +1768,7 @@ void FileDescriptorTables::FieldsByLowercaseNamesLazyInitInternal() const { for (Symbol symbol : symbols_by_parent_) { const FieldDescriptor* field = symbol.field_descriptor(); if (!field) continue; - (*map)[{FindParentForFieldsByMap(field), field->lowercase_name().c_str()}] = - field; + (*map)[{FindParentForFieldsByMap(field), field->lowercase_name()}] = field; } fields_by_lowercase_name_.store(map, std::memory_order_release); } @@ -1799,8 +1798,7 @@ void FileDescriptorTables::FieldsByCamelcaseNamesLazyInitInternal() const { const void* parent = FindParentForFieldsByMap(field); // If we already have a field with this camelCase name, keep the field with // the smallest field number. This way we get a deterministic mapping. - const FieldDescriptor*& found = - (*map)[{parent, field->camelcase_name().c_str()}]; + const FieldDescriptor*& found = (*map)[{parent, field->camelcase_name()}]; if (found == nullptr || found->number() > field->number()) { found = field; } @@ -1867,8 +1865,8 @@ FileDescriptorTables::FindEnumValueByNumberCreatingIfUnknown( // EnumDescriptor (it's not a part of the enum as originally defined), but // we do insert it into the table so that we can return the same pointer // later. - std::string enum_value_name = absl::StrFormat( - "UNKNOWN_ENUM_VALUE_%s_%d", parent->name().c_str(), number); + std::string enum_value_name = + absl::StrFormat("UNKNOWN_ENUM_VALUE_%s_%d", parent->name(), number); auto* pool = DescriptorPool::generated_pool(); auto* tables = const_cast(pool->tables_.get()); internal::FlatAllocator alloc; @@ -2448,8 +2446,8 @@ void DescriptorPool::FindAllExtensions( if (fallback_database_ != nullptr && tables_->extensions_loaded_from_db_.count(extendee) == 0) { std::vector numbers; - if (fallback_database_->FindAllExtensionNumbers(extendee->full_name(), - &numbers)) { + if (fallback_database_->FindAllExtensionNumbers( + std::string(extendee->full_name()), &numbers)) { for (int number : numbers) { if (tables_->FindExtension(extendee, number) == nullptr) { TryFindExtensionInFallbackDatabase(extendee, number, @@ -2780,7 +2778,8 @@ bool DescriptorPool::TryFindExtensionInFallbackDatabase( auto& file_proto = deferred_validation.CreateProto(); if (!fallback_database_->FindFileContainingExtension( - containing_type->full_name(), field_number, &file_proto)) { + std::string(containing_type->full_name()), field_number, + &file_proto)) { return false; } @@ -2829,11 +2828,11 @@ std::string FieldDescriptor::DefaultValueAsString( if (type() == TYPE_BYTES) { return absl::CEscape(default_value_string()); } else { - return default_value_string(); + return std::string(default_value_string()); } } case CPPTYPE_ENUM: - return default_value_enum()->name(); + return std::string(default_value_enum()->name()); case CPPTYPE_MESSAGE: ABSL_DLOG(FATAL) << "Messages can't have default values!"; break; @@ -3226,8 +3225,7 @@ bool RetrieveOptions(int depth, const Message& options, factory.GetPrototype(option_descriptor)->New()); std::string serialized = options.SerializeAsString(); io::CodedInputStream input( - reinterpret_cast(serialized.c_str()), - serialized.size()); + reinterpret_cast(serialized.data()), serialized.size()); input.SetExtensionRegistry(pool, &factory); if (dynamic_options->ParseFromCodedStream(&input)) { return RetrieveOptionsAssumingRightPool(depth, *dynamic_options, @@ -4219,10 +4217,10 @@ class DescriptorBuilder { // The `const char*` overload should only be used for string literal messages // where this is a frustrating amount of overhead and there is no harm in // directly using the literal. - void AddError(const std::string& element_name, const Message& descriptor, + void AddError(absl::string_view element_name, const Message& descriptor, DescriptorPool::ErrorCollector::ErrorLocation location, absl::FunctionRef make_error); - void AddError(const std::string& element_name, const Message& descriptor, + void AddError(absl::string_view element_name, const Message& descriptor, DescriptorPool::ErrorCollector::ErrorLocation location, const char* error); void AddRecursiveImportError(const FileDescriptorProto& proto, int from_here); @@ -4232,14 +4230,14 @@ class DescriptorBuilder { // Adds an error indicating that undefined_symbol was not defined. Must // only be called after LookupSymbol() fails. void AddNotDefinedError( - const std::string& element_name, const Message& descriptor, + absl::string_view element_name, const Message& descriptor, DescriptorPool::ErrorCollector::ErrorLocation location, - const std::string& undefined_symbol); + absl::string_view undefined_symbol); - void AddWarning(const std::string& element_name, const Message& descriptor, + void AddWarning(absl::string_view element_name, const Message& descriptor, DescriptorPool::ErrorCollector::ErrorLocation location, absl::FunctionRef make_error); - void AddWarning(const std::string& element_name, const Message& descriptor, + void AddWarning(absl::string_view element_name, const Message& descriptor, DescriptorPool::ErrorCollector::ErrorLocation location, const char* error); @@ -4256,16 +4254,16 @@ class DescriptorBuilder { // - Search the pool's underlay if not found in tables_. // - Insure that the resulting Symbol is from one of the file's declared // dependencies. - Symbol FindSymbol(const std::string& name, bool build_it = true); + Symbol FindSymbol(absl::string_view name, bool build_it = true); // Like FindSymbol() but does not require that the symbol is in one of the // file's declared dependencies. - Symbol FindSymbolNotEnforcingDeps(const std::string& name, + Symbol FindSymbolNotEnforcingDeps(absl::string_view name, bool build_it = true); // This implements the body of FindSymbolNotEnforcingDeps(). Symbol FindSymbolNotEnforcingDepsHelper(const DescriptorPool* pool, - const std::string& name, + absl::string_view name, bool build_it = true); // Like FindSymbol(), but looks up the name relative to some other symbol @@ -4283,7 +4281,7 @@ class DescriptorBuilder { // if it believes that's all it could refer to. The caller should always // check that it receives the type of symbol it was expecting. enum ResolveMode { LOOKUP_ALL, LOOKUP_TYPES }; - Symbol LookupSymbol(const std::string& name, const std::string& relative_to, + Symbol LookupSymbol(absl::string_view name, absl::string_view relative_to, DescriptorPool::PlaceholderType placeholder_type = DescriptorPool::PLACEHOLDER_MESSAGE, ResolveMode resolve_mode = LOOKUP_ALL, @@ -4291,28 +4289,28 @@ class DescriptorBuilder { // Like LookupSymbol() but will not return a placeholder even if // AllowUnknownDependencies() has been used. - Symbol LookupSymbolNoPlaceholder(const std::string& name, - const std::string& relative_to, + Symbol LookupSymbolNoPlaceholder(absl::string_view name, + absl::string_view relative_to, ResolveMode resolve_mode = LOOKUP_ALL, bool build_it = true); // Calls tables_->AddSymbol() and records an error if it fails. Returns // true if successful or false if failed, though most callers can ignore // the return value since an error has already been recorded. - bool AddSymbol(const std::string& full_name, const void* parent, - const std::string& name, const Message& proto, Symbol symbol); + bool AddSymbol(absl::string_view full_name, const void* parent, + absl::string_view name, const Message& proto, Symbol symbol); // Like AddSymbol(), but succeeds if the symbol is already defined as long // as the existing definition is also a package (because it's OK to define // the same package in two different files). Also adds all parents of the // package to the symbol table (e.g. AddPackage("foo.bar", ...) will add // "foo.bar" and "foo" to the table). - void AddPackage(const std::string& name, const Message& proto, - FileDescriptor* file); + void AddPackage(absl::string_view name, const Message& proto, + FileDescriptor* file, bool toplevel); // Checks that the symbol name contains only alphanumeric characters and // underscores. Records an error otherwise. - void ValidateSymbolName(const std::string& name, const std::string& full_name, + void ValidateSymbolName(absl::string_view name, absl::string_view full_name, const Message& proto); // Allocates a copy of orig_options in tables_ and stores it in the @@ -4365,8 +4363,8 @@ class DescriptorBuilder { // Allocates an array of two strings, the first one is a copy of // `proto_name`, and the second one is the full name. Full proto name is // "scope.proto_name" if scope is non-empty and "proto_name" otherwise. - const std::string* AllocateNameStrings(const std::string& scope, - const std::string& proto_name, + const std::string* AllocateNameStrings(absl::string_view scope, + absl::string_view proto_name, internal::FlatAllocator& alloc); // These methods all have the same signature for the sake of the BUILD_ARRAY @@ -4411,7 +4409,7 @@ class DescriptorBuilder { void CheckFieldJsonNameUniqueness(const DescriptorProto& proto, const Descriptor* result); - void CheckFieldJsonNameUniqueness(const std::string& message_name, + void CheckFieldJsonNameUniqueness(absl::string_view message_name, const DescriptorProto& message, const Descriptor* descriptor, bool use_custom_names); @@ -4641,7 +4639,7 @@ class DescriptorBuilder { void ValidateExtensionRangeOptions(const DescriptorProto& proto, const Descriptor& message); void ValidateExtensionDeclaration( - const std::string& full_name, + absl::string_view full_name, const RepeatedPtrField& declarations, const DescriptorProto_ExtensionRange& proto, absl::flat_hash_set& full_name_set); @@ -4780,7 +4778,7 @@ DescriptorBuilder::DescriptorBuilder( DescriptorBuilder::~DescriptorBuilder() = default; PROTOBUF_NOINLINE void DescriptorBuilder::AddError( - const std::string& element_name, const Message& descriptor, + const absl::string_view element_name, const Message& descriptor, DescriptorPool::ErrorCollector::ErrorLocation location, absl::FunctionRef make_error) { std::string error = make_error(); @@ -4798,15 +4796,15 @@ PROTOBUF_NOINLINE void DescriptorBuilder::AddError( } PROTOBUF_NOINLINE void DescriptorBuilder::AddError( - const std::string& element_name, const Message& descriptor, + const absl::string_view element_name, const Message& descriptor, DescriptorPool::ErrorCollector::ErrorLocation location, const char* error) { AddError(element_name, descriptor, location, [error] { return error; }); } PROTOBUF_NOINLINE void DescriptorBuilder::AddNotDefinedError( - const std::string& element_name, const Message& descriptor, + const absl::string_view element_name, const Message& descriptor, DescriptorPool::ErrorCollector::ErrorLocation location, - const std::string& undefined_symbol) { + const absl::string_view undefined_symbol) { if (possible_undeclared_dependency_ == nullptr && undefine_resolved_name_.empty()) { AddError(element_name, descriptor, location, [&] { @@ -4840,7 +4838,7 @@ PROTOBUF_NOINLINE void DescriptorBuilder::AddNotDefinedError( } PROTOBUF_NOINLINE void DescriptorBuilder::AddWarning( - const std::string& element_name, const Message& descriptor, + const absl::string_view element_name, const Message& descriptor, DescriptorPool::ErrorCollector::ErrorLocation location, absl::FunctionRef make_error) { std::string error = make_error(); @@ -4853,7 +4851,7 @@ PROTOBUF_NOINLINE void DescriptorBuilder::AddWarning( } PROTOBUF_NOINLINE void DescriptorBuilder::AddWarning( - const std::string& element_name, const Message& descriptor, + const absl::string_view element_name, const Message& descriptor, DescriptorPool::ErrorCollector::ErrorLocation location, const char* error) { AddWarning(element_name, descriptor, location, [error]() -> std::string { return error; }); @@ -4874,7 +4872,7 @@ void DescriptorBuilder::RecordPublicDependencies(const FileDescriptor* file) { } Symbol DescriptorBuilder::FindSymbolNotEnforcingDepsHelper( - const DescriptorPool* pool, const std::string& name, bool build_it) { + const DescriptorPool* pool, const absl::string_view name, bool build_it) { // If we are looking at an underlay, we must lock its mutex_, since we are // accessing the underlay's tables_ directly. absl::MutexLockMaybe lock((pool == pool_) ? nullptr : pool->mutex_); @@ -4902,8 +4900,8 @@ Symbol DescriptorBuilder::FindSymbolNotEnforcingDepsHelper( return result; } -Symbol DescriptorBuilder::FindSymbolNotEnforcingDeps(const std::string& name, - bool build_it) { +Symbol DescriptorBuilder::FindSymbolNotEnforcingDeps( + const absl::string_view name, bool build_it) { Symbol result = FindSymbolNotEnforcingDepsHelper(pool_, name, build_it); // Only find symbols which were defined in this file or one of its // dependencies. @@ -4914,7 +4912,8 @@ Symbol DescriptorBuilder::FindSymbolNotEnforcingDeps(const std::string& name, return result; } -Symbol DescriptorBuilder::FindSymbol(const std::string& name, bool build_it) { +Symbol DescriptorBuilder::FindSymbol(const absl::string_view name, + bool build_it) { Symbol result = FindSymbolNotEnforcingDeps(name, build_it); if (result.IsNull()) return result; @@ -4947,12 +4946,12 @@ Symbol DescriptorBuilder::FindSymbol(const std::string& name, bool build_it) { } possible_undeclared_dependency_ = file; - possible_undeclared_dependency_name_ = name; + possible_undeclared_dependency_name_ = std::string(name); return Symbol(); } Symbol DescriptorBuilder::LookupSymbolNoPlaceholder( - const std::string& name, const std::string& relative_to, + const absl::string_view name, const absl::string_view relative_to, ResolveMode resolve_mode, bool build_it) { possible_undeclared_dependency_ = nullptr; undefine_resolved_name_.clear(); @@ -4974,7 +4973,7 @@ Symbol DescriptorBuilder::LookupSymbolNoPlaceholder( // So, we look for just "Foo" first, then look for "Bar.baz" within it if // found. std::string::size_type name_dot_pos = name.find_first_of('.'); - std::string first_part_of_name; + absl::string_view first_part_of_name; if (name_dot_pos == std::string::npos) { first_part_of_name = name; } else { @@ -4994,16 +4993,15 @@ Symbol DescriptorBuilder::LookupSymbolNoPlaceholder( // Append ".first_part_of_name" and try to find. std::string::size_type old_size = scope_to_try.size(); - scope_to_try.append(1, '.'); - scope_to_try.append(first_part_of_name); + absl::StrAppend(&scope_to_try, ".", first_part_of_name); Symbol result = FindSymbol(scope_to_try, build_it); if (!result.IsNull()) { if (first_part_of_name.size() < name.size()) { // name is a compound symbol, of which we only found the first part. // Now try to look up the rest of it. if (result.IsAggregate()) { - scope_to_try.append(name, first_part_of_name.size(), - name.size() - first_part_of_name.size()); + absl::StrAppend(&scope_to_try, + name.substr(first_part_of_name.size())); result = FindSymbol(scope_to_try, build_it); if (result.IsNull()) { undefine_resolved_name_ = scope_to_try; @@ -5027,7 +5025,7 @@ Symbol DescriptorBuilder::LookupSymbolNoPlaceholder( } Symbol DescriptorBuilder::LookupSymbol( - const std::string& name, const std::string& relative_to, + const absl::string_view name, const absl::string_view relative_to, DescriptorPool::PlaceholderType placeholder_type, ResolveMode resolve_mode, bool build_it) { Symbol result = @@ -5191,7 +5189,7 @@ Symbol DescriptorPool::NewPlaceholderWithMutexHeld( } FileDescriptor* DescriptorPool::NewPlaceholderFile( - absl::string_view name) const { + const absl::string_view name) const { absl::MutexLockMaybe lock(mutex_); internal::FlatAllocator alloc; alloc.PlanArray(1); @@ -5202,7 +5200,7 @@ FileDescriptor* DescriptorPool::NewPlaceholderFile( } FileDescriptor* DescriptorPool::NewPlaceholderFileWithMutexHeld( - absl::string_view name, internal::FlatAllocator& alloc) const { + const absl::string_view name, internal::FlatAllocator& alloc) const { if (mutex_) { mutex_->AssertHeld(); } @@ -5224,8 +5222,9 @@ FileDescriptor* DescriptorPool::NewPlaceholderFileWithMutexHeld( return placeholder; } -bool DescriptorBuilder::AddSymbol(const std::string& full_name, - const void* parent, const std::string& name, +bool DescriptorBuilder::AddSymbol(const absl::string_view full_name, + const void* parent, + const absl::string_view name, const Message& proto, Symbol symbol) { // If the caller passed nullptr for the parent, the symbol is at file scope. // Use its file as the parent instead. @@ -5277,8 +5276,9 @@ bool DescriptorBuilder::AddSymbol(const std::string& full_name, } } -void DescriptorBuilder::AddPackage(const std::string& name, - const Message& proto, FileDescriptor* file) { +void DescriptorBuilder::AddPackage(const absl::string_view name, + const Message& proto, FileDescriptor* file, + bool toplevel) { if (absl::StrContains(name, '\0')) { AddError(name, proto, DescriptorPool::ErrorCollector::NAME, [&] { return absl::StrCat("\"", name, "\" contains null character."); @@ -5289,7 +5289,7 @@ void DescriptorBuilder::AddPackage(const std::string& name, Symbol existing_symbol = tables_->FindSymbol(name); // It's OK to redefine a package. if (existing_symbol.IsNull()) { - if (name.data() == file->package().data()) { + if (toplevel) { // It is the toplevel package name, so insert the descriptor directly. tables_->AddSymbol(file->package(), Symbol(file)); } else { @@ -5307,7 +5307,7 @@ void DescriptorBuilder::AddPackage(const std::string& name, ValidateSymbolName(name, name, proto); } else { // Has parent. - AddPackage(name.substr(0, dot_pos), proto, file); + AddPackage(name.substr(0, dot_pos), proto, file, false); ValidateSymbolName(name.substr(dot_pos + 1), name, proto); } } else if (!existing_symbol.IsPackage()) { @@ -5323,8 +5323,8 @@ void DescriptorBuilder::AddPackage(const std::string& name, } } -void DescriptorBuilder::ValidateSymbolName(const std::string& name, - const std::string& full_name, +void DescriptorBuilder::ValidateSymbolName(const absl::string_view name, + const absl::string_view full_name, const Message& proto) { if (name.empty()) { AddError(full_name, proto, DescriptorPool::ErrorCollector::NAME, @@ -5949,7 +5949,7 @@ FileDescriptor* DescriptorBuilder::BuildFileImpl( "Exceeds Maximum Package Depth"); return nullptr; } - AddPackage(result->package(), proto, result); + AddPackage(result->package(), proto, result, true); } // Make sure all dependencies are loaded. @@ -6028,7 +6028,7 @@ FileDescriptor* DescriptorBuilder::BuildFileImpl( for (int i = 0; i < proto.dependency_size(); i++) { if (result->dependencies_[i] == nullptr) { - memcpy(name_data, proto.dependency(i).c_str(), + memcpy(name_data, proto.dependency(i).data(), proto.dependency(i).size()); name_data += proto.dependency(i).size(); } @@ -6122,12 +6122,13 @@ FileDescriptor* DescriptorBuilder::BuildFileImpl( if (field.options_->has_ctype() && field.options_->features() .GetExtension(pb::cpp) .has_string_type()) { - AddError( - field.full_name(), proto, DescriptorPool::ErrorCollector::TYPE, - absl::StrFormat("Field %s specifies both string_type and ctype " - "which is not supported.", - field.full_name()) - .c_str()); + AddError(field.full_name(), proto, + DescriptorPool::ErrorCollector::TYPE, [&] { + return absl::StrFormat( + "Field %s specifies both string_type and ctype " + "which is not supported.", + field.full_name()); + }); } }); @@ -6225,7 +6226,7 @@ FileDescriptor* DescriptorBuilder::BuildFileImpl( const std::string* DescriptorBuilder::AllocateNameStrings( - const std::string& scope, const std::string& proto_name, + const absl::string_view scope, const absl::string_view proto_name, internal::FlatAllocator& alloc) { if (scope.empty()) { return alloc.AllocateStrings(proto_name, proto_name); @@ -6261,7 +6262,7 @@ void DescriptorBuilder::BuildMessage(const DescriptorProto& proto, const Descriptor* parent, Descriptor* result, internal::FlatAllocator& alloc) { - const std::string& scope = + const absl::string_view scope = (parent == nullptr) ? file_->package() : parent->full_name(); result->all_names_ = AllocateNameStrings(scope, proto.name(), alloc); ValidateSymbolName(proto.name(), result->full_name(), proto); @@ -6430,7 +6431,7 @@ void DescriptorBuilder::BuildMessage(const DescriptorProto& proto, void DescriptorBuilder::CheckFieldJsonNameUniqueness( const DescriptorProto& proto, const Descriptor* result) { - std::string message_name = result->full_name(); + const absl::string_view message_name = result->full_name(); if (!pool_->deprecated_legacy_json_field_conflicts_ && !IsLegacyJsonFieldConflictEnabled(result->options())) { // Check both with and without taking json_name into consideration. This is @@ -6466,7 +6467,7 @@ bool JsonNameLooksLikeExtension(std::string name) { } // namespace void DescriptorBuilder::CheckFieldJsonNameUniqueness( - const std::string& message_name, const DescriptorProto& message, + const absl::string_view message_name, const DescriptorProto& message, const Descriptor* descriptor, bool use_custom_names) { absl::flat_hash_map name_to_field; for (const FieldDescriptorProto& field : message.field()) { @@ -6530,7 +6531,7 @@ void DescriptorBuilder::BuildFieldOrExtension(const FieldDescriptorProto& proto, FieldDescriptor* result, bool is_extension, internal::FlatAllocator& alloc) { - const std::string& scope = + const absl::string_view scope = (parent == nullptr) ? file_->package() : parent->full_name(); // We allocate all names in a single array, and dedup them. @@ -6966,7 +6967,7 @@ void DescriptorBuilder::BuildEnum(const EnumDescriptorProto& proto, const Descriptor* parent, EnumDescriptor* result, internal::FlatAllocator& alloc) { - const std::string& scope = + const absl::string_view scope = (parent == nullptr) ? file_->package() : parent->full_name(); result->all_names_ = AllocateNameStrings(scope, proto.name(), alloc); @@ -7592,9 +7593,9 @@ void DescriptorBuilder::CrossLinkField(FieldDescriptor* field, if (!file_tables_->AddFieldByNumber(field)) { const FieldDescriptor* conflicting_field = file_tables_->FindFieldByNumber( field->containing_type(), field->number()); - std::string containing_type_name = + const absl::string_view containing_type_name = field->containing_type() == nullptr - ? "unknown" + ? absl::string_view("unknown") : field->containing_type()->full_name(); if (field->is_extension()) { AddError(field->full_name(), proto, @@ -7621,9 +7622,9 @@ void DescriptorBuilder::CrossLinkField(FieldDescriptor* field, auto make_error = [&] { const FieldDescriptor* conflicting_field = tables_->FindExtension(field->containing_type(), field->number()); - std::string containing_type_name = + const absl::string_view containing_type_name = field->containing_type() == nullptr - ? "unknown" + ? absl::string_view("unknown") : field->containing_type()->full_name(); return absl::Substitute( "Extension number $0 has already been used in \"$1\" by " @@ -8276,7 +8277,7 @@ absl::optional ValidateSymbolForDeclaration( void DescriptorBuilder::ValidateExtensionDeclaration( - const std::string& full_name, + const absl::string_view full_name, const RepeatedPtrField& declarations, const DescriptorProto_ExtensionRange& proto, absl::flat_hash_set& full_name_set) { @@ -9292,7 +9293,7 @@ bool DescriptorBuilder::OptionInterpreter::SetOptionValue( if (enum_type->file()->pool() != DescriptorPool::generated_pool()) { // Note that the enum value's fully-qualified name is a sibling of the // enum's name, not a child of it. - std::string fully_qualified_name = enum_type->full_name(); + std::string fully_qualified_name = std::string(enum_type->full_name()); fully_qualified_name.resize(fully_qualified_name.size() - enum_type->name().size()); fully_qualified_name += value_name; @@ -9621,7 +9622,7 @@ void FieldDescriptor::InternalTypeOnceInit() const { if (lazy_default_value_enum_name[0] != '\0') { // Have to build the full name now instead of at CrossLink time, // because enum_type may not be known at the time. - std::string name = enum_type->full_name(); + std::string name = std::string(enum_type->full_name()); // Enum values reside in the same scope as the enum type. std::string::size_type last_dot = name.find_last_of('.'); if (last_dot != std::string::npos) { @@ -9678,7 +9679,8 @@ const EnumValueDescriptor* FieldDescriptor::default_value_enum() const { return default_value_enum_; } -const std::string& FieldDescriptor::PrintableNameForExtension() const { +internal::DescriptorStringView FieldDescriptor::PrintableNameForExtension() + const { const bool is_message_set_extension = is_extension() && containing_type()->options().message_set_wire_format() && diff --git a/src/google/protobuf/descriptor.h b/src/google/protobuf/descriptor.h index 2a1dfb59de518..28db34d95ee61 100644 --- a/src/google/protobuf/descriptor.h +++ b/src/google/protobuf/descriptor.h @@ -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 { @@ -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. @@ -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. @@ -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(); } @@ -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; @@ -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. @@ -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 @@ -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. @@ -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. @@ -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 --------------------------------------------------- @@ -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. @@ -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; @@ -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; @@ -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; @@ -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. @@ -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; @@ -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. @@ -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; @@ -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; @@ -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. @@ -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) \ @@ -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]; } @@ -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_]; } @@ -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; } diff --git a/src/google/protobuf/descriptor_unittest.cc b/src/google/protobuf/descriptor_unittest.cc index 3529aca8f9009..0c13b3a2b46e4 100644 --- a/src/google/protobuf/descriptor_unittest.cc +++ b/src/google/protobuf/descriptor_unittest.cc @@ -570,15 +570,15 @@ TEST_F(FileDescriptorTest, CopyHeadingTo) { } void ExtractDebugString( - const FileDescriptor* file, absl::flat_hash_set* visited, - std::vector>* debug_strings) { + const FileDescriptor* file, absl::flat_hash_set* visited, + std::vector>* 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 { @@ -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 visited; - std::vector> debug_strings; + absl::flat_hash_set visited; + std::vector> debug_strings; ExtractDebugString(protobuf_unittest::TestAllTypes::descriptor()->file(), &visited, &debug_strings); ExtractDebugString( @@ -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; @@ -850,16 +850,17 @@ TEST_F(DescriptorTest, ContainingType) { TEST_F(DescriptorTest, FieldNamesDedup) { const auto collect_unique_names = [](const FieldDescriptor* field) { - absl::btree_set names{field->name(), field->lowercase_name(), - field->camelcase_name(), - field->json_name()}; + absl::btree_set 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{ - &field->name(), &field->lowercase_name(), - &field->camelcase_name(), &field->json_name()} - .size())) + EXPECT_EQ(names.size(), + (absl::flat_hash_set{ + field->name().data(), field->lowercase_name().data(), + field->camelcase_name().data(), field->json_name().data()} + .size())) << testing::PrintToString(names); return names; }; diff --git a/src/google/protobuf/descriptor_visitor_test.cc b/src/google/protobuf/descriptor_visitor_test.cc index 0c0007676f8c7..aacf19e652019 100644 --- a/src/google/protobuf/descriptor_visitor_test.cc +++ b/src/google/protobuf/descriptor_visitor_test.cc @@ -29,7 +29,7 @@ constexpr absl::string_view kUnittestProtoFile = TEST(VisitDescriptorsTest, SingleTypeNoProto) { const FileDescriptor& file = *protobuf_unittest::TestAllTypes::GetDescriptor()->file(); - std::vector descriptors; + std::vector descriptors; VisitDescriptors(file, [&](const Descriptor& descriptor) { descriptors.push_back(descriptor.full_name()); }); @@ -43,7 +43,7 @@ TEST(VisitDescriptorsTest, SingleTypeWithProto) { *protobuf_unittest::TestAllTypes::GetDescriptor()->file(); FileDescriptorProto proto; file.CopyTo(&proto); - std::vector descriptors; + std::vector descriptors; VisitDescriptors( file, proto, [&](const Descriptor& descriptor, const DescriptorProto& proto) { @@ -60,7 +60,7 @@ TEST(VisitDescriptorsTest, SingleTypeMutableProto) { *protobuf_unittest::TestAllTypes::GetDescriptor()->file(); FileDescriptorProto proto; file.CopyTo(&proto); - std::vector descriptors; + std::vector descriptors; VisitDescriptors(file, proto, [&](const Descriptor& descriptor, DescriptorProto& proto) { descriptors.push_back(descriptor.full_name()); @@ -76,7 +76,7 @@ TEST(VisitDescriptorsTest, SingleTypeMutableProto) { TEST(VisitDescriptorsTest, AllTypesDeduce) { const FileDescriptor& file = *protobuf_unittest::TestAllTypes::GetDescriptor()->file(); - std::vector descriptors; + std::vector descriptors; VisitDescriptors(file, [&](const auto& descriptor) { descriptors.push_back(descriptor.name()); }); @@ -90,7 +90,7 @@ TEST(VisitDescriptorsTest, AllTypesDeduce) { TEST(VisitDescriptorsTest, AllTypesDeduceSelective) { const FileDescriptor& file = *protobuf_unittest::TestAllTypes::GetDescriptor()->file(); - std::vector descriptors; + std::vector descriptors; VisitDescriptors( file, // Only select on descriptors with a full_name method. @@ -112,12 +112,12 @@ TEST(VisitDescriptorsTest, AllTypesDeduceSelective) { } void TestHandle(const Descriptor& message, const DescriptorProto& proto, - std::vector* result) { + std::vector* result) { if (result != nullptr) result->push_back(message.full_name()); EXPECT_EQ(message.name(), proto.name()); } void TestHandle(const EnumDescriptor& enm, const EnumDescriptorProto& proto, - std::vector* result) { + std::vector* result) { if (result != nullptr) result->push_back(enm.full_name()); EXPECT_EQ(enm.name(), proto.name()); } @@ -126,7 +126,7 @@ TEST(VisitDescriptorsTest, AllTypesDeduceDelegate) { *protobuf_unittest::TestAllTypes::GetDescriptor()->file(); FileDescriptorProto proto; file.CopyTo(&proto); - std::vector descriptors; + std::vector descriptors; VisitDescriptors(file, proto, [&](const auto& descriptor, const auto& proto) diff --git a/src/google/protobuf/generated_message_reflection.cc b/src/google/protobuf/generated_message_reflection.cc index 607addc73bdbe..fba915fe5d1f0 100644 --- a/src/google/protobuf/generated_message_reflection.cc +++ b/src/google/protobuf/generated_message_reflection.cc @@ -113,7 +113,7 @@ bool ParseNamedEnum(const EnumDescriptor* descriptor, absl::string_view name, const std::string& NameOfEnum(const EnumDescriptor* descriptor, int value) { const EnumValueDescriptor* d = descriptor->FindValueByNumber(value); - return (d == nullptr ? GetEmptyString() : d->name()); + return (d == nullptr ? GetEmptyString() : internal::NameOfEnumAsString(d)); } // Internal helper routine for NameOfDenseEnum in the header file. @@ -131,7 +131,7 @@ const std::string** MakeDenseEnumCache(const EnumDescriptor* desc, int min_val, if (str_ptrs[num - min_val] == nullptr) { // Don't over-write an existing entry, because in case of duplication, the // first one wins. - str_ptrs[num - min_val] = &desc->value(i)->name(); + str_ptrs[num - min_val] = &internal::NameOfEnumAsString(desc->value(i)); } } // Change any unfilled entries to point to the empty string. @@ -1796,11 +1796,11 @@ std::string Reflection::GetString(const Message& message, const FieldDescriptor* field) const { USAGE_CHECK_ALL(GetString, SINGULAR, STRING); if (field->is_extension()) { - return GetExtensionSet(message).GetString(field->number(), - field->default_value_string()); + return GetExtensionSet(message).GetString( + field->number(), internal::DefaultValueStringAsString(field)); } else { if (schema_.InRealOneof(field) && !HasOneofField(message, field)) { - return field->default_value_string(); + return std::string(field->default_value_string()); } switch (internal::cpp::EffectiveStringCType(field)) { case FieldOptions::CORD: @@ -1815,7 +1815,8 @@ std::string Reflection::GetString(const Message& message, return GetField(message, field).GetNoArena(); } else { const auto& str = GetField(message, field); - return str.IsDefault() ? field->default_value_string() : str.Get(); + return str.IsDefault() ? std::string(field->default_value_string()) + : str.Get(); } } } @@ -1827,11 +1828,11 @@ const std::string& Reflection::GetStringReference(const Message& message, (void)scratch; // Parameter is used by Google-internal code. USAGE_CHECK_ALL(GetStringReference, SINGULAR, STRING); if (field->is_extension()) { - return GetExtensionSet(message).GetString(field->number(), - field->default_value_string()); + return GetExtensionSet(message).GetString( + field->number(), internal::DefaultValueStringAsString(field)); } else { if (schema_.InRealOneof(field) && !HasOneofField(message, field)) { - return field->default_value_string(); + return internal::DefaultValueStringAsString(field); } switch (internal::cpp::EffectiveStringCType(field)) { case FieldOptions::CORD: @@ -1848,7 +1849,8 @@ const std::string& Reflection::GetStringReference(const Message& message, return GetField(message, field).GetNoArena(); } else { const auto& str = GetField(message, field); - return str.IsDefault() ? field->default_value_string() : str.Get(); + return str.IsDefault() ? internal::DefaultValueStringAsString(field) + : str.Get(); } } } @@ -1859,7 +1861,7 @@ absl::Cord Reflection::GetCord(const Message& message, USAGE_CHECK_ALL(GetCord, SINGULAR, STRING); if (field->is_extension()) { return absl::Cord(GetExtensionSet(message).GetString( - field->number(), field->default_value_string())); + field->number(), internal::DefaultValueStringAsString(field))); } else { if (schema_.InRealOneof(field) && !HasOneofField(message, field)) { return absl::Cord(field->default_value_string()); @@ -1894,8 +1896,8 @@ absl::string_view Reflection::GetStringView(const Message& message, USAGE_CHECK_ALL(GetStringView, SINGULAR, STRING); if (field->is_extension()) { - return GetExtensionSet(message).GetString(field->number(), - field->default_value_string()); + return GetExtensionSet(message).GetString( + field->number(), internal::DefaultValueStringAsString(field)); } if (schema_.InRealOneof(field) && !HasOneofField(message, field)) { return field->default_value_string(); diff --git a/src/google/protobuf/io/printer.h b/src/google/protobuf/io/printer.h index 323dbb8b5fe2b..7677e9dbb9e0c 100644 --- a/src/google/protobuf/io/printer.h +++ b/src/google/protobuf/io/printer.h @@ -532,12 +532,13 @@ class PROTOBUF_EXPORT Printer { // Pushes a new variable lookup frame that stores `vars` by value. // // Returns an RAII object that pops the lookup frame. - template , - typename = std::enable_if_t::value>, - // Prefer the more specific span impl if this could be turned into - // a span. - typename = std::enable_if_t< - !std::is_convertible>::value>> + template < + typename Map = absl::flat_hash_map, + typename = std::enable_if_t::value>, + // Prefer the more specific span impl if this could be turned into + // a span. + typename = std::enable_if_t< + !std::is_convertible>::value>> auto WithVars(Map&& vars); // Pushes a new variable lookup frame that stores `vars` by value. @@ -608,7 +609,8 @@ class PROTOBUF_EXPORT Printer { // -- Old-style API below; to be deprecated and removed. -- // TODO: Deprecate these APIs. - template > + template < + typename Map = absl::flat_hash_map> void Print(const Map& vars, absl::string_view text); template diff --git a/src/google/protobuf/json/internal/descriptor_traits.h b/src/google/protobuf/json/internal/descriptor_traits.h index 609a956d5e4ea..72ece4fa387d7 100644 --- a/src/google/protobuf/json/internal/descriptor_traits.h +++ b/src/google/protobuf/json/internal/descriptor_traits.h @@ -277,7 +277,7 @@ struct Proto2Descriptor { static absl::StatusOr EnumNameByNumber(Field f, int32_t number) { if (const auto* ev = f->enum_type()->FindValueByNumber(number)) { - return ev->name(); + return std::string(ev->name()); } return absl::InvalidArgumentError( absl::StrFormat("unknown enum number: '%d'", number)); diff --git a/src/google/protobuf/port_def.inc b/src/google/protobuf/port_def.inc index 37d80e591782f..52fa1b315bcfa 100644 --- a/src/google/protobuf/port_def.inc +++ b/src/google/protobuf/port_def.inc @@ -145,6 +145,13 @@ static_assert(PROTOBUF_ABSL_MIN(20230125, 3), #endif +#ifdef PROTOBUF_FUTURE_STRING_VIEW_RETURN_TYPE +#error PROTOBUF_FUTURE_STRING_VIEW_RETURN_TYPE was previously defined +#endif +#if defined(PROTOBUF_TEMPORARY_ENABLE_STRING_VIEW_RETURN_TYPE) +#define PROTOBUF_FUTURE_STRING_VIEW_RETURN_TYPE 1 +#endif + #ifdef PROTOBUF_MINIMUM_EDITION #error PROTOBUF_MINIMUM_EDITION was previously defined #endif diff --git a/src/google/protobuf/port_undef.inc b/src/google/protobuf/port_undef.inc index 152f1229bb79b..0855b44c7f17e 100644 --- a/src/google/protobuf/port_undef.inc +++ b/src/google/protobuf/port_undef.inc @@ -83,6 +83,7 @@ #ifdef PROTOBUF_FUTURE_BREAKING_CHANGES #undef PROTOBUF_FUTURE_BREAKING_CHANGES #undef PROTOBUF_FUTURE_DESCRIPTOR_EXTENSION_DECL +#undef PROTOBUF_FUTURE_STRING_VIEW_RETURN_TYPE #endif // Restore macros that may have been #undef'd in port_def.inc. diff --git a/src/google/protobuf/reflection_ops.cc b/src/google/protobuf/reflection_ops.cc index 233d25847c360..3b77cf41e0ec9 100644 --- a/src/google/protobuf/reflection_ops.cc +++ b/src/google/protobuf/reflection_ops.cc @@ -33,10 +33,9 @@ static const Reflection* GetReflectionOrDie(const Message& m) { const Reflection* r = m.GetReflection(); if (r == nullptr) { const Descriptor* d = m.GetDescriptor(); - const std::string& mtype = d ? d->name() : "unknown"; // RawMessage is one known type for which GetReflection() returns nullptr. - ABSL_LOG(FATAL) << "Message does not support reflection (type " << mtype - << ")."; + ABSL_LOG(FATAL) << "Message does not support reflection (type " + << (d ? d->name() : "unknown") << ")."; } return r; } @@ -389,7 +388,7 @@ void ReflectionOps::FindInitializationErrors(const Message& message, for (int i = 0; i < field_count; i++) { if (descriptor->field(i)->is_required()) { if (!reflection->HasField(message, descriptor->field(i))) { - errors->push_back(prefix + descriptor->field(i)->name()); + errors->push_back(absl::StrCat(prefix, descriptor->field(i)->name())); } } } diff --git a/src/google/protobuf/reflection_tester.cc b/src/google/protobuf/reflection_tester.cc index 8698f9775ea1e..259f0fb713802 100644 --- a/src/google/protobuf/reflection_tester.cc +++ b/src/google/protobuf/reflection_tester.cc @@ -22,7 +22,7 @@ namespace protobuf { MapReflectionTester::MapReflectionTester(const Descriptor* base_descriptor) : base_descriptor_(base_descriptor) { const DescriptorPool* pool = base_descriptor->file()->pool(); - std::string package = base_descriptor->file()->package(); + const absl::string_view package = base_descriptor->file()->package(); map_enum_foo_ = pool->FindEnumValueByName(absl::StrCat(package, ".MAP_ENUM_FOO")); diff --git a/src/google/protobuf/test_util.h b/src/google/protobuf/test_util.h index c52c38fc4589b..338ba5a92a14a 100644 --- a/src/google/protobuf/test_util.h +++ b/src/google/protobuf/test_util.h @@ -12,6 +12,7 @@ #ifndef GOOGLE_PROTOBUF_TEST_UTIL_H__ #define GOOGLE_PROTOBUF_TEST_UTIL_H__ +#include "absl/strings/string_view.h" #include "google/protobuf/unittest.pb.h" #define UNITTEST ::protobuf_unittest @@ -105,10 +106,10 @@ inline TestUtil::ReflectionTester::ReflectionTester( const Descriptor* base_descriptor) : base_descriptor_(base_descriptor) { const DescriptorPool* pool = base_descriptor->file()->pool(); - std::string package = base_descriptor->file()->package(); + const absl::string_view package = base_descriptor->file()->package(); const FieldDescriptor* import_descriptor = pool->FindFieldByName( absl::StrCat(package, ".TestAllTypes.optional_import_message")); - std::string import_package = + const absl::string_view import_package = import_descriptor->message_type()->file()->package(); nested_b_ = pool->FindFieldByName( diff --git a/src/google/protobuf/text_format.cc b/src/google/protobuf/text_format.cc index e91d26b91dae4..1ed6e575bcc9e 100644 --- a/src/google/protobuf/text_format.cc +++ b/src/google/protobuf/text_format.cc @@ -2803,7 +2803,8 @@ void TextFormat::Printer::PrintFieldValue(const Message& message, const EnumValueDescriptor* enum_desc = field->enum_type()->FindValueByNumber(enum_value); if (enum_desc != nullptr) { - printer->PrintEnum(enum_value, enum_desc->name(), generator); + printer->PrintEnum(enum_value, internal::NameOfEnumAsString(enum_desc), + generator); } else { // Ordinarily, enum_desc should not be null, because proto2 has the // invariant that set enum field values must be in-range, but with the @@ -2923,7 +2924,7 @@ void TextFormat::Printer::PrintUnknownFields( } case UnknownField::TYPE_LENGTH_DELIMITED: { OutOfLinePrintString(generator, field.number()); - const std::string& value = field.length_delimited(); + const absl::string_view value = field.length_delimited(); // We create a CodedInputStream so that we can adhere to our recursion // budget when we attempt to parse the data. UnknownFieldSet parsing is // recursive because of groups. diff --git a/src/google/protobuf/unknown_field_set.h b/src/google/protobuf/unknown_field_set.h index ea66917724337..e985b0f4d460f 100644 --- a/src/google/protobuf/unknown_field_set.h +++ b/src/google/protobuf/unknown_field_set.h @@ -17,11 +17,13 @@ #include +#include #include #include #include "google/protobuf/stubs/common.h" #include "absl/log/absl_check.h" +#include "absl/strings/cord.h" #include "absl/strings/string_view.h" #include "google/protobuf/io/coded_stream.h" #include "google/protobuf/io/zero_copy_stream_impl_lite.h" @@ -43,6 +45,12 @@ class InternalMetadata; // metadata_lite.h class WireFormat; // wire_format.h class MessageSetFieldSkipperUsingCord; // extension_set_heavy.cc + +#if defined(PROTOBUF_FUTURE_STRING_VIEW_RETURN_TYPE) +using UFSStringView = absl::string_view; +#else +using UFSStringView = const std::string&; +#endif } // namespace internal class Message; // message.h @@ -233,7 +241,7 @@ class PROTOBUF_EXPORT UnknownField { inline uint64_t varint() const; inline uint32_t fixed32() const; inline uint64_t fixed64() const; - inline const std::string& length_delimited() const; + inline internal::UFSStringView length_delimited() const; inline const UnknownFieldSet& group() const; inline void set_varint(uint64_t value); @@ -329,7 +337,7 @@ inline uint64_t UnknownField::fixed64() const { assert(type() == TYPE_FIXED64); return data_.fixed64_; } -inline const std::string& UnknownField::length_delimited() const { +inline internal::UFSStringView UnknownField::length_delimited() const { assert(type() == TYPE_LENGTH_DELIMITED); return *data_.length_delimited_.string_value; } diff --git a/src/google/protobuf/util/message_differencer.cc b/src/google/protobuf/util/message_differencer.cc index 258c0e9bec337..7a4141cfedfd4 100644 --- a/src/google/protobuf/util/message_differencer.cc +++ b/src/google/protobuf/util/message_differencer.cc @@ -898,7 +898,7 @@ bool MessageDifferencer::CompareWithFieldsInternal( const bool ignore_field = IsIgnored(message1, message2, field2, *parent_fields); if (!ignore_field && force_compare_no_presence_fields_.contains(field2)) { - force_compare_failure_triggering_fields_.insert(field2->full_name()); + force_compare_failure_triggering_fields_.emplace(field2->full_name()); } // Field 2 is not in the field list for message 1. @@ -990,7 +990,7 @@ bool MessageDifferencer::CompareWithFieldsInternal( message1, message2, unpacked_any, field1, -1, -1, parent_fields); if (force_compare_no_presence_fields_.contains(field1)) { - force_compare_failure_triggering_fields_.insert(field1->full_name()); + force_compare_failure_triggering_fields_.emplace(field1->full_name()); } if (reporter_ != nullptr) { @@ -2382,3 +2382,5 @@ MessageDifferencer::CreateMultipleFieldsMapKeyComparator( } // namespace util } // namespace protobuf } // namespace google + +#include "google/protobuf/port_undef.inc" diff --git a/src/google/protobuf/util/type_resolver_util.cc b/src/google/protobuf/util/type_resolver_util.cc index b7a2f064e092f..694abed81542f 100644 --- a/src/google/protobuf/util/type_resolver_util.cc +++ b/src/google/protobuf/util/type_resolver_util.cc @@ -197,11 +197,11 @@ std::string DefaultValueAsString(const FieldDescriptor& descriptor) { if (descriptor.type() == FieldDescriptor::TYPE_BYTES) { return absl::CEscape(descriptor.default_value_string()); } else { - return descriptor.default_value_string(); + return std::string(descriptor.default_value_string()); } break; case FieldDescriptor::CPPTYPE_ENUM: - return descriptor.default_value_enum()->name(); + return std::string(descriptor.default_value_enum()->name()); break; case FieldDescriptor::CPPTYPE_MESSAGE: ABSL_DLOG(FATAL) << "Messages can't have default values!"; diff --git a/src/google/protobuf/wire_format.cc b/src/google/protobuf/wire_format.cc index 99db63f76e75e..4d7fb6707c040 100644 --- a/src/google/protobuf/wire_format.cc +++ b/src/google/protobuf/wire_format.cc @@ -544,12 +544,12 @@ bool WireFormat::ParseAndMergeField( if (strict_utf8_check) { if (!WireFormatLite::VerifyUtf8String(value.data(), value.length(), WireFormatLite::PARSE, - field->full_name().c_str())) { + field->full_name())) { return false; } } else { VerifyUTF8StringNamedField(value.data(), value.length(), PARSE, - field->full_name().c_str()); + field->full_name()); } if (field->is_repeated()) { message_reflection->AddString(message, field, value); @@ -1017,12 +1017,12 @@ const char* WireFormat::_InternalParseAndMergeField( if (strict_utf8_check) { if (!WireFormatLite::VerifyUtf8String(value.data(), value.length(), WireFormatLite::PARSE, - field->full_name().c_str())) { + field->full_name())) { return nullptr; } } else { VerifyUTF8StringNamedField(value.data(), value.length(), PARSE, - field->full_name().c_str()); + field->full_name()); } } if (field->is_repeated()) { @@ -1415,10 +1415,10 @@ uint8_t* WireFormat::InternalSerializeField(const FieldDescriptor* field, if (strict_utf8_check) { WireFormatLite::VerifyUtf8String(value.data(), value.length(), WireFormatLite::SERIALIZE, - field->full_name().c_str()); + field->full_name()); } else { VerifyUTF8StringNamedField(value.data(), value.length(), SERIALIZE, - field->full_name().c_str()); + field->full_name()); } target = stream->WriteString(field->number(), value, target); break; diff --git a/src/google/protobuf/wire_format.h b/src/google/protobuf/wire_format.h index 200a35f0efe6f..fd3be6d575c7d 100644 --- a/src/google/protobuf/wire_format.h +++ b/src/google/protobuf/wire_format.h @@ -260,7 +260,8 @@ class PROTOBUF_EXPORT WireFormat { // The NamedField variant takes a field name in order to produce an // informative error message if verification fails. static void VerifyUTF8StringNamedField(const char* data, int size, - Operation op, const char* field_name); + Operation op, + absl::string_view field_name); private: struct MessageSetParser; @@ -345,9 +346,9 @@ inline void WireFormat::VerifyUTF8String(const char* data, int size, #endif } -inline void WireFormat::VerifyUTF8StringNamedField(const char* data, int size, - WireFormat::Operation op, - const char* field_name) { +inline void WireFormat::VerifyUTF8StringNamedField( + const char* data, int size, WireFormat::Operation op, + const absl::string_view field_name) { #ifdef GOOGLE_PROTOBUF_UTF8_VALIDATION_ENABLED WireFormatLite::VerifyUtf8String( data, size, static_cast(op), field_name); diff --git a/src/google/protobuf/wire_format_lite.cc b/src/google/protobuf/wire_format_lite.cc index f6d97df6c0c65..8fc29af555a45 100644 --- a/src/google/protobuf/wire_format_lite.cc +++ b/src/google/protobuf/wire_format_lite.cc @@ -604,7 +604,7 @@ void PrintUTF8ErrorLog(absl::string_view message_name, } bool WireFormatLite::VerifyUtf8String(const char* data, int size, Operation op, - const char* field_name) { + const absl::string_view field_name) { if (!utf8_range::IsStructurallyValid({data, static_cast(size)})) { const char* operation_str = nullptr; switch (op) { diff --git a/src/google/protobuf/wire_format_lite.h b/src/google/protobuf/wire_format_lite.h index 872c5ff02b334..ad0167c8504fc 100644 --- a/src/google/protobuf/wire_format_lite.h +++ b/src/google/protobuf/wire_format_lite.h @@ -315,7 +315,7 @@ class PROTOBUF_EXPORT WireFormatLite { // Returns true if the data is valid UTF-8. static bool VerifyUtf8String(const char* data, int size, Operation op, - const char* field_name); + absl::string_view field_name); template static inline bool ReadGroup(int field_number, io::CodedInputStream* input,