From 278fb1b4a3c960b965ec60feeb99ce34a0c7a227 Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Wed, 27 Mar 2024 10:32:47 -0700 Subject: [PATCH] Merge two functions that calculate the effective string field type and migrate all callers to the new one. The one removed was not taking into account edition features. PiperOrigin-RevId: 619577375 --- src/google/protobuf/compiler/cpp/field.cc | 37 +-- .../cpp/field_generators/string_field.cc | 12 +- .../cpp/field_generators/string_view_field.cc | 11 - src/google/protobuf/compiler/cpp/helpers.cc | 16 +- src/google/protobuf/compiler/cpp/helpers.h | 13 +- src/google/protobuf/compiler/cpp/message.cc | 7 +- src/google/protobuf/descriptor.cc | 40 +++ src/google/protobuf/descriptor.h | 24 +- src/google/protobuf/dynamic_message.cc | 42 ++- .../protobuf/generated_message_reflection.cc | 252 ++++++++++++------ .../protobuf/generated_message_tctable_gen.cc | 41 +-- src/google/protobuf/message.cc | 7 +- src/google/protobuf/message.h | 63 +++-- src/google/protobuf/no_field_presence_test.cc | 5 +- .../protobuf/reflection_visit_field_info.h | 5 +- src/google/protobuf/reflection_visit_fields.h | 21 +- src/google/protobuf/wire_format.cc | 10 +- 17 files changed, 363 insertions(+), 243 deletions(-) diff --git a/src/google/protobuf/compiler/cpp/field.cc b/src/google/protobuf/compiler/cpp/field.cc index c333a5818b3b..3141d99f0731 100644 --- a/src/google/protobuf/compiler/cpp/field.cc +++ b/src/google/protobuf/compiler/cpp/field.cc @@ -40,6 +40,8 @@ namespace protobuf { namespace compiler { namespace cpp { using ::google::protobuf::internal::WireFormat; +using ::google::protobuf::internal::cpp::GetStringType; +using ::google::protobuf::internal::cpp::StringType; using Sub = ::google::protobuf::io::Printer::Sub; std::vector FieldVars(const FieldDescriptor* field, const Options& opts) { @@ -228,39 +230,6 @@ void FieldGeneratorBase::GenerateCopyConstructorCode(io::Printer* p) const { } namespace { -// Use internal types instead of ctype or string_type. -enum class StringType { - kView, - kString, - kCord, - kStringPiece, -}; - -StringType GetStringType(const FieldDescriptor& field) { - ABSL_CHECK_EQ(field.cpp_type(), FieldDescriptor::CPPTYPE_STRING); - - if (field.options().has_ctype()) { - switch (field.options().ctype()) { - case FieldOptions::CORD: - return StringType::kCord; - case FieldOptions::STRING_PIECE: - return StringType::kStringPiece; - default: - return StringType::kString; - } - } - - const pb::CppFeatures& cpp_features = - CppGenerator::GetResolvedSourceFeatures(field).GetExtension(::pb::cpp); - switch (cpp_features.string_type()) { - case pb::CppFeatures::CORD: - return StringType::kCord; - case pb::CppFeatures::VIEW: - return StringType::kView; - default: - return StringType::kString; - } -} std::unique_ptr MakeGenerator(const FieldDescriptor* field, const Options& options, @@ -278,7 +247,7 @@ std::unique_ptr MakeGenerator(const FieldDescriptor* field, case FieldDescriptor::CPPTYPE_MESSAGE: return MakeRepeatedMessageGenerator(field, options, scc); case FieldDescriptor::CPPTYPE_STRING: { - if (GetStringType(*field) == StringType::kView) { + if (internal::cpp::StringTypeIsStringView(*field)) { return MakeRepeatedStringViewGenerator(field, options, scc); } else { return MakeRepeatedStringGenerator(field, options, scc); diff --git a/src/google/protobuf/compiler/cpp/field_generators/string_field.cc b/src/google/protobuf/compiler/cpp/field_generators/string_field.cc index bf277f4716b7..ce6291c47db4 100644 --- a/src/google/protobuf/compiler/cpp/field_generators/string_field.cc +++ b/src/google/protobuf/compiler/cpp/field_generators/string_field.cc @@ -235,12 +235,9 @@ void SingularString::GenerateAccessorDeclarations(io::Printer* p) const { // files that applied the ctype. The field can still be accessed via the // reflection interface since the reflection interface is independent of // the string's underlying representation. - bool unknown_ctype = - field_->options().ctype() != internal::cpp::EffectiveStringCType(field_); - - if (unknown_ctype) { + if (!internal::cpp::StringTypeIsSupported(*field_)) { p->Emit(R"cc( - private: // Hidden due to unknown ctype option. + private: // Hidden due to unsupported option. )cc"); } @@ -822,10 +819,7 @@ class RepeatedString : public FieldGeneratorBase { }; void RepeatedString::GenerateAccessorDeclarations(io::Printer* p) const { - bool unknown_ctype = - field_->options().ctype() != internal::cpp::EffectiveStringCType(field_); - - if (unknown_ctype) { + if (!internal::cpp::StringTypeIsSupported(*field_)) { p->Emit(R"cc( private: // Hidden due to unknown ctype option. )cc"); diff --git a/src/google/protobuf/compiler/cpp/field_generators/string_view_field.cc b/src/google/protobuf/compiler/cpp/field_generators/string_view_field.cc index 6e73c0f5a37b..8dde52aa053e 100644 --- a/src/google/protobuf/compiler/cpp/field_generators/string_view_field.cc +++ b/src/google/protobuf/compiler/cpp/field_generators/string_view_field.cc @@ -216,8 +216,6 @@ void SingularStringView::GenerateStaticMembers(io::Printer* p) const { } void SingularStringView::GenerateAccessorDeclarations(io::Printer* p) const { - ABSL_CHECK(!field_->options().has_ctype()); - auto vars = AnnotatedAccessors(field_, {"", "set_allocated_"}); vars.push_back(Sub{ "release_name", @@ -648,15 +646,6 @@ class RepeatedStringView : public FieldGeneratorBase { }; void RepeatedStringView::GenerateAccessorDeclarations(io::Printer* p) const { - bool unknown_ctype = - field_->options().ctype() != internal::cpp::EffectiveStringCType(field_); - - if (unknown_ctype) { - p->Emit(R"cc( - private: // Hidden due to unknown ctype option. - )cc"); - } - auto v1 = p->WithVars(AnnotatedAccessors(field_, {"", "_internal_"})); auto v2 = p->WithVars( AnnotatedAccessors(field_, {"set_", "add_"}, AnnotationCollector::kSet)); diff --git a/src/google/protobuf/compiler/cpp/helpers.cc b/src/google/protobuf/compiler/cpp/helpers.cc index 047a52709ea3..b16ba85b5996 100644 --- a/src/google/protobuf/compiler/cpp/helpers.cc +++ b/src/google/protobuf/compiler/cpp/helpers.cc @@ -1071,17 +1071,10 @@ bool HasRepeatedFields(const FileDescriptor* file) { return false; } -static bool IsStringPieceField(const FieldDescriptor* field, - const Options& options) { - return field->cpp_type() == FieldDescriptor::CPPTYPE_STRING && - internal::cpp::EffectiveStringCType(field) == - FieldOptions::STRING_PIECE; -} - static bool HasStringPieceFields(const Descriptor* descriptor, const Options& options) { for (int i = 0; i < descriptor->field_count(); ++i) { - if (IsStringPieceField(descriptor->field(i), options)) return true; + if (IsStringPiece(descriptor->field(i))) return true; } for (int i = 0; i < descriptor->nested_type_count(); ++i) { if (HasStringPieceFields(descriptor->nested_type(i), options)) return true; @@ -1096,15 +1089,10 @@ bool HasStringPieceFields(const FileDescriptor* file, const Options& options) { return false; } -static bool IsCordField(const FieldDescriptor* field, const Options& options) { - return field->cpp_type() == FieldDescriptor::CPPTYPE_STRING && - internal::cpp::EffectiveStringCType(field) == FieldOptions::CORD; -} - static bool HasCordFields(const Descriptor* descriptor, const Options& options) { for (int i = 0; i < descriptor->field_count(); ++i) { - if (IsCordField(descriptor->field(i), options)) return true; + if (IsCord(descriptor->field(i))) return true; } for (int i = 0; i < descriptor->nested_type_count(); ++i) { if (HasCordFields(descriptor->nested_type(i), options)) return true; diff --git a/src/google/protobuf/compiler/cpp/helpers.h b/src/google/protobuf/compiler/cpp/helpers.h index 1c58b9ccb384..8d847bf1bc8c 100644 --- a/src/google/protobuf/compiler/cpp/helpers.h +++ b/src/google/protobuf/compiler/cpp/helpers.h @@ -330,18 +330,23 @@ inline bool IsWeak(const FieldDescriptor* field, const Options& options) { inline bool IsCord(const FieldDescriptor* field) { return field->cpp_type() == FieldDescriptor::CPPTYPE_STRING && - internal::cpp::EffectiveStringCType(field) == FieldOptions::CORD; + internal::cpp::StringTypeIsCord(*field); } inline bool IsString(const FieldDescriptor* field) { return field->cpp_type() == FieldDescriptor::CPPTYPE_STRING && - internal::cpp::EffectiveStringCType(field) == FieldOptions::STRING; + internal::cpp::StringTypeIsStdString(*field); +} + +inline bool IsStringView(const FieldDescriptor* field) { + return field->cpp_type() == FieldDescriptor::CPPTYPE_STRING && + internal::cpp::StringTypeIsStringView(*field); } inline bool IsStringPiece(const FieldDescriptor* field) { return field->cpp_type() == FieldDescriptor::CPPTYPE_STRING && - internal::cpp::EffectiveStringCType(field) == - FieldOptions::STRING_PIECE; + internal::cpp::GetStringType(*field) == + internal::cpp::StringType::kStringPiece; } bool IsProfileDriven(const Options& options); diff --git a/src/google/protobuf/compiler/cpp/message.cc b/src/google/protobuf/compiler/cpp/message.cc index 647f804d2043..ed5e4e78a806 100644 --- a/src/google/protobuf/compiler/cpp/message.cc +++ b/src/google/protobuf/compiler/cpp/message.cc @@ -60,7 +60,9 @@ namespace cpp { namespace { using ::google::protobuf::internal::WireFormat; using ::google::protobuf::internal::WireFormatLite; +using ::google::protobuf::internal::cpp::GetStringType; using ::google::protobuf::internal::cpp::HasHasbit; +using ::google::protobuf::internal::cpp::StringType; using Semantic = ::google::protobuf::io::AnnotationCollector::Semantic; using Sub = ::google::protobuf::io::Printer::Sub; @@ -302,8 +304,9 @@ bool IsCrossFileMaybeMap(const FieldDescriptor* field) { bool HasNonSplitOptionalString(const Descriptor* desc, const Options& options) { for (const auto* field : FieldRange(desc)) { - if (IsString(field) && !field->is_repeated() && - !field->real_containing_oneof() && !ShouldSplit(field, options)) { + if (!field->is_repeated() && !field->real_containing_oneof() && + (IsString(field) || IsStringView(field)) && + !ShouldSplit(field, options)) { return true; } } diff --git a/src/google/protobuf/descriptor.cc b/src/google/protobuf/descriptor.cc index db45ca90ca24..88d37d3abba9 100644 --- a/src/google/protobuf/descriptor.cc +++ b/src/google/protobuf/descriptor.cc @@ -9747,6 +9747,46 @@ bool IsLazilyInitializedFile(absl::string_view filename) { filename == "google/protobuf/descriptor.proto"; } +StringType GetStringType(const FieldDescriptor& field, bool should_normalize) { + ABSL_CHECK_EQ(field.cpp_type(), FieldDescriptor::CPPTYPE_STRING); + + const pb::CppFeatures& cpp_features = + InternalFeatureHelper::GetFeatures(field).GetExtension(::pb::cpp); + switch (cpp_features.string_type()) { + case pb::CppFeatures::CORD: + ABSL_DCHECK(!field.options().has_ctype() || + field.options().ctype() == FieldOptions::CORD); + if (should_normalize && (field.type() == FieldDescriptor::TYPE_STRING || + field.is_repeated() || field.is_extension())) { + return StringType::kString; + } + return StringType::kCord; + case pb::CppFeatures::STRING_PIECE: + ABSL_DCHECK(!field.options().has_ctype() || + field.options().ctype() == FieldOptions::STRING_PIECE); + if (should_normalize) { + return StringType::kString; + } + return StringType::kStringPiece; + case pb::CppFeatures::VIEW: + return StringType::kView; + default: + return StringType::kString; + } +} + +bool StringTypeIsStdString(const FieldDescriptor& field) { + return GetStringType(field) == StringType::kString; +} + +bool StringTypeIsStringView(const FieldDescriptor& field) { + return GetStringType(field) == StringType::kView; +} + +bool StringTypeIsCord(const FieldDescriptor& field) { + return GetStringType(field) == StringType::kCord; +} + } // namespace cpp } // namespace internal diff --git a/src/google/protobuf/descriptor.h b/src/google/protobuf/descriptor.h index 443454543d11..cea54c8e399d 100644 --- a/src/google/protobuf/descriptor.h +++ b/src/google/protobuf/descriptor.h @@ -2880,22 +2880,18 @@ PROTOBUF_EXPORT bool HasPreservingUnknownEnumSemantics( PROTOBUF_EXPORT bool HasHasbit(const FieldDescriptor* field); -// For a string field, returns the effective ctype. If the actual ctype is -// not supported, returns the default of STRING. -template -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; - } - return FieldOpts::STRING; +#ifndef SWIG +enum class StringType : uint8_t; +PROTOBUF_EXPORT StringType GetStringType(const FieldDescriptor& field, + bool should_normalize = true); +PROTOBUF_EXPORT inline bool StringTypeIsSupported( + const FieldDescriptor& field) { + return GetStringType(field, true) == GetStringType(field, false); } +PROTOBUF_EXPORT bool StringTypeIsStdString(const FieldDescriptor& field); +PROTOBUF_EXPORT bool StringTypeIsStringView(const FieldDescriptor& field); +PROTOBUF_EXPORT bool StringTypeIsCord(const FieldDescriptor& field); -#ifndef SWIG enum class Utf8CheckMode { kStrict = 0, // Parsing will fail if non UTF-8 data is in string fields. kVerify = 1, // Only log an error but parsing will succeed. diff --git a/src/google/protobuf/dynamic_message.cc b/src/google/protobuf/dynamic_message.cc index 792710781513..e444c71472c3 100644 --- a/src/google/protobuf/dynamic_message.cc +++ b/src/google/protobuf/dynamic_message.cc @@ -117,9 +117,12 @@ int FieldSpaceUsed(const FieldDescriptor* field) { } case FD::CPPTYPE_STRING: - switch (field->options().ctype()) { + switch (internal::cpp::GetStringType(*field)) { default: // TODO: Support other string reps. - case FieldOptions::STRING: + case internal::cpp::StringType::kView: + // Currently VIEW has the same ABI than string, so this fallback is + // fine. + case internal::cpp::StringType::kString: return sizeof(RepeatedPtrField); } break; @@ -147,9 +150,12 @@ int FieldSpaceUsed(const FieldDescriptor* field) { return sizeof(Message*); case FD::CPPTYPE_STRING: - switch (field->options().ctype()) { + switch (internal::cpp::GetStringType(*field)) { default: // TODO: Support other string reps. - case FieldOptions::STRING: + case internal::cpp::StringType::kView: + // Currently VIEW has the same ABI than string, so this fallback is + // fine. + case internal::cpp::StringType::kString: return sizeof(ArenaStringPtr); } break; @@ -401,9 +407,12 @@ void DynamicMessage::SharedCtor(bool lock_factory) { break; case FieldDescriptor::CPPTYPE_STRING: - switch (field->options().ctype()) { + switch (internal::cpp::GetStringType(*field)) { default: // TODO: Support other string reps. - case FieldOptions::STRING: + case internal::cpp::StringType::kView: + // Currently VIEW has the same ABI than string, so this fallback is + // fine. + case internal::cpp::StringType::kString: if (!field->is_repeated()) { ArenaStringPtr* asp = new (field_ptr) ArenaStringPtr(); asp->InitDefault(); @@ -493,9 +502,12 @@ DynamicMessage::~DynamicMessage() { if (*(reinterpret_cast(field_ptr)) == field->number()) { field_ptr = MutableOneofFieldRaw(field); if (field->cpp_type() == FieldDescriptor::CPPTYPE_STRING) { - switch (field->options().ctype()) { + switch (internal::cpp::GetStringType(*field)) { default: - case FieldOptions::STRING: { + case internal::cpp::StringType::kView: + // Currently VIEW has the same ABI than string, so this fallback is + // fine. + case internal::cpp::StringType::kString: { reinterpret_cast(field_ptr)->Destroy(); break; } @@ -527,9 +539,12 @@ DynamicMessage::~DynamicMessage() { #undef HANDLE_TYPE case FieldDescriptor::CPPTYPE_STRING: - switch (field->options().ctype()) { + switch (internal::cpp::GetStringType(*field)) { default: // TODO: Support other string reps. - case FieldOptions::STRING: + case internal::cpp::StringType::kView: + // Currently VIEW has the same ABI than string, so this fallback is + // fine. + case internal::cpp::StringType::kString: reinterpret_cast*>(field_ptr) ->~RepeatedPtrField(); break; @@ -547,9 +562,12 @@ DynamicMessage::~DynamicMessage() { } } else if (field->cpp_type() == FieldDescriptor::CPPTYPE_STRING) { - switch (field->options().ctype()) { + switch (internal::cpp::GetStringType(*field)) { default: // TODO: Support other string reps. - case FieldOptions::STRING: { + case internal::cpp::StringType::kView: + // Currently VIEW has the same ABI than string, so this fallback is + // fine. + case internal::cpp::StringType::kString: { reinterpret_cast(field_ptr)->Destroy(); break; } diff --git a/src/google/protobuf/generated_message_reflection.cc b/src/google/protobuf/generated_message_reflection.cc index 4d7d7481057b..50b44ffe43c5 100644 --- a/src/google/protobuf/generated_message_reflection.cc +++ b/src/google/protobuf/generated_message_reflection.cc @@ -31,6 +31,7 @@ #include "absl/strings/str_format.h" #include "absl/strings/string_view.h" #include "absl/synchronization/mutex.h" +#include "absl/types/optional.h" #include "google/protobuf/descriptor.h" #include "google/protobuf/descriptor.pb.h" #include "google/protobuf/extension_set.h" @@ -402,9 +403,12 @@ size_t Reflection::SpaceUsedLong(const Message& message) const { #undef HANDLE_TYPE case FieldDescriptor::CPPTYPE_STRING: - switch (field->options().ctype()) { + switch (internal::cpp::GetStringType(*field)) { default: // TODO: Support other string reps. - case FieldOptions::STRING: + case internal::cpp::StringType::kView: + // Currently VIEW has the same ABI than string, so this fallback + // is fine. + case internal::cpp::StringType::kString: total_size += GetRaw >(message, field) .SpaceUsedExcludingSelfLong(); @@ -443,8 +447,8 @@ size_t Reflection::SpaceUsedLong(const Message& message) const { break; case FieldDescriptor::CPPTYPE_STRING: { - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: + switch (internal::cpp::GetStringType(*field)) { + case internal::cpp::StringType::kCord: if (schema_.InRealOneof(field)) { total_size += GetField(message, field) ->EstimatedMemoryUsage(); @@ -457,7 +461,10 @@ size_t Reflection::SpaceUsedLong(const Message& message) const { } break; default: - case FieldOptions::STRING: + case internal::cpp::StringType::kView: + // Currently VIEW has the same ABI than string, so this fallback + // is fine. + case internal::cpp::StringType::kString: if (IsInlined(field)) { const std::string* ptr = &GetField(message, field).GetNoArena(); @@ -553,12 +560,15 @@ struct OneofFieldMover { to->SetString(from->GetString()); break; } - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: + switch (internal::cpp::GetStringType(*field)) { + case internal::cpp::StringType::kCord: to->SetCord(from->GetCord()); break; default: - case FieldOptions::STRING: { + case internal::cpp::StringType::kView: + // Currently VIEW has the same ABI than string, so this fallback is + // fine. + case internal::cpp::StringType::kString: { to->SetArenaStringPtr(from->GetArenaStringPtr()); break; } @@ -624,9 +634,12 @@ template void SwapFieldHelper::SwapRepeatedStringField(const Reflection* r, Message* lhs, Message* rhs, const FieldDescriptor* field) { - switch (field->options().ctype()) { + switch (internal::cpp::GetStringType(*field)) { default: - case FieldOptions::STRING: { + case internal::cpp::StringType::kView: + // Currently VIEW has the same ABI than string, so this fallback + // is fine. + case internal::cpp::StringType::kString: { auto* lhs_string = r->MutableRaw(lhs, field); auto* rhs_string = r->MutableRaw(rhs, field); if (unsafe_shallow_swap) { @@ -690,14 +703,17 @@ template void SwapFieldHelper::SwapStringField(const Reflection* r, Message* lhs, Message* rhs, const FieldDescriptor* field) { - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: + switch (internal::cpp::GetStringType(*field)) { + case internal::cpp::StringType::kCord: // Always shallow swap for Cord. std::swap(*r->MutableRaw(lhs, field), *r->MutableRaw(rhs, field)); break; default: - case FieldOptions::STRING: { + case internal::cpp::StringType::kView: + // Currently VIEW has the same ABI than string, so this fallback + // is fine. + case internal::cpp::StringType::kString: { if (r->IsInlined(field)) { SwapFieldHelper::SwapInlinedStrings(r, lhs, rhs, field); @@ -1169,7 +1185,8 @@ void Reflection::SwapFieldsImpl( // may depend on the information in has bits. if (!field->is_repeated()) { SwapBit(message1, message2, field); - if (field->options().ctype() == FieldOptions::STRING && + if (field->cpp_type() == FieldDescriptor::CPPTYPE_STRING && + internal::cpp::StringTypeIsStdString(*field) && IsInlined(field)) { ABSL_DCHECK(!unsafe_shallow_swap || message1->GetArena() == message2->GetArena()); @@ -1286,7 +1303,8 @@ void Reflection::InternalSwap(Message* lhs, Message* rhs) const { const FieldDescriptor* field = descriptor_->field(i); if (field->is_extension() || field->is_repeated() || schema_.InRealOneof(field) || - field->options().ctype() != FieldOptions::STRING || + (field->cpp_type() == FieldDescriptor::CPPTYPE_STRING && + !internal::cpp::StringTypeIsStdString(*field)) || !IsInlined(field)) { continue; } @@ -1396,8 +1414,8 @@ void Reflection::ClearField(Message* message, break; case FieldDescriptor::CPPTYPE_STRING: { - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: + switch (internal::cpp::GetStringType(*field)) { + case internal::cpp::StringType::kCord: if (field->has_default_value()) { *MutableRaw(message, field) = field->default_value_string(); @@ -1406,7 +1424,10 @@ void Reflection::ClearField(Message* message, } break; default: - case FieldOptions::STRING: + case internal::cpp::StringType::kView: + // Currently VIEW has the same ABI than string, so this fallback + // is fine. + case internal::cpp::StringType::kString: if (IsInlined(field)) { // Currently, string with default value can't be inlined. So we // don't have to handle default value here. @@ -1453,9 +1474,12 @@ void Reflection::ClearField(Message* message, #undef HANDLE_TYPE case FieldDescriptor::CPPTYPE_STRING: { - switch (field->options().ctype()) { + switch (internal::cpp::GetStringType(*field)) { default: // TODO: Support other string reps. - case FieldOptions::STRING: + case internal::cpp::StringType::kView: + // Currently VIEW has the same ABI than string, so this fallback + // is fine. + case internal::cpp::StringType::kString: MutableRaw >(message, field)->Clear(); break; } @@ -1503,9 +1527,12 @@ void Reflection::RemoveLast(Message* message, #undef HANDLE_TYPE case FieldDescriptor::CPPTYPE_STRING: - switch (field->options().ctype()) { + switch (internal::cpp::GetStringType(*field)) { default: // TODO: Support other string reps. - case FieldOptions::STRING: + case internal::cpp::StringType::kView: + // Currently VIEW has the same ABI than string, so this fallback + // is fine. + case internal::cpp::StringType::kString: MutableRaw >(message, field) ->RemoveLast(); break; @@ -1803,15 +1830,18 @@ std::string Reflection::GetString(const Message& message, if (schema_.InRealOneof(field) && !HasOneofField(message, field)) { return field->default_value_string(); } - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: + switch (internal::cpp::GetStringType(*field)) { + case internal::cpp::StringType::kCord: if (schema_.InRealOneof(field)) { return std::string(*GetField(message, field)); } else { return std::string(GetField(message, field)); } default: - case FieldOptions::STRING: + case internal::cpp::StringType::kView: + // Currently VIEW has the same ABI than string, so this fallback + // is fine. + case internal::cpp::StringType::kString: if (IsInlined(field)) { return GetField(message, field).GetNoArena(); } else { @@ -1834,8 +1864,8 @@ const std::string& Reflection::GetStringReference(const Message& message, if (schema_.InRealOneof(field) && !HasOneofField(message, field)) { return field->default_value_string(); } - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: + switch (internal::cpp::GetStringType(*field)) { + case internal::cpp::StringType::kCord: if (schema_.InRealOneof(field)) { absl::CopyCordToString(*GetField(message, field), scratch); @@ -1844,7 +1874,10 @@ const std::string& Reflection::GetStringReference(const Message& message, } return *scratch; default: - case FieldOptions::STRING: + case internal::cpp::StringType::kView: + // Currently VIEW has the same ABI than string, so this fallback + // is fine. + case internal::cpp::StringType::kString: if (IsInlined(field)) { return GetField(message, field).GetNoArena(); } else { @@ -1865,15 +1898,18 @@ absl::Cord Reflection::GetCord(const Message& message, if (schema_.InRealOneof(field) && !HasOneofField(message, field)) { return absl::Cord(field->default_value_string()); } - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: + switch (internal::cpp::GetStringType(*field)) { + case internal::cpp::StringType::kCord: if (schema_.InRealOneof(field)) { return *GetField(message, field); } else { return GetField(message, field); } default: - case FieldOptions::STRING: + case internal::cpp::StringType::kView: + // Currently VIEW has the same ABI than string, so this fallback + // is fine. + case internal::cpp::StringType::kString: if (IsInlined(field)) { return absl::Cord( GetField(message, field).GetNoArena()); @@ -1902,8 +1938,8 @@ absl::string_view Reflection::GetStringView(const Message& message, return field->default_value_string(); } - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: { + switch (internal::cpp::GetStringType(*field)) { + case internal::cpp::StringType::kCord: { const auto& cord = schema_.InRealOneof(field) ? *GetField(message, field) : GetField(message, field); @@ -1923,8 +1959,8 @@ void Reflection::SetString(Message* message, const FieldDescriptor* field, return MutableExtensionSet(message)->SetString( field->number(), field->type(), std::move(value), field); } else { - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: + switch (internal::cpp::GetStringType(*field)) { + case internal::cpp::StringType::kCord: if (schema_.InRealOneof(field)) { if (!HasOneofField(*message, field)) { ClearOneof(message, field->containing_oneof()); @@ -1937,7 +1973,10 @@ void Reflection::SetString(Message* message, const FieldDescriptor* field, *MutableField(message, field) = value; break; default: - case FieldOptions::STRING: { + case internal::cpp::StringType::kView: + // Currently VIEW has the same ABI than string, so this fallback is + // fine. + case internal::cpp::StringType::kString: { if (IsInlined(field)) { const uint32_t index = schema_.InlinedStringIndex(field); ABSL_DCHECK_GT(index, 0u); @@ -1975,8 +2014,8 @@ void Reflection::SetString(Message* message, const FieldDescriptor* field, MutableExtensionSet(message)->MutableString( field->number(), field->type(), field)); } else { - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: + switch (internal::cpp::GetStringType(*field)) { + case internal::cpp::StringType::kCord: if (schema_.InRealOneof(field)) { if (!HasOneofField(*message, field)) { ClearOneof(message, field->containing_oneof()); @@ -1989,7 +2028,10 @@ void Reflection::SetString(Message* message, const FieldDescriptor* field, } break; default: - case FieldOptions::STRING: { + case internal::cpp::StringType::kView: + // Currently VIEW has the same ABI than string, so this fallback + // is fine. + case internal::cpp::StringType::kString: { // Oneof string fields are never set as a default instance. // We just need to pass some arbitrary default string to make it work. // This allows us to not have the real default accessible from @@ -2025,9 +2067,12 @@ std::string Reflection::GetRepeatedString(const Message& message, if (field->is_extension()) { return GetExtensionSet(message).GetRepeatedString(field->number(), index); } else { - switch (field->options().ctype()) { + switch (internal::cpp::GetStringType(*field)) { default: // TODO: Support other string reps. - case FieldOptions::STRING: + case internal::cpp::StringType::kView: + // Currently VIEW has the same ABI than string, so this fallback + // is fine. + case internal::cpp::StringType::kString: return GetRepeatedPtrField(message, field, index); } } @@ -2041,9 +2086,12 @@ const std::string& Reflection::GetRepeatedStringReference( if (field->is_extension()) { return GetExtensionSet(message).GetRepeatedString(field->number(), index); } else { - switch (field->options().ctype()) { + switch (internal::cpp::GetStringType(*field)) { default: // TODO: Support other string reps. - case FieldOptions::STRING: + case internal::cpp::StringType::kView: + // Currently VIEW has the same ABI than string, so this fallback + // is fine. + case internal::cpp::StringType::kString: return GetRepeatedPtrField(message, field, index); } } @@ -2060,8 +2108,11 @@ absl::string_view Reflection::GetRepeatedStringView( return GetExtensionSet(message).GetRepeatedString(field->number(), index); } - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::STRING: + switch (internal::cpp::GetStringType(*field)) { + case internal::cpp::StringType::kView: + // Currently VIEW has the same ABI than string, so this fallback is + // fine. + case internal::cpp::StringType::kString: default: return GetRepeatedPtrField(message, field, index); } @@ -2076,9 +2127,12 @@ void Reflection::SetRepeatedString(Message* message, MutableExtensionSet(message)->SetRepeatedString(field->number(), index, std::move(value)); } else { - switch (field->options().ctype()) { + switch (internal::cpp::GetStringType(*field)) { default: // TODO: Support other string reps. - case FieldOptions::STRING: + case internal::cpp::StringType::kView: + // Currently VIEW has the same ABI than string, so this fallback + // is fine. + case internal::cpp::StringType::kString: MutableRepeatedField(message, field, index) ->assign(std::move(value)); break; @@ -2094,9 +2148,12 @@ void Reflection::AddString(Message* message, const FieldDescriptor* field, MutableExtensionSet(message)->AddString(field->number(), field->type(), std::move(value), field); } else { - switch (field->options().ctype()) { + switch (internal::cpp::GetStringType(*field)) { default: // TODO: Support other string reps. - case FieldOptions::STRING: + case internal::cpp::StringType::kView: + // Currently VIEW has the same ABI than string, so this fallback + // is fine. + case internal::cpp::StringType::kString: AddField(message, field)->assign(std::move(value)); break; } @@ -2582,12 +2639,11 @@ void Reflection::UnsafeArenaAddAllocatedMessage(Message* message, } } -void* Reflection::MutableRawRepeatedField(Message* message, - const FieldDescriptor* field, - FieldDescriptor::CppType cpptype, - int ctype, - const Descriptor* desc) const { - (void)ctype; // Parameter is used by Google-internal code. +void* Reflection::MutableRawRepeatedField( + Message* message, const FieldDescriptor* field, + FieldDescriptor::CppType cpptype, + absl::optional string_type, + const Descriptor* desc) const { USAGE_CHECK_REPEATED("MutableRawRepeatedField"); if (field->cpp_type() != cpptype && @@ -2595,6 +2651,20 @@ void* Reflection::MutableRawRepeatedField(Message* message, cpptype != FieldDescriptor::CPPTYPE_INT32)) ReportReflectionUsageTypeError(descriptor_, field, "MutableRawRepeatedField", cpptype); + if (string_type.has_value()) { + // VIEW and STRING currently both use the same ABI for repeated field, so + // both are accessed through the same call looking for a + // `RepeatedPtrField`. Normalize that here to avoid false + // positives on the check. + auto field_string_type = internal::cpp::GetStringType(*field); + if (field_string_type == internal::cpp::StringType::kView) { + field_string_type = internal::cpp::StringType::kString; + } + ABSL_CHECK_EQ(static_cast(field_string_type), + static_cast(*string_type)) + << "subtype mismatch"; + } + if (desc != nullptr) ABSL_CHECK_EQ(field->message_type(), desc) << "wrong submessage type"; if (field->is_extension()) { @@ -2610,19 +2680,22 @@ void* Reflection::MutableRawRepeatedField(Message* message, } } -const void* Reflection::GetRawRepeatedField(const Message& message, - const FieldDescriptor* field, - FieldDescriptor::CppType cpptype, - int ctype, - const Descriptor* desc) const { +const void* Reflection::GetRawRepeatedField( + const Message& message, const FieldDescriptor* field, + FieldDescriptor::CppType cpptype, + absl::optional string_type, + const Descriptor* desc) const { USAGE_CHECK_REPEATED("GetRawRepeatedField"); if (field->cpp_type() != cpptype && (field->cpp_type() != FieldDescriptor::CPPTYPE_ENUM || cpptype != FieldDescriptor::CPPTYPE_INT32)) ReportReflectionUsageTypeError(descriptor_, field, "GetRawRepeatedField", cpptype); - if (ctype >= 0) - ABSL_CHECK_EQ(field->options().ctype(), ctype) << "subtype mismatch"; + if (string_type.has_value()) { + ABSL_CHECK_EQ(static_cast(internal::cpp::GetStringType(*field)), + static_cast(*string_type)) + << "subtype mismatch"; + } if (desc != nullptr) ABSL_CHECK_EQ(field->message_type(), desc) << "wrong submessage type"; if (field->is_extension()) { @@ -2748,7 +2821,7 @@ static Type* AllocIfDefault(const FieldDescriptor* field, Type*& ptr, // be e.g. char). if (field->cpp_type() < FieldDescriptor::CPPTYPE_STRING || (field->cpp_type() == FieldDescriptor::CPPTYPE_STRING && - internal::cpp::EffectiveStringCType(field) == FieldOptions::CORD)) { + internal::cpp::StringTypeIsCord(*field))) { ptr = reinterpret_cast(Arena::Create>(arena)); } else { @@ -2927,11 +3000,14 @@ bool Reflection::HasBit(const Message& message, // (which uses HasField()) needs to be consistent with this. switch (field->cpp_type()) { case FieldDescriptor::CPPTYPE_STRING: - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: + switch (internal::cpp::GetStringType(*field)) { + case internal::cpp::StringType::kCord: return !GetField(message, field).empty(); default: - case FieldOptions::STRING: { + case internal::cpp::StringType::kView: + // Currently VIEW has the same ABI than string, so this fallback + // is fine. + case internal::cpp::StringType::kString: { if (IsInlined(field)) { return !GetField(message, field) .GetNoArena() @@ -3042,12 +3118,15 @@ void Reflection::ClearOneof(Message* message, if (message->GetArena() == nullptr) { switch (field->cpp_type()) { case FieldDescriptor::CPPTYPE_STRING: { - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: + switch (internal::cpp::GetStringType(*field)) { + case internal::cpp::StringType::kCord: delete *MutableRaw(message, field); break; default: - case FieldOptions::STRING: { + case internal::cpp::StringType::kView: + // Currently VIEW has the same ABI than string, so this fallback + // is fine. + case internal::cpp::StringType::kString: { // Oneof string fields are never set as a default instance. // We just need to pass some arbitrary default string to make it // work. This allows us to not have the real default accessible @@ -3086,32 +3165,29 @@ void Reflection::ClearOneof(Message* message, MutableRawRepeatedField(message, field, CPPTYPE, CTYPE, nullptr)); \ } -HANDLE_TYPE(int32_t, FieldDescriptor::CPPTYPE_INT32, -1); -HANDLE_TYPE(int64_t, FieldDescriptor::CPPTYPE_INT64, -1); -HANDLE_TYPE(uint32_t, FieldDescriptor::CPPTYPE_UINT32, -1); -HANDLE_TYPE(uint64_t, FieldDescriptor::CPPTYPE_UINT64, -1); -HANDLE_TYPE(float, FieldDescriptor::CPPTYPE_FLOAT, -1); -HANDLE_TYPE(double, FieldDescriptor::CPPTYPE_DOUBLE, -1); -HANDLE_TYPE(bool, FieldDescriptor::CPPTYPE_BOOL, -1); +HANDLE_TYPE(int32_t, FieldDescriptor::CPPTYPE_INT32, absl::nullopt); +HANDLE_TYPE(int64_t, FieldDescriptor::CPPTYPE_INT64, absl::nullopt); +HANDLE_TYPE(uint32_t, FieldDescriptor::CPPTYPE_UINT32, absl::nullopt); +HANDLE_TYPE(uint64_t, FieldDescriptor::CPPTYPE_UINT64, absl::nullopt); +HANDLE_TYPE(float, FieldDescriptor::CPPTYPE_FLOAT, absl::nullopt); +HANDLE_TYPE(double, FieldDescriptor::CPPTYPE_DOUBLE, absl::nullopt); +HANDLE_TYPE(bool, FieldDescriptor::CPPTYPE_BOOL, absl::nullopt); #undef HANDLE_TYPE -const void* Reflection::GetRawRepeatedString(const Message& message, - const FieldDescriptor* field, - bool is_string) const { - (void)is_string; // Parameter is used by Google-internal code. +const void* Reflection::GetRawRepeatedString( + const Message& message, const FieldDescriptor* field, + internal::cpp::StringType string_type) const { return GetRawRepeatedField(message, field, FieldDescriptor::CPPTYPE_STRING, - FieldOptions::STRING, nullptr); + string_type, nullptr); } -void* Reflection::MutableRawRepeatedString(Message* message, - const FieldDescriptor* field, - bool is_string) const { - (void)is_string; // Parameter is used by Google-internal code. - return MutableRawRepeatedField(message, field, - FieldDescriptor::CPPTYPE_STRING, - FieldOptions::STRING, nullptr); +void* Reflection::MutableRawRepeatedString( + Message* message, const FieldDescriptor* field, + internal::cpp::StringType string_type) const { + return MutableRawRepeatedField( + message, field, FieldDescriptor::CPPTYPE_STRING, string_type, nullptr); } // Template implementations of basic accessors. Inline because each diff --git a/src/google/protobuf/generated_message_tctable_gen.cc b/src/google/protobuf/generated_message_tctable_gen.cc index 577c6f50cb0f..b54bd096aa3f 100644 --- a/src/google/protobuf/generated_message_tctable_gen.cc +++ b/src/google/protobuf/generated_message_tctable_gen.cc @@ -27,6 +27,7 @@ #include "google/protobuf/descriptor.pb.h" #include "google/protobuf/generated_message_tctable_decl.h" #include "google/protobuf/generated_message_tctable_impl.h" +#include "google/protobuf/port.h" #include "google/protobuf/wire_format.h" // Must come last: @@ -168,11 +169,11 @@ TailCallTableInfo::FastFieldInfo::Field MakeFastFieldEntry( : field->is_repeated() ? PROTOBUF_PICK_FUNCTION(fn##R) \ : PROTOBUF_PICK_FUNCTION(fn##S)) -#define PROTOBUF_PICK_STRING_FUNCTION(fn) \ - (field->options().ctype() == FieldOptions::CORD \ - ? PROTOBUF_PICK_FUNCTION(fn##cS) \ - : options.is_string_inlined ? PROTOBUF_PICK_FUNCTION(fn##iS) \ - : PROTOBUF_PICK_REPEATABLE_FUNCTION(fn)) +#define PROTOBUF_PICK_STRING_FUNCTION(fn) \ + (cpp::StringTypeIsCord(*field) ? PROTOBUF_PICK_FUNCTION(fn##cS) \ + : /* Missing case for kView */ \ + options.is_string_inlined ? PROTOBUF_PICK_FUNCTION(fn##iS) \ + : PROTOBUF_PICK_REPEATABLE_FUNCTION(fn)) const FieldDescriptor* field = entry.field; info.aux_idx = static_cast(entry.aux_idx); @@ -305,13 +306,20 @@ bool IsFieldEligibleForFastParsing( // Some bytes fields can be handled on fast path. case FieldDescriptor::TYPE_STRING: case FieldDescriptor::TYPE_BYTES: - if (field->options().ctype() == FieldOptions::STRING) { - // strings are fine... - } else if (field->options().ctype() == FieldOptions::CORD) { - // Cords are worth putting into the fast table, if they're not repeated - if (field->is_repeated()) return false; - } else { - return false; + switch (cpp::GetStringType(*field)) { + case cpp::StringType::kView: + case cpp::StringType::kString: + // strings are fine... + break; + case cpp::StringType::kCord: + // Cords are worth putting into the fast table, if they're not + // repeated + if (field->is_repeated()) return false; + break; + case cpp::StringType::kStringPiece: + return false; + default: + Unreachable(); } if (options.is_string_inlined) { ABSL_CHECK(!field->is_repeated()); @@ -760,12 +768,15 @@ uint16_t MakeTypeCardForField( // Fill in extra information about string and bytes field representations. if (field->type() == FieldDescriptor::TYPE_BYTES || field->type() == FieldDescriptor::TYPE_STRING) { - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: + switch (cpp::GetStringType(*field)) { + case cpp::StringType::kCord: // `Cord` is always used, even for repeated fields. type_card |= fl::kRepCord; break; - case FieldOptions::STRING: + case cpp::StringType::kView: + // Currently VIEW has the same ABI than string, so this fallback is + // fine. + case cpp::StringType::kString: if (field->is_repeated()) { // A repeated string field uses RepeatedPtrField // (unless it has a ctype option; see above). diff --git a/src/google/protobuf/message.cc b/src/google/protobuf/message.cc index caf941ad5b4a..c0e126eb126c 100644 --- a/src/google/protobuf/message.cc +++ b/src/google/protobuf/message.cc @@ -456,9 +456,12 @@ const internal::RepeatedFieldAccessor* Reflection::RepeatedFieldAccessor( HANDLE_PRIMITIVE_TYPE(ENUM, int32_t) #undef HANDLE_PRIMITIVE_TYPE case FieldDescriptor::CPPTYPE_STRING: - switch (field->options().ctype()) { + switch (internal::cpp::GetStringType(*field)) { default: - case FieldOptions::STRING: + case internal::cpp::StringType::kView: + // Currently VIEW has the same ABI than string, so this fallback is + // fine. + case internal::cpp::StringType::kString: return GetSingleton(); } break; diff --git a/src/google/protobuf/message.h b/src/google/protobuf/message.h index 0354c9e9d7e7..ae50691f810e 100644 --- a/src/google/protobuf/message.h +++ b/src/google/protobuf/message.h @@ -139,6 +139,7 @@ class MapReflectionTester; class TextFormat; namespace internal { + struct FuzzPeer; struct DescriptorTable; template @@ -229,6 +230,17 @@ bool CreateUnknownEnumValues(const FieldDescriptor* field); PROTOBUF_EXPORT bool IsDescendant(Message& root, const Message& message); inline void MaybePoisonAfterClear(Message* root); + +namespace cpp { +// Use internal types instead of ctype or string_type. +enum class StringType : uint8_t { + kView, + kString, + kCord, + kStringPiece, +}; +} // namespace cpp + } // namespace internal // Abstract interface for protocol messages. @@ -969,17 +981,20 @@ class PROTOBUF_EXPORT Reflection final { // Obtain a pointer to a Repeated Field Structure and do some type checking: // on field->cpp_type(), - // on field->field_option().ctype() (if ctype >= 0) + // on string type if `string_type.has_value()` // of field->message_type() (if message_type != nullptr). // We use 2 routine rather than 4 (const vs mutable) x (scalar vs pointer). - void* MutableRawRepeatedField(Message* message, const FieldDescriptor* field, - FieldDescriptor::CppType cpptype, int ctype, - const Descriptor* desc) const; + void* MutableRawRepeatedField( + Message* message, const FieldDescriptor* field, + FieldDescriptor::CppType cpptype, + absl::optional string_type, + const Descriptor* desc) const; - const void* GetRawRepeatedField(const Message& message, - const FieldDescriptor* field, - FieldDescriptor::CppType cpptype, int ctype, - const Descriptor* desc) const; + const void* GetRawRepeatedField( + const Message& message, const FieldDescriptor* field, + FieldDescriptor::CppType cpptype, + absl::optional string_type, + const Descriptor* desc) const; // The following methods are used to implement (Mutable)RepeatedFieldRef. // A Ref object will store a raw pointer to the repeated field data (obtained @@ -1113,9 +1128,9 @@ class PROTOBUF_EXPORT Reflection final { // file here is not possible because it would cause a circular include cycle. const void* GetRawRepeatedString(const Message& message, const FieldDescriptor* field, - bool is_string) const; + internal::cpp::StringType string_type) const; void* MutableRawRepeatedString(Message* message, const FieldDescriptor* field, - bool is_string) const; + internal::cpp::StringType string_type) const; friend class MapReflectionTester; // Returns true if key is in map. Returns false if key is not in map field. @@ -1545,15 +1560,15 @@ inline const RepeatedPtrField& Reflection::GetRepeatedPtrFieldInternal( const Message& message, const FieldDescriptor* field) const { return *static_cast*>( - GetRawRepeatedString(message, field, true)); + GetRawRepeatedString(message, field, internal::cpp::StringType::kString)); } template <> inline RepeatedPtrField* Reflection::MutableRepeatedPtrFieldInternal( Message* message, const FieldDescriptor* field) const { - return static_cast*>( - MutableRawRepeatedString(message, field, true)); + return static_cast*>(MutableRawRepeatedString( + message, field, internal::cpp::StringType::kString)); } @@ -1562,31 +1577,33 @@ Reflection::MutableRepeatedPtrFieldInternal( template <> inline const RepeatedPtrField& Reflection::GetRepeatedPtrFieldInternal( const Message& message, const FieldDescriptor* field) const { - return *static_cast*>(GetRawRepeatedField( - message, field, FieldDescriptor::CPPTYPE_MESSAGE, -1, nullptr)); + return *static_cast*>( + GetRawRepeatedField(message, field, FieldDescriptor::CPPTYPE_MESSAGE, + absl::nullopt, nullptr)); } template <> inline RepeatedPtrField* Reflection::MutableRepeatedPtrFieldInternal( Message* message, const FieldDescriptor* field) const { - return static_cast*>(MutableRawRepeatedField( - message, field, FieldDescriptor::CPPTYPE_MESSAGE, -1, nullptr)); + return static_cast*>( + MutableRawRepeatedField(message, field, FieldDescriptor::CPPTYPE_MESSAGE, + absl::nullopt, nullptr)); } template inline const RepeatedPtrField& Reflection::GetRepeatedPtrFieldInternal( const Message& message, const FieldDescriptor* field) const { - return *static_cast*>( - GetRawRepeatedField(message, field, FieldDescriptor::CPPTYPE_MESSAGE, -1, - PB::default_instance().GetDescriptor())); + return *static_cast*>(GetRawRepeatedField( + message, field, FieldDescriptor::CPPTYPE_MESSAGE, absl::nullopt, + PB::default_instance().GetDescriptor())); } template inline RepeatedPtrField* Reflection::MutableRepeatedPtrFieldInternal( Message* message, const FieldDescriptor* field) const { - return static_cast*>( - MutableRawRepeatedField(message, field, FieldDescriptor::CPPTYPE_MESSAGE, - -1, PB::default_instance().GetDescriptor())); + return static_cast*>(MutableRawRepeatedField( + message, field, FieldDescriptor::CPPTYPE_MESSAGE, absl::nullopt, + PB::default_instance().GetDescriptor())); } template diff --git a/src/google/protobuf/no_field_presence_test.cc b/src/google/protobuf/no_field_presence_test.cc index 56586731c75d..766b19978407 100644 --- a/src/google/protobuf/no_field_presence_test.cc +++ b/src/google/protobuf/no_field_presence_test.cc @@ -283,10 +283,11 @@ TEST(NoFieldPresenceTest, ReflectionHasFieldTest) { if (field->is_repeated() || field->containing_oneof()) { continue; } - if (field->options().ctype() != FieldOptions::STRING) { + if (field->cpp_type() == field->CPPTYPE_STRING && + !internal::cpp::StringTypeIsSupported(*field)) { continue; } - EXPECT_EQ(true, r->HasField(message, field)); + EXPECT_EQ(true, r->HasField(message, field)) << field->full_name(); } message.Clear(); diff --git a/src/google/protobuf/reflection_visit_field_info.h b/src/google/protobuf/reflection_visit_field_info.h index bca311a4f8ac..65c487ea3324 100644 --- a/src/google/protobuf/reflection_visit_field_info.h +++ b/src/google/protobuf/reflection_visit_field_info.h @@ -116,8 +116,9 @@ struct DynamicFieldInfoHelper { static absl::string_view GetStringView(const Reflection* reflection, const Message& message, const FieldDescriptor* field) { - auto ctype = cpp::EffectiveStringCType(field); - ABSL_DCHECK_NE(ctype, FieldOptions::CORD); + auto type = cpp::GetStringType(*field); + ABSL_DCHECK_NE(static_cast(type), + static_cast(cpp::StringType::kCord)); ABSL_DCHECK(!is_oneof || reflection->HasOneofField(message, field)); auto str = Get(reflection, message, field); ABSL_DCHECK(!str.IsDefault()); diff --git a/src/google/protobuf/reflection_visit_fields.h b/src/google/protobuf/reflection_visit_fields.h index 0e3acb0e75d6..f31ec7bd9e48 100644 --- a/src/google/protobuf/reflection_visit_fields.h +++ b/src/google/protobuf/reflection_visit_fields.h @@ -174,9 +174,12 @@ void ReflectionVisit::VisitFields(MessageT& message, CallbackFn&& func, reflection, message, field, rep}); \ } - switch (cpp::EffectiveStringCType(field)) { + switch (cpp::GetStringType(*field)) { default: - case FieldOptions::STRING: + case cpp::StringType::kView: + // Currently VIEW has the same ABI than string, so this fallback + // is fine. + case cpp::StringType::kString: PROTOBUF_IMPL_STRING_CASE(std::string, String); break; } @@ -227,11 +230,14 @@ void ReflectionVisit::VisitFields(MessageT& message, CallbackFn&& func, case FieldDescriptor::TYPE_BYTES: case FieldDescriptor::TYPE_STRING: { - auto ctype = cpp::EffectiveStringCType(field); - if (ctype == FieldOptions::CORD) { + auto type = cpp::GetStringType(*field); + if (type == cpp::StringType::kCord) { func(CordDynamicFieldInfo{reflection, message, field}); } else { + ABSL_DCHECK(type == cpp::StringType::kStringPiece || + type == cpp::StringType::kString || + type == cpp::StringType::kView); func(StringDynamicFieldInfo{reflection, message, field}); } @@ -279,11 +285,14 @@ void ReflectionVisit::VisitFields(MessageT& message, CallbackFn&& func, break; case FieldDescriptor::TYPE_BYTES: case FieldDescriptor::TYPE_STRING: { - auto ctype = cpp::EffectiveStringCType(field); - if (ctype == FieldOptions::CORD) { + auto type = cpp::GetStringType(*field); + if (type == cpp::StringType::kCord) { func(CordDynamicFieldInfo{reflection, message, field}); } else { + ABSL_DCHECK(type == cpp::StringType::kStringPiece || + type == cpp::StringType::kString || + type == cpp::StringType::kView); func(StringDynamicFieldInfo{reflection, message, field}); } diff --git a/src/google/protobuf/wire_format.cc b/src/google/protobuf/wire_format.cc index 72e13c4ba7f7..e14724d6eca0 100644 --- a/src/google/protobuf/wire_format.cc +++ b/src/google/protobuf/wire_format.cc @@ -560,7 +560,7 @@ bool WireFormat::ParseAndMergeField( } case FieldDescriptor::TYPE_BYTES: { - if (internal::cpp::EffectiveStringCType(field) == FieldOptions::CORD) { + if (cpp::StringTypeIsCord(*field)) { absl::Cord value; if (!WireFormatLite::ReadBytes(input, &value)) return false; message_reflection->SetString(message, field, value); @@ -965,7 +965,7 @@ const char* WireFormat::_InternalParseAndMergeField( case FieldDescriptor::TYPE_BYTES: { int size = ReadSize(&ptr); if (ptr == nullptr) return nullptr; - if (internal::cpp::EffectiveStringCType(field) == FieldOptions::CORD) { + if (cpp::StringTypeIsCord(*field)) { absl::Cord value; ptr = ctx->ReadCord(ptr, size, &value); if (ptr == nullptr) return nullptr; @@ -1023,7 +1023,7 @@ const char* WireFormat::_InternalParseAndMergeField( auto* value_field = field->message_type()->map_value(); auto* enum_type = value_field->enum_type(); if (enum_type != nullptr && - !internal::cpp::HasPreservingUnknownEnumSemantics(value_field) && + !cpp::HasPreservingUnknownEnumSemantics(value_field) && enum_type->FindValueByNumber( sub_message->GetReflection()->GetEnumValue( *sub_message, value_field)) == nullptr) { @@ -1424,7 +1424,7 @@ uint8_t* WireFormat::InternalSerializeField(const FieldDescriptor* field, } case FieldDescriptor::TYPE_BYTES: { - if (internal::cpp::EffectiveStringCType(field) == FieldOptions::CORD) { + if (cpp::StringTypeIsCord(*field)) { absl::Cord value = message_reflection->GetCord(message, field); target = stream->WriteString(field->number(), value, target); break; @@ -1725,7 +1725,7 @@ size_t WireFormat::FieldDataOnlyByteSize(const FieldDescriptor* field, // instead of copying. case FieldDescriptor::TYPE_STRING: case FieldDescriptor::TYPE_BYTES: { - if (internal::cpp::EffectiveStringCType(field) == FieldOptions::CORD) { + if (cpp::StringTypeIsCord(*field)) { for (size_t j = 0; j < count; j++) { absl::Cord value = message_reflection->GetCord(message, field); data_size += WireFormatLite::StringSize(value);