From ee391369e65a10d0de6bb033f2b7f377d50b9f12 Mon Sep 17 00:00:00 2001 From: Mike Kruskal Date: Wed, 2 Oct 2024 17:11:15 -0700 Subject: [PATCH] Delete internal EffectiveStringCType helper. This has been replaced with a public cpp_string_type descriptor API, that supports both ctype and string_type smoothly between editions. PiperOrigin-RevId: 681647787 --- src/google/protobuf/compiler/cpp/field.cc | 6 +++ src/google/protobuf/compiler/cpp/field.h | 3 ++ .../cpp/field_generators/string_field.cc | 6 +-- .../cpp/field_generators/string_view_field.cc | 3 +- src/google/protobuf/compiler/cpp/helpers.cc | 17 +-------- src/google/protobuf/compiler/cpp/helpers.h | 10 ++--- src/google/protobuf/compiler/cpp/message.cc | 13 ++----- src/google/protobuf/descriptor.h | 14 ------- .../protobuf/generated_message_reflection.cc | 15 +++++++- .../protobuf/reflection_visit_field_info.h | 4 +- src/google/protobuf/reflection_visit_fields.h | 37 +++++++++++-------- 11 files changed, 57 insertions(+), 71 deletions(-) diff --git a/src/google/protobuf/compiler/cpp/field.cc b/src/google/protobuf/compiler/cpp/field.cc index b7a8d1f450..a38518d96f 100644 --- a/src/google/protobuf/compiler/cpp/field.cc +++ b/src/google/protobuf/compiler/cpp/field.cc @@ -227,6 +227,12 @@ void FieldGeneratorBase::GenerateCopyConstructorCode(io::Printer* p) const { } } +pb::CppFeatures::StringType FieldGeneratorBase::GetDeclaredStringType() const { + return CppGenerator::GetResolvedSourceFeatures(*field_) + .GetExtension(pb::cpp) + .string_type(); +} + namespace { std::unique_ptr MakeGenerator(const FieldDescriptor* field, const Options& options, diff --git a/src/google/protobuf/compiler/cpp/field.h b/src/google/protobuf/compiler/cpp/field.h index 4722fc5236..7a0a8f1d16 100644 --- a/src/google/protobuf/compiler/cpp/field.h +++ b/src/google/protobuf/compiler/cpp/field.h @@ -25,6 +25,7 @@ #include "absl/types/span.h" #include "google/protobuf/compiler/cpp/helpers.h" #include "google/protobuf/compiler/cpp/options.h" +#include "google/protobuf/cpp_features.pb.h" #include "google/protobuf/descriptor.h" #include "google/protobuf/io/printer.h" @@ -199,6 +200,8 @@ class FieldGeneratorBase { MessageSCCAnalyzer* scc_; absl::flat_hash_map variables_; + pb::CppFeatures::StringType GetDeclaredStringType() const; + private: bool should_split_ = false; bool is_trivial_ = false; 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 88635e3a88..1fc3951d49 100644 --- a/src/google/protobuf/compiler/cpp/field_generators/string_field.cc +++ b/src/google/protobuf/compiler/cpp/field_generators/string_field.cc @@ -234,8 +234,7 @@ 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_); + bool unknown_ctype = GetDeclaredStringType() != pb::CppFeatures::STRING; if (unknown_ctype) { p->Emit(R"cc( @@ -820,8 +819,7 @@ class RepeatedString : public FieldGeneratorBase { }; void RepeatedString::GenerateAccessorDeclarations(io::Printer* p) const { - bool unknown_ctype = - field_->options().ctype() != internal::cpp::EffectiveStringCType(field_); + bool unknown_ctype = GetDeclaredStringType() != pb::CppFeatures::STRING; if (unknown_ctype) { p->Emit(R"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 43a4beb881..6f09abc557 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 @@ -637,8 +637,7 @@ class RepeatedStringView : public FieldGeneratorBase { }; void RepeatedStringView::GenerateAccessorDeclarations(io::Printer* p) const { - bool unknown_ctype = - field_->options().ctype() != internal::cpp::EffectiveStringCType(field_); + bool unknown_ctype = GetDeclaredStringType() != pb::CppFeatures::VIEW; if (unknown_ctype) { p->Emit(R"cc( diff --git a/src/google/protobuf/compiler/cpp/helpers.cc b/src/google/protobuf/compiler/cpp/helpers.cc index b6fd838ec3..6eaa38fb9c 100644 --- a/src/google/protobuf/compiler/cpp/helpers.cc +++ b/src/google/protobuf/compiler/cpp/helpers.cc @@ -1094,18 +1094,8 @@ 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; - } for (int i = 0; i < descriptor->nested_type_count(); ++i) { if (HasStringPieceFields(descriptor->nested_type(i), options)) return true; } @@ -1119,15 +1109,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 09dcb20a2f..c8078e15e4 100644 --- a/src/google/protobuf/compiler/cpp/helpers.h +++ b/src/google/protobuf/compiler/cpp/helpers.h @@ -330,19 +330,15 @@ 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; + field->cpp_string_type() == FieldDescriptor::CppStringType::kCord; } inline bool IsString(const FieldDescriptor* field) { return field->cpp_type() == FieldDescriptor::CPPTYPE_STRING && - internal::cpp::EffectiveStringCType(field) == FieldOptions::STRING; + (field->cpp_string_type() == FieldDescriptor::CppStringType::kString || + field->cpp_string_type() == FieldDescriptor::CppStringType::kView); } -inline bool IsStringPiece(const FieldDescriptor* field) { - return field->cpp_type() == FieldDescriptor::CPPTYPE_STRING && - internal::cpp::EffectiveStringCType(field) == - FieldOptions::STRING_PIECE; -} bool IsProfileDriven(const Options& options); diff --git a/src/google/protobuf/compiler/cpp/message.cc b/src/google/protobuf/compiler/cpp/message.cc index 26128b3899..2af5856224 100644 --- a/src/google/protobuf/compiler/cpp/message.cc +++ b/src/google/protobuf/compiler/cpp/message.cc @@ -3880,21 +3880,16 @@ MessageGenerator::NewOpRequirements MessageGenerator::GetNewOp( break; case FieldDescriptor::CPPTYPE_STRING: - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::STRING_PIECE: - op.needs_arena_seeding = true; - print_arena_offset(); - break; - case FieldOptions::CORD: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: // Cord fields are currently rejected above because of ArenaDtor // requirements. ABSL_CHECK(op.needs_to_run_constructor); break; - case FieldOptions::STRING: + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: op.needs_memcpy = true; break; - default: - ABSL_LOG(FATAL); } break; case FieldDescriptor::CPPTYPE_MESSAGE: diff --git a/src/google/protobuf/descriptor.h b/src/google/protobuf/descriptor.h index ccbc409746..eda070f5c6 100644 --- a/src/google/protobuf/descriptor.h +++ b/src/google/protobuf/descriptor.h @@ -2949,20 +2949,6 @@ PROTOBUF_EXPORT bool HasPreservingUnknownEnumSemantics( PROTOBUF_EXPORT bool HasHasbit(const FieldDescriptor* field); #ifndef SWIG -// For a string field, returns the effective ctype. If the actual ctype is -// not supported, returns the default of STRING. -template -typename FieldOpts::CType EffectiveStringCType(const FieldDesc* field) { - // TODO Replace this function with - // FieldDescriptor::cpp_string_type; - switch (field->cpp_string_type()) { - case FieldDescriptor::CppStringType::kCord: - return FieldOpts::CORD; - default: - return FieldOpts::STRING; - } -} enum class Utf8CheckMode : uint8_t { kStrict = 0, // Parsing will fail if non UTF-8 data is in string fields. diff --git a/src/google/protobuf/generated_message_reflection.cc b/src/google/protobuf/generated_message_reflection.cc index 391318ed21..6857caf65f 100644 --- a/src/google/protobuf/generated_message_reflection.cc +++ b/src/google/protobuf/generated_message_reflection.cc @@ -33,6 +33,7 @@ #include "absl/synchronization/mutex.h" #include "google/protobuf/descriptor.h" #include "google/protobuf/descriptor.pb.h" +#include "google/protobuf/descriptor_lite.h" #include "google/protobuf/extension_set.h" #include "google/protobuf/generated_message_tctable_decl.h" #include "google/protobuf/generated_message_tctable_gen.h" @@ -162,6 +163,17 @@ PROTOBUF_NOINLINE const std::string& NameOfDenseEnumSlow( } } +bool IsMatchingCType(const FieldDescriptor* field, int ctype) { + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: + return ctype == FieldOptions::CORD; + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: + return ctype == FieldOptions::STRING; + } + internal::Unreachable(); +} + } // namespace internal // =================================================================== @@ -2669,8 +2681,7 @@ const void* Reflection::GetRawRepeatedField(const Message& message, ReportReflectionUsageTypeError(descriptor_, field, "GetRawRepeatedField", cpptype); if (ctype >= 0) - ABSL_CHECK_EQ(internal::cpp::EffectiveStringCType(field), ctype) - << "subtype mismatch"; + ABSL_CHECK(IsMatchingCType(field, ctype)) << "subtype mismatch"; if (desc != nullptr) ABSL_CHECK_EQ(field->message_type(), desc) << "wrong submessage type"; if (field->is_extension()) { diff --git a/src/google/protobuf/reflection_visit_field_info.h b/src/google/protobuf/reflection_visit_field_info.h index 2edc56984a..dee9c8526d 100644 --- a/src/google/protobuf/reflection_visit_field_info.h +++ b/src/google/protobuf/reflection_visit_field_info.h @@ -117,8 +117,8 @@ 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 string_type = field->cpp_string_type(); + ABSL_DCHECK(string_type != FieldDescriptor::CppStringType::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 d1b317bc2f..005b6d8311 100644 --- a/src/google/protobuf/reflection_visit_fields.h +++ b/src/google/protobuf/reflection_visit_fields.h @@ -174,9 +174,10 @@ void ReflectionVisit::VisitFields(MessageT& message, CallbackFn&& func, reflection, message, field, rep}); \ } - switch (cpp::EffectiveStringCType(field)) { - default: - case FieldOptions::STRING: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: PROTOBUF_IMPL_STRING_CASE(std::string, String); break; } @@ -227,13 +228,16 @@ void ReflectionVisit::VisitFields(MessageT& message, CallbackFn&& func, case FieldDescriptor::TYPE_BYTES: case FieldDescriptor::TYPE_STRING: { - auto ctype = cpp::EffectiveStringCType(field); - if (ctype == FieldOptions::CORD) { - func(CordDynamicFieldInfo{reflection, message, - field}); - } else { - func(StringDynamicFieldInfo{reflection, message, + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: + func(CordDynamicFieldInfo{reflection, message, field}); + break; + case FieldDescriptor::CppStringType::kString: + case FieldDescriptor::CppStringType::kView: + func(StringDynamicFieldInfo{reflection, message, + field}); + break; } break; } @@ -279,13 +283,16 @@ 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) { - func(CordDynamicFieldInfo{reflection, message, - field}); - } else { - func(StringDynamicFieldInfo{reflection, message, + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: + func(CordDynamicFieldInfo{reflection, message, field}); + break; + case FieldDescriptor::CppStringType::kString: + case FieldDescriptor::CppStringType::kView: + func(StringDynamicFieldInfo{reflection, message, + field}); + break; } break; }