From 17f4f915a3e81fe179ae3ecb156655e61cbf7b64 Mon Sep 17 00:00:00 2001 From: Mike Kruskal Date: Thu, 7 Dec 2023 14:43:36 -0800 Subject: [PATCH] Remove descriptor_legacy usage from C++ runtime PiperOrigin-RevId: 588915020 --- conformance/BUILD.bazel | 1 - conformance/conformance_test.cc | 12 +- src/google/protobuf/BUILD.bazel | 1 - src/google/protobuf/descriptor.cc | 147 ++++++++---------- src/google/protobuf/descriptor.h | 1 + src/google/protobuf/descriptor_legacy.h | 6 +- src/google/protobuf/descriptor_unittest.cc | 18 +-- src/google/protobuf/dynamic_message.cc | 32 ++-- .../protobuf/generated_message_reflection.cc | 21 ++- src/google/protobuf/proto3_arena_unittest.cc | 12 +- src/google/protobuf/util/BUILD.bazel | 1 - .../protobuf/util/type_resolver_util.cc | 16 +- .../protobuf/util/type_resolver_util_test.cc | 7 +- 13 files changed, 110 insertions(+), 165 deletions(-) diff --git a/conformance/BUILD.bazel b/conformance/BUILD.bazel index 5b9e1da84ae9..99827fc59f2f 100644 --- a/conformance/BUILD.bazel +++ b/conformance/BUILD.bazel @@ -144,7 +144,6 @@ cc_library( includes = ["."], deps = [ ":conformance_cc_proto", - "//src/google/protobuf:descriptor_legacy", "//src/google/protobuf/util:differencer", "//src/google/protobuf/util:json_util", "//src/google/protobuf/util:type_resolver_util", diff --git a/conformance/conformance_test.cc b/conformance/conformance_test.cc index dbcaf45836e6..ef049c8a539d 100644 --- a/conformance/conformance_test.cc +++ b/conformance/conformance_test.cc @@ -24,7 +24,6 @@ #include "absl/strings/string_view.h" #include "conformance/conformance.pb.h" #include "conformance/conformance.pb.h" -#include "google/protobuf/descriptor_legacy.h" #include "google/protobuf/message.h" #include "google/protobuf/text_format.h" @@ -143,13 +142,12 @@ ConformanceTestSuite::ConformanceRequestSetting::NewTestMessage() const { std::string ConformanceTestSuite::ConformanceRequestSetting::GetSyntaxIdentifier() const { - switch (FileDescriptorLegacy(prototype_message_.GetDescriptor()->file()) - .syntax()) { - case FileDescriptorLegacy::Syntax::SYNTAX_PROTO3: + switch (prototype_message_.GetDescriptor()->file()->edition()) { + case Edition::EDITION_PROTO3: return "Proto3"; - case FileDescriptorLegacy::Syntax::SYNTAX_PROTO2: + case Edition::EDITION_PROTO2: return "Proto2"; - case FileDescriptorLegacy::Syntax::SYNTAX_EDITIONS: { + default: { std::string id = "Editions"; if (prototype_message_.GetDescriptor()->name() == "TestAllTypesProto2") { absl::StrAppend(&id, "_Proto2"); @@ -159,8 +157,6 @@ ConformanceTestSuite::ConformanceRequestSetting::GetSyntaxIdentifier() const { } return id; } - default: - return "Unknown"; } } diff --git a/src/google/protobuf/BUILD.bazel b/src/google/protobuf/BUILD.bazel index 03e8913c4ab5..9cf00c7ae8db 100644 --- a/src/google/protobuf/BUILD.bazel +++ b/src/google/protobuf/BUILD.bazel @@ -486,7 +486,6 @@ cc_library( "descriptor.h", "descriptor.pb.h", "descriptor_database.h", - "descriptor_legacy.h", "descriptor_visitor.h", "dynamic_message.h", "feature_resolver.h", diff --git a/src/google/protobuf/descriptor.cc b/src/google/protobuf/descriptor.cc index ce37423c94b5..95a341bd3d33 100644 --- a/src/google/protobuf/descriptor.cc +++ b/src/google/protobuf/descriptor.cc @@ -55,7 +55,6 @@ #include "google/protobuf/cpp_features.pb.h" #include "google/protobuf/descriptor.pb.h" #include "google/protobuf/descriptor_database.h" -#include "google/protobuf/descriptor_legacy.h" #include "google/protobuf/descriptor_visitor.h" #include "google/protobuf/dynamic_message.h" #include "google/protobuf/feature_resolver.h" @@ -811,7 +810,6 @@ const char* const FieldDescriptor::kLabelToName[MAX_LABEL + 1] = { "repeated", // LABEL_REPEATED }; -PROTOBUF_IGNORE_DEPRECATION_START const char* FileDescriptor::SyntaxName(FileDescriptor::Syntax syntax) { switch (syntax) { case SYNTAX_PROTO2: @@ -826,7 +824,6 @@ const char* FileDescriptor::SyntaxName(FileDescriptor::Syntax syntax) { ABSL_LOG(FATAL) << "can't reach here."; return nullptr; } -PROTOBUF_IGNORE_DEPRECATION_STOP static const char* const kNonLinkedWeakMessageReplacementName = "google.protobuf.Empty"; @@ -1181,6 +1178,25 @@ const FeatureSet& GetParentFeatures(const ServiceDescriptor* service) { const FeatureSet& GetParentFeatures(const MethodDescriptor* method) { return internal::InternalFeatureHelper::GetFeatures(*method->service()); } + +bool IsLegacyEdition(const FileDescriptor* file) { + return file->edition() < Edition::EDITION_2023; +} + +template +bool IsLegacyEdition(const DescriptorT* descriptor) { + return IsLegacyEdition(descriptor->file()); +} + +Edition GetDescriptorEdition(const FileDescriptor* descriptor) { + return descriptor->edition(); +} + +template +Edition GetDescriptorEdition(const DescriptorT* descriptor) { + return GetDescriptorEdition(descriptor->file()); +} + } // anonymous namespace // Contains tables specific to a particular file. These tables are not @@ -2755,13 +2771,10 @@ void FileDescriptor::CopyHeadingTo(FileDescriptorProto* proto) const { proto->set_package(package()); } - // TODO: Also populate when syntax="proto2". - FileDescriptorLegacy::Syntax syntax = FileDescriptorLegacy(this).syntax(); - if (syntax == FileDescriptorLegacy::Syntax::SYNTAX_PROTO3 || - syntax == FileDescriptorLegacy::Syntax::SYNTAX_EDITIONS) { - proto->set_syntax(FileDescriptorLegacy::SyntaxName(syntax)); - } - if (syntax == FileDescriptorLegacy::Syntax::SYNTAX_EDITIONS) { + if (edition() == Edition::EDITION_PROTO3) { + proto->set_syntax("proto3"); + } else if (!IsLegacyEdition(this)) { + proto->set_syntax("editions"); proto->set_edition(edition()); } @@ -2858,8 +2871,7 @@ void FieldDescriptor::CopyTo(FieldDescriptorProto* proto) const { } // Some compilers do not allow static_cast directly between two enum types, // so we must cast to int first. - if (is_required() && FileDescriptorLegacy(file()).syntax() == - FileDescriptorLegacy::Syntax::SYNTAX_EDITIONS) { + if (is_required() && !IsLegacyEdition(this)) { // Editions files have no required keyword, and we only set this label // during descriptor build. proto->set_label(static_cast( @@ -2868,9 +2880,7 @@ void FieldDescriptor::CopyTo(FieldDescriptorProto* proto) const { proto->set_label(static_cast( absl::implicit_cast(label()))); } - if (type() == TYPE_GROUP && - FileDescriptorLegacy(file()).syntax() == - FileDescriptorLegacy::Syntax::SYNTAX_EDITIONS) { + if (type() == TYPE_GROUP && !IsLegacyEdition(this)) { // Editions files have no group keyword, and we only set this label // during descriptor build. proto->set_type(static_cast( @@ -3007,11 +3017,7 @@ void MethodDescriptor::CopyTo(MethodDescriptorProto* proto) const { namespace { bool IsGroupSyntax(const FieldDescriptor* desc) { - if (FileDescriptorLegacy(desc->file()).syntax() == - FileDescriptorLegacy::SYNTAX_EDITIONS) { - return false; - } - return desc->type() == FieldDescriptor::TYPE_GROUP; + return IsLegacyEdition(desc) && desc->type() == FieldDescriptor::TYPE_GROUP; } template @@ -3125,6 +3131,13 @@ bool FormatLineOptions(int depth, const Message& options, return !all_options.empty(); } +static std::string GetLegacySyntaxName(const FileDescriptor* file) { + if (file->edition() == Edition::EDITION_PROTO3) { + return "proto3"; + } + return "proto2"; +} + class SourceLocationCommentPrinter { public: @@ -3202,13 +3215,11 @@ std::string FileDescriptor::DebugStringWithOptions( SourceLocationCommentPrinter syntax_comment(this, path, "", debug_string_options); syntax_comment.AddPreComment(&contents); - if (FileDescriptorLegacy(this).syntax() == - FileDescriptorLegacy::SYNTAX_EDITIONS) { - absl::SubstituteAndAppend(&contents, "edition = \"$0\";\n\n", edition()); - } else { + if (IsLegacyEdition(this)) { absl::SubstituteAndAppend(&contents, "syntax = \"$0\";\n\n", - FileDescriptorLegacy::SyntaxName( - FileDescriptorLegacy(this).syntax())); + GetLegacySyntaxName(this)); + } else { + absl::SubstituteAndAppend(&contents, "edition = \"$0\";\n\n", edition()); } syntax_comment.AddPostComment(&contents); } @@ -3481,13 +3492,11 @@ void FieldDescriptor::DebugString( // Label is omitted for maps, oneof, and plain proto3 fields. if (is_map() || real_containing_oneof() || - (is_optional() && !FieldDescriptorLegacy(this).has_optional_keyword())) { + (is_optional() && !has_optional_keyword())) { label.clear(); } // Label is omitted for optional and required fields under editions. - if ((is_optional() || is_required()) && - FileDescriptorLegacy(file()).syntax() == - FileDescriptorLegacy::SYNTAX_EDITIONS) { + if ((is_optional() || is_required()) && !IsLegacyEdition(this)) { label.clear(); } @@ -4261,7 +4270,6 @@ class DescriptorBuilder { void CheckFieldJsonNameUniqueness(const std::string& message_name, const DescriptorProto& message, const Descriptor* descriptor, - FileDescriptorLegacy::Syntax syntax, bool use_custom_names); void CheckEnumValueUniqueness(const EnumDescriptorProto& proto, const EnumDescriptor* result); @@ -5061,7 +5069,7 @@ FileDescriptor* DescriptorPool::NewPlaceholderFileWithMutexHeld( placeholder->tables_ = &FileDescriptorTables::GetEmptyInstance(); placeholder->source_code_info_ = &SourceCodeInfo::default_instance(); placeholder->is_placeholder_ = true; - placeholder->syntax_ = FileDescriptorLegacy::SYNTAX_UNKNOWN; + placeholder->syntax_ = FileDescriptor::SYNTAX_UNKNOWN; placeholder->finished_building_ = true; // All other fields are zero or nullptr. @@ -5283,14 +5291,12 @@ typename DescriptorT::OptionsType* DescriptorBuilder::AllocateOptionsImpl( template static void InferLegacyProtoFeatures(const ProtoT& proto, - const OptionsT& options, - FileDescriptorLegacy::Syntax syntax, + const OptionsT& options, Edition edition, FeatureSet& features) {} static void InferLegacyProtoFeatures(const FieldDescriptorProto& proto, const FieldOptions& options, - FileDescriptorLegacy::Syntax syntax, - FeatureSet& features) { + Edition edition, FeatureSet& features) { if (proto.label() == FieldDescriptorProto::LABEL_REQUIRED) { features.set_field_presence(FeatureSet::LEGACY_REQUIRED); } @@ -5300,25 +5306,13 @@ static void InferLegacyProtoFeatures(const FieldDescriptorProto& proto, if (options.packed()) { features.set_repeated_field_encoding(FeatureSet::PACKED); } - if (syntax == FileDescriptorLegacy::SYNTAX_PROTO3) { + if (edition == Edition::EDITION_PROTO3) { if (options.has_packed() && !options.packed()) { features.set_repeated_field_encoding(FeatureSet::EXPANDED); } } } -template -FileDescriptorLegacy::Syntax GetDescriptorSyntax( - const DescriptorT* descriptor) { - return FileDescriptorLegacy(descriptor->file()).syntax(); -} - -template <> -FileDescriptorLegacy::Syntax GetDescriptorSyntax( - const FileDescriptor* descriptor) { - return FileDescriptorLegacy(descriptor).syntax(); -} - template void DescriptorBuilder::ResolveFeaturesImpl( const typename DescriptorT::Proto& proto, DescriptorT* descriptor, @@ -5342,13 +5336,12 @@ void DescriptorBuilder::ResolveFeaturesImpl( FeatureSet base_features = *descriptor->proto_features_; // Handle feature inference from proto2/proto3. - if (GetDescriptorSyntax(descriptor) != - FileDescriptorLegacy::SYNTAX_EDITIONS) { + if (IsLegacyEdition(descriptor)) { if (descriptor->proto_features_ != &FeatureSet::default_instance()) { AddError(descriptor->name(), proto, error_location, "Features are only valid under editions."); } - InferLegacyProtoFeatures(proto, *options, GetDescriptorSyntax(descriptor), + InferLegacyProtoFeatures(proto, *options, GetDescriptorEdition(descriptor), base_features); } @@ -5464,13 +5457,9 @@ PROTOBUF_NOINLINE static bool ExistingFileMatchesProto( const FileDescriptor* existing_file, const FileDescriptorProto& proto) { FileDescriptorProto existing_proto; existing_file->CopyTo(&existing_proto); - // TODO: Remove it when CopyTo supports copying syntax params when - // syntax="proto2". - if (FileDescriptorLegacy(existing_file).syntax() == - FileDescriptorLegacy::Syntax::SYNTAX_PROTO2 && + if (existing_file->edition() == Edition::EDITION_PROTO2 && proto.has_syntax()) { - existing_proto.set_syntax(FileDescriptorLegacy::SyntaxName( - FileDescriptorLegacy(existing_file).syntax())); + existing_proto.set_syntax("proto2"); } return existing_proto.SerializeAsString() == proto.SerializeAsString(); @@ -5692,16 +5681,16 @@ FileDescriptor* DescriptorBuilder::BuildFileImpl( // TODO: Report error when the syntax is empty after all the protos // have added the syntax statement. if (proto.syntax().empty() || proto.syntax() == "proto2") { - file_->syntax_ = FileDescriptorLegacy::SYNTAX_PROTO2; + file_->syntax_ = FileDescriptor::SYNTAX_PROTO2; file_->edition_ = Edition::EDITION_PROTO2; } else if (proto.syntax() == "proto3") { - file_->syntax_ = FileDescriptorLegacy::SYNTAX_PROTO3; + file_->syntax_ = FileDescriptor::SYNTAX_PROTO3; file_->edition_ = Edition::EDITION_PROTO3; } else if (proto.syntax() == "editions") { - file_->syntax_ = FileDescriptorLegacy::SYNTAX_EDITIONS; + file_->syntax_ = FileDescriptor::SYNTAX_EDITIONS; file_->edition_ = proto.edition(); } else { - file_->syntax_ = FileDescriptorLegacy::SYNTAX_UNKNOWN; + file_->syntax_ = FileDescriptor::SYNTAX_UNKNOWN; file_->edition_ = Edition::EDITION_UNKNOWN; AddError(proto.name(), proto, DescriptorPool::ErrorCollector::OTHER, [&] { return absl::StrCat("Unrecognized syntax: ", proto.syntax()); @@ -6215,21 +6204,19 @@ void DescriptorBuilder::BuildMessage(const DescriptorProto& proto, void DescriptorBuilder::CheckFieldJsonNameUniqueness( const DescriptorProto& proto, const Descriptor* result) { - FileDescriptorLegacy::Syntax syntax = - FileDescriptorLegacy(result->file()).syntax(); std::string message_name = result->full_name(); if (pool_->deprecated_legacy_json_field_conflicts_ || IsLegacyJsonFieldConflictEnabled(result->options())) { - if (syntax == FileDescriptorLegacy::Syntax::SYNTAX_PROTO3) { + if (result->file()->edition() == Edition::EDITION_PROTO3) { // Only check default JSON names for conflicts in proto3. This is legacy // behavior that will be removed in a later version. - CheckFieldJsonNameUniqueness(message_name, proto, result, syntax, false); + CheckFieldJsonNameUniqueness(message_name, proto, result, false); } } else { // Check both with and without taking json_name into consideration. This is // needed for field masks, which don't use json_name. - CheckFieldJsonNameUniqueness(message_name, proto, result, syntax, false); - CheckFieldJsonNameUniqueness(message_name, proto, result, syntax, true); + CheckFieldJsonNameUniqueness(message_name, proto, result, false); + CheckFieldJsonNameUniqueness(message_name, proto, result, true); } } @@ -6260,8 +6247,7 @@ bool JsonNameLooksLikeExtension(std::string name) { void DescriptorBuilder::CheckFieldJsonNameUniqueness( const std::string& message_name, const DescriptorProto& message, - const Descriptor* descriptor, FileDescriptorLegacy::Syntax syntax, - bool use_custom_names) { + const Descriptor* descriptor, bool use_custom_names) { absl::flat_hash_map name_to_field; for (const FieldDescriptorProto& field : message.field()) { JsonNameDetails details = GetJsonNameDetails(&field, use_custom_names); @@ -6345,9 +6331,7 @@ void DescriptorBuilder::BuildFieldOrExtension(const FieldDescriptorProto& proto, result->is_oneof_ = false; result->proto3_optional_ = proto.proto3_optional(); - if (proto.proto3_optional() && - FileDescriptorLegacy(file_).syntax() != - FileDescriptorLegacy::Syntax::SYNTAX_PROTO3) { + if (proto.proto3_optional() && file_->edition() != Edition::EDITION_PROTO3) { AddError(result->full_name(), proto, DescriptorPool::ErrorCollector::TYPE, [&] { return absl::StrCat( @@ -6747,8 +6731,7 @@ void DescriptorBuilder::CheckEnumValueUniqueness( // compatibility we issue only a warning for proto2. if ((pool_->deprecated_legacy_json_field_conflicts_ || IsLegacyJsonFieldConflictEnabled(result->options())) && - FileDescriptorLegacy(result->file()).syntax() == - FileDescriptorLegacy::Syntax::SYNTAX_PROTO2) { + result->file()->edition() == Edition::EDITION_PROTO2) { AddWarning(value->full_name(), proto.value(i), DescriptorPool::ErrorCollector::NAME, make_error); continue; @@ -7095,7 +7078,7 @@ void DescriptorBuilder::CrossLinkMessage(Descriptor* message, const FieldDescriptor* field = message->field(i); if (field->proto3_optional_) { if (!field->containing_oneof() || - !OneofDescriptorLegacy(field->containing_oneof()).is_synthetic()) { + !field->containing_oneof()->is_synthetic()) { AddError(message->full_name(), proto.field(i), DescriptorPool::ErrorCollector::OTHER, "Fields with proto3_optional set must be " @@ -7107,8 +7090,7 @@ void DescriptorBuilder::CrossLinkMessage(Descriptor* message, // Synthetic oneofs must be last. int first_synthetic = -1; for (int i = 0; i < message->oneof_decl_count(); i++) { - const OneofDescriptor* oneof = message->oneof_decl(i); - if (OneofDescriptorLegacy(oneof).is_synthetic()) { + if (message->oneof_decl(i)->is_synthetic()) { if (first_synthetic == -1) { first_synthetic = i; } @@ -7672,8 +7654,7 @@ void DescriptorBuilder::ValidateOptions(const FileDescriptor* file, } } } - if (FileDescriptorLegacy(file).syntax() == - FileDescriptorLegacy::Syntax::SYNTAX_PROTO3) { + if (file->edition() == Edition::EDITION_PROTO3) { ValidateProto3(file, proto); } } @@ -7915,8 +7896,7 @@ static bool IsStringMapType(const FieldDescriptor& field) { void DescriptorBuilder::ValidateFileFeatures(const FileDescriptor* file, const FileDescriptorProto& proto) { // Rely on our legacy validation for proto2/proto3 files. - if (FileDescriptorLegacy(file).syntax() != - FileDescriptorLegacy::SYNTAX_EDITIONS) { + if (IsLegacyEdition(file)) { return; } @@ -7935,8 +7915,7 @@ void DescriptorBuilder::ValidateFileFeatures(const FileDescriptor* file, void DescriptorBuilder::ValidateFieldFeatures( const FieldDescriptor* field, const FieldDescriptorProto& proto) { // Rely on our legacy validation for proto2/proto3 files. - if (FileDescriptorLegacy(field->file()).syntax() != - FileDescriptorLegacy::SYNTAX_EDITIONS) { + if (field->file()->edition() < Edition::EDITION_2023) { return; } diff --git a/src/google/protobuf/descriptor.h b/src/google/protobuf/descriptor.h index c9bd246bfbb6..4470ffe5ee42 100644 --- a/src/google/protobuf/descriptor.h +++ b/src/google/protobuf/descriptor.h @@ -1250,6 +1250,7 @@ class PROTOBUF_EXPORT OneofDescriptor : private internal::SymbolBase { friend class DescriptorBuilder; friend class Descriptor; friend class FieldDescriptor; + friend class Reflection; }; PROTOBUF_INTERNAL_CHECK_CLASS_SIZE(OneofDescriptor, 56); diff --git a/src/google/protobuf/descriptor_legacy.h b/src/google/protobuf/descriptor_legacy.h index e7a1f7c3640e..463e8f615526 100644 --- a/src/google/protobuf/descriptor_legacy.h +++ b/src/google/protobuf/descriptor_legacy.h @@ -46,7 +46,7 @@ namespace protobuf { PROTOBUF_IGNORE_DEPRECATION_START // Wraps FileDescriptor. -class PROTOBUF_EXPORT FileDescriptorLegacy { +class FileDescriptorLegacy { public: explicit FileDescriptorLegacy(const FileDescriptor* desc) : desc_(desc) {} @@ -68,7 +68,7 @@ class PROTOBUF_EXPORT FileDescriptorLegacy { const FileDescriptor* desc_; }; -class PROTOBUF_EXPORT FieldDescriptorLegacy { +class FieldDescriptorLegacy { public: explicit FieldDescriptorLegacy(const FieldDescriptor* desc) : desc_(desc) {} @@ -78,7 +78,7 @@ class PROTOBUF_EXPORT FieldDescriptorLegacy { const FieldDescriptor* desc_; }; -class PROTOBUF_EXPORT OneofDescriptorLegacy { +class OneofDescriptorLegacy { public: explicit OneofDescriptorLegacy(const OneofDescriptor* desc) : desc_(desc) {} diff --git a/src/google/protobuf/descriptor_unittest.cc b/src/google/protobuf/descriptor_unittest.cc index 843b181143c4..703fb77861f5 100644 --- a/src/google/protobuf/descriptor_unittest.cc +++ b/src/google/protobuf/descriptor_unittest.cc @@ -47,7 +47,6 @@ #include "google/protobuf/compiler/parser.h" #include "google/protobuf/cpp_features.pb.h" #include "google/protobuf/descriptor_database.h" -#include "google/protobuf/descriptor_legacy.h" #include "google/protobuf/dynamic_message.h" #include "google/protobuf/feature_resolver.h" #include "google/protobuf/io/tokenizer.h" @@ -473,32 +472,30 @@ TEST_F(FileDescriptorTest, BuildAgainWithSyntax) { EXPECT_EQ(proto3_descriptor, pool_.BuildFile(proto_syntax3)); } -TEST_F(FileDescriptorTest, Syntax) { +TEST_F(FileDescriptorTest, Edition) { FileDescriptorProto proto; proto.set_name("foo"); - // Enable the test when we also populate the syntax for proto2. -#if 0 { proto.set_syntax("proto2"); DescriptorPool pool; const FileDescriptor* file = pool.BuildFile(proto); ASSERT_TRUE(file != nullptr); - EXPECT_EQ(FileDescriptorLegacy::Syntax::SYNTAX_PROTO2, FileDescriptorLegacy(file).syntax()); + EXPECT_EQ(file->edition(), Edition::EDITION_PROTO2); FileDescriptorProto other; file->CopyTo(&other); - EXPECT_EQ("proto2", other.syntax()); + EXPECT_EQ("", other.syntax()); + EXPECT_FALSE(other.has_edition()); } -#endif { proto.set_syntax("proto3"); DescriptorPool pool; const FileDescriptor* file = pool.BuildFile(proto); ASSERT_TRUE(file != nullptr); - EXPECT_EQ(FileDescriptorLegacy::Syntax::SYNTAX_PROTO3, - FileDescriptorLegacy(file).syntax()); + EXPECT_EQ(file->edition(), Edition::EDITION_PROTO3); FileDescriptorProto other; file->CopyTo(&other); EXPECT_EQ("proto3", other.syntax()); + EXPECT_FALSE(other.has_edition()); } { proto.set_syntax("editions"); @@ -506,8 +503,7 @@ TEST_F(FileDescriptorTest, Syntax) { DescriptorPool pool; const FileDescriptor* file = pool.BuildFile(proto); ASSERT_TRUE(file != nullptr); - EXPECT_EQ(FileDescriptorLegacy::Syntax::SYNTAX_EDITIONS, - FileDescriptorLegacy(file).syntax()); + EXPECT_EQ(file->edition(), Edition::EDITION_2023); EXPECT_EQ(file->edition(), EDITION_2023); FileDescriptorProto other; file->CopyTo(&other); diff --git a/src/google/protobuf/dynamic_message.cc b/src/google/protobuf/dynamic_message.cc index 1dcbe30c4de2..d1f6ca6b7b0f 100644 --- a/src/google/protobuf/dynamic_message.cc +++ b/src/google/protobuf/dynamic_message.cc @@ -49,7 +49,6 @@ #include "google/protobuf/arenastring.h" #include "google/protobuf/descriptor.h" #include "google/protobuf/descriptor.pb.h" -#include "google/protobuf/descriptor_legacy.h" #include "google/protobuf/extension_set.h" #include "google/protobuf/generated_message_reflection.h" #include "google/protobuf/generated_message_util.h" @@ -84,8 +83,7 @@ bool IsMapFieldInApi(const FieldDescriptor* field) { return field->is_map(); } inline bool InRealOneof(const FieldDescriptor* field) { - return field->containing_oneof() && - !OneofDescriptorLegacy(field->containing_oneof()).is_synthetic(); + return field->real_containing_oneof() != nullptr; } // Compute the byte size of the in-memory representation of the field. @@ -354,9 +352,7 @@ void DynamicMessage::SharedCtor(bool lock_factory) { Arena* arena = GetArena(); // Initialize oneof cases. int oneof_count = 0; - for (int i = 0; i < descriptor->oneof_decl_count(); ++i) { - if (OneofDescriptorLegacy(descriptor->oneof_decl(i)).is_synthetic()) - continue; + for (int i = 0; i < descriptor->real_oneof_decl_count(); ++i) { new (MutableOneofCaseRaw(oneof_count++)) uint32_t{0}; } @@ -652,12 +648,7 @@ const Message* DynamicMessageFactory::GetPrototypeNoLock( // this block. // - A big bitfield containing a bit for each field indicating whether // or not that field is set. - int real_oneof_count = 0; - for (int i = 0; i < type->oneof_decl_count(); i++) { - if (!OneofDescriptorLegacy(type->oneof_decl(i)).is_synthetic()) { - real_oneof_count++; - } - } + int real_oneof_count = type->real_oneof_decl_count(); // Compute size and offsets. uint32_t* offsets = new uint32_t[type->field_count() + real_oneof_count]; @@ -727,12 +718,10 @@ const Message* DynamicMessageFactory::GetPrototypeNoLock( } // The oneofs. - for (int i = 0; i < type->oneof_decl_count(); i++) { - if (!OneofDescriptorLegacy(type->oneof_decl(i)).is_synthetic()) { - size = AlignTo(size, kSafeAlignment); - offsets[type->field_count() + i] = size; - size += kMaxOneofUnionSize; - } + for (int i = 0; i < type->real_oneof_decl_count(); i++) { + size = AlignTo(size, kSafeAlignment); + offsets[type->field_count() + i] = size; + size += kMaxOneofUnionSize; } type_info->weak_field_map_offset = -1; @@ -745,10 +734,9 @@ const Message* DynamicMessageFactory::GetPrototypeNoLock( // Compute the size of default oneof instance and offsets of default // oneof fields. - for (int i = 0; i < type->oneof_decl_count(); i++) { - if (OneofDescriptorLegacy(type->oneof_decl(i)).is_synthetic()) continue; - for (int j = 0; j < type->oneof_decl(i)->field_count(); j++) { - const FieldDescriptor* field = type->oneof_decl(i)->field(j); + for (int i = 0; i < type->real_oneof_decl_count(); i++) { + for (int j = 0; j < type->real_oneof_decl(i)->field_count(); j++) { + const FieldDescriptor* field = type->real_oneof_decl(i)->field(j); // oneof fields are not accessed through offsets, but we still have the // entry from a legacy implementation. This should be removed at some // point. diff --git a/src/google/protobuf/generated_message_reflection.cc b/src/google/protobuf/generated_message_reflection.cc index 288d60f1fe2c..647b4cc747b8 100644 --- a/src/google/protobuf/generated_message_reflection.cc +++ b/src/google/protobuf/generated_message_reflection.cc @@ -30,7 +30,6 @@ #include "absl/synchronization/mutex.h" #include "google/protobuf/descriptor.h" #include "google/protobuf/descriptor.pb.h" -#include "google/protobuf/descriptor_legacy.h" #include "google/protobuf/extension_set.h" #include "google/protobuf/generated_message_tctable_decl.h" #include "google/protobuf/generated_message_tctable_gen.h" @@ -1008,7 +1007,7 @@ void Reflection::SwapOneofField(Message* lhs, Message* rhs, const FieldDescriptor* field; }; - ABSL_DCHECK(!OneofDescriptorLegacy(oneof_descriptor).is_synthetic()); + ABSL_DCHECK(!oneof_descriptor->is_synthetic()); uint32_t oneof_case_lhs = GetOneofCase(*lhs, oneof_descriptor); uint32_t oneof_case_rhs = GetOneofCase(*rhs, oneof_descriptor); @@ -1230,12 +1229,10 @@ void Reflection::InternalSwap(Message* lhs, Message* rhs) const { if (schema_.IsSplit()) { std::swap(*MutableSplitField(lhs), *MutableSplitField(rhs)); } - const int oneof_decl_count = descriptor_->oneof_decl_count(); + const int oneof_decl_count = descriptor_->real_oneof_decl_count(); for (int i = 0; i < oneof_decl_count; i++) { - const OneofDescriptor* oneof = descriptor_->oneof_decl(i); - if (!OneofDescriptorLegacy(oneof).is_synthetic()) { - SwapOneofField(lhs, rhs, oneof); - } + const OneofDescriptor* oneof = descriptor_->real_oneof_decl(i); + SwapOneofField(lhs, rhs, oneof); } // Swapping bits need to happen after swapping fields, because the latter may @@ -2574,7 +2571,7 @@ const void* Reflection::GetRawRepeatedField(const Message& message, const FieldDescriptor* Reflection::GetOneofFieldDescriptor( const Message& message, const OneofDescriptor* oneof_descriptor) const { - if (OneofDescriptorLegacy(oneof_descriptor).is_synthetic()) { + if (oneof_descriptor->is_synthetic()) { const FieldDescriptor* field = oneof_descriptor->field(0); return HasField(message, field) ? field : nullptr; } @@ -2754,14 +2751,14 @@ uint32_t* Reflection::MutableHasBits(Message* message) const { uint32_t Reflection::GetOneofCase( const Message& message, const OneofDescriptor* oneof_descriptor) const { - ABSL_DCHECK(!OneofDescriptorLegacy(oneof_descriptor).is_synthetic()); + ABSL_DCHECK(!oneof_descriptor->is_synthetic()); return internal::GetConstRefAtOffset( message, schema_.GetOneofCaseOffset(oneof_descriptor)); } uint32_t* Reflection::MutableOneofCase( Message* message, const OneofDescriptor* oneof_descriptor) const { - ABSL_DCHECK(!OneofDescriptorLegacy(oneof_descriptor).is_synthetic()); + ABSL_DCHECK(!oneof_descriptor->is_synthetic()); return GetPointerAtOffset( message, schema_.GetOneofCaseOffset(oneof_descriptor)); } @@ -2955,7 +2952,7 @@ void Reflection::SwapBit(Message* message1, Message* message2, bool Reflection::HasOneof(const Message& message, const OneofDescriptor* oneof_descriptor) const { - if (OneofDescriptorLegacy(oneof_descriptor).is_synthetic()) { + if (oneof_descriptor->is_synthetic()) { return HasField(message, oneof_descriptor->field(0)); } return (GetOneofCase(message, oneof_descriptor) > 0); @@ -2975,7 +2972,7 @@ void Reflection::ClearOneofField(Message* message, void Reflection::ClearOneof(Message* message, const OneofDescriptor* oneof_descriptor) const { - if (OneofDescriptorLegacy(oneof_descriptor).is_synthetic()) { + if (oneof_descriptor->is_synthetic()) { ClearField(message, oneof_descriptor->field(0)); return; } diff --git a/src/google/protobuf/proto3_arena_unittest.cc b/src/google/protobuf/proto3_arena_unittest.cc index 3ada399c595b..87088158a5ac 100644 --- a/src/google/protobuf/proto3_arena_unittest.cc +++ b/src/google/protobuf/proto3_arena_unittest.cc @@ -12,7 +12,6 @@ #include #include "absl/strings/match.h" #include "google/protobuf/arena.h" -#include "google/protobuf/descriptor_legacy.h" #include "google/protobuf/test_util.h" #include "google/protobuf/text_format.h" #include "google/protobuf/unittest.pb.h" @@ -262,15 +261,13 @@ TEST(Proto3OptionalTest, OptionalFieldDescriptor) { for (int i = 0; i < d->field_count(); i++) { const FieldDescriptor* f = d->field(i); if (absl::StartsWith(f->name(), "singular")) { - EXPECT_FALSE(FieldDescriptorLegacy(f).has_optional_keyword()) - << f->full_name(); EXPECT_FALSE(f->has_presence()) << f->full_name(); EXPECT_FALSE(f->containing_oneof()) << f->full_name(); + EXPECT_FALSE(f->real_containing_oneof()) << f->full_name(); } else { - EXPECT_TRUE(FieldDescriptorLegacy(f).has_optional_keyword()) - << f->full_name(); EXPECT_TRUE(f->has_presence()) << f->full_name(); EXPECT_TRUE(f->containing_oneof()) << f->full_name(); + EXPECT_FALSE(f->real_containing_oneof()) << f->full_name(); } } } @@ -283,8 +280,6 @@ TEST(Proto3OptionalTest, Extensions) { "protobuf_unittest.Proto3OptionalExtensions.ext_with_optional"); ABSL_CHECK(no_optional); ABSL_CHECK(with_optional); - EXPECT_FALSE(FieldDescriptorLegacy(no_optional).has_optional_keyword()); - EXPECT_TRUE(FieldDescriptorLegacy(with_optional).has_optional_keyword()); const Descriptor* d = protobuf_unittest::Proto3OptionalExtensions::descriptor(); EXPECT_TRUE(d->options().HasExtension( @@ -332,7 +327,8 @@ TEST(Proto3OptionalTest, OptionalFieldReflection) { const google::protobuf::OneofDescriptor* o = d->FindOneofByName("_optional_int32"); ABSL_CHECK(f); ABSL_CHECK(o); - EXPECT_TRUE(OneofDescriptorLegacy(o).is_synthetic()); + EXPECT_EQ(f->containing_oneof(), o); + EXPECT_EQ(f->real_containing_oneof(), nullptr); EXPECT_FALSE(r->HasField(msg, f)); EXPECT_FALSE(r->HasOneof(msg, o)); diff --git a/src/google/protobuf/util/BUILD.bazel b/src/google/protobuf/util/BUILD.bazel index 35ede30466b3..149b86c3517f 100644 --- a/src/google/protobuf/util/BUILD.bazel +++ b/src/google/protobuf/util/BUILD.bazel @@ -170,7 +170,6 @@ cc_library( visibility = ["//:__subpackages__"], deps = [ "//src/google/protobuf", - "//src/google/protobuf:descriptor_legacy", "//src/google/protobuf/io", "//src/google/protobuf/stubs", "@com_google_absl//absl/log:absl_log", diff --git a/src/google/protobuf/util/type_resolver_util.cc b/src/google/protobuf/util/type_resolver_util.cc index 1bff19088666..078e7f0535b2 100644 --- a/src/google/protobuf/util/type_resolver_util.cc +++ b/src/google/protobuf/util/type_resolver_util.cc @@ -20,7 +20,6 @@ #include "absl/strings/str_cat.h" #include "absl/strings/string_view.h" #include "absl/strings/strip.h" -#include "google/protobuf/descriptor_legacy.h" #include "google/protobuf/io/strtod.h" #include "google/protobuf/util/type_resolver.h" @@ -251,17 +250,17 @@ void ConvertFieldDescriptor(absl::string_view url_prefix, ConvertFieldOptions(descriptor.options(), *field->mutable_options()); } -Syntax ConvertSyntax(FileDescriptorLegacy::Syntax syntax) { - switch (syntax) { - default: - return Syntax::SYNTAX_PROTO2; +Syntax ConvertSyntax(Edition edition) { + if (edition >= Edition::EDITION_2023) { + return Syntax::SYNTAX_EDITIONS; } + // TODO This should propagate proto3 as expected. + return Syntax::SYNTAX_PROTO2; } void ConvertEnumDescriptor(const EnumDescriptor& descriptor, Enum* enum_type) { enum_type->Clear(); - enum_type->set_syntax( - ConvertSyntax(FileDescriptorLegacy(descriptor.file()).syntax())); + enum_type->set_syntax(ConvertSyntax(descriptor.file()->edition())); enum_type->set_name(descriptor.full_name()); enum_type->mutable_source_context()->set_file_name(descriptor.file()->name()); @@ -282,8 +281,7 @@ void ConvertDescriptor(absl::string_view url_prefix, const Descriptor& descriptor, Type* type) { type->Clear(); type->set_name(descriptor.full_name()); - type->set_syntax( - ConvertSyntax(FileDescriptorLegacy(descriptor.file()).syntax())); + type->set_syntax(ConvertSyntax(descriptor.file()->edition())); for (int i = 0; i < descriptor.field_count(); ++i) { ConvertFieldDescriptor(url_prefix, *descriptor.field(i), type->add_fields()); diff --git a/src/google/protobuf/util/type_resolver_util_test.cc b/src/google/protobuf/util/type_resolver_util_test.cc index e79f2d911d87..dae2728e9be5 100644 --- a/src/google/protobuf/util/type_resolver_util_test.cc +++ b/src/google/protobuf/util/type_resolver_util_test.cc @@ -21,7 +21,6 @@ #include #include #include "google/protobuf/descriptor.h" -#include "google/protobuf/descriptor_legacy.h" #include "google/protobuf/util/json_format_proto3.pb.h" #include "google/protobuf/map_unittest.pb.h" #include "google/protobuf/unittest.pb.h" @@ -444,8 +443,7 @@ class DescriptorPoolTypeResolverSyntaxTest : public testing::Test { TEST_F(DescriptorPoolTypeResolverSyntaxTest, SyntaxProto2) { const FileDescriptor* file = BuildFile("proto2"); - ASSERT_EQ(FileDescriptorLegacy::Syntax::SYNTAX_PROTO2, - FileDescriptorLegacy(file).syntax()); + ASSERT_EQ(file->edition(), Edition::EDITION_PROTO2); Type type; ASSERT_TRUE( @@ -456,8 +454,7 @@ TEST_F(DescriptorPoolTypeResolverSyntaxTest, SyntaxProto2) { TEST_F(DescriptorPoolTypeResolverSyntaxTest, SyntaxProto3) { const FileDescriptor* file = BuildFile("proto3"); - ASSERT_EQ(FileDescriptorLegacy::Syntax::SYNTAX_PROTO3, - FileDescriptorLegacy(file).syntax()); + ASSERT_EQ(file->edition(), Edition::EDITION_PROTO3); Type type; ASSERT_TRUE(