From fb9fdf01ac4d9a7036d476ed0c40f968d1900da8 Mon Sep 17 00:00:00 2001 From: Mike Kruskal Date: Tue, 26 Dec 2023 20:23:01 -0800 Subject: [PATCH] Remove edition getter from C++ descriptor APIs PiperOrigin-RevId: 593909007 --- conformance/BUILD.bazel | 1 + conformance/conformance_test.cc | 4 +- pkg/BUILD.bazel | 1 + python/descriptor.c | 7 -- python/google/protobuf/descriptor.py | 7 -- .../protobuf/internal/descriptor_test.py | 3 - python/google/protobuf/pyext/descriptor.cc | 5 -- src/google/protobuf/BUILD.bazel | 13 +++ src/google/protobuf/compiler/code_generator.h | 5 ++ .../compiler/command_line_interface.cc | 22 +++-- src/google/protobuf/compiler/java/generator.h | 1 + .../protobuf/compiler/java/message_lite.cc | 7 +- .../protobuf/compiler/mock_code_generator.cc | 2 +- .../protobuf/compiler/objectivec/file.cc | 7 +- .../protobuf/compiler/objectivec/file.h | 3 +- .../protobuf/compiler/objectivec/generator.cc | 3 +- .../protobuf/compiler/python/generator.cc | 4 +- src/google/protobuf/descriptor.cc | 86 +++++++++---------- src/google/protobuf/descriptor.h | 17 ++-- src/google/protobuf/descriptor_legacy.h | 25 ++++++ src/google/protobuf/descriptor_unittest.cc | 8 +- src/google/protobuf/util/BUILD.bazel | 2 + .../protobuf/util/type_resolver_util.cc | 7 +- .../protobuf/util/type_resolver_util_test.cc | 5 +- 24 files changed, 143 insertions(+), 102 deletions(-) create mode 100644 src/google/protobuf/descriptor_legacy.h diff --git a/conformance/BUILD.bazel b/conformance/BUILD.bazel index 1d9ea1585c73..171422c3822e 100644 --- a/conformance/BUILD.bazel +++ b/conformance/BUILD.bazel @@ -144,6 +144,7 @@ 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 ef049c8a539d..2d7dfcd344b4 100644 --- a/conformance/conformance_test.cc +++ b/conformance/conformance_test.cc @@ -24,6 +24,7 @@ #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" @@ -142,7 +143,8 @@ ConformanceTestSuite::ConformanceRequestSetting::NewTestMessage() const { std::string ConformanceTestSuite::ConformanceRequestSetting::GetSyntaxIdentifier() const { - switch (prototype_message_.GetDescriptor()->file()->edition()) { + switch (FileDescriptorLegacy(prototype_message_.GetDescriptor()->file()) + .edition()) { case Edition::EDITION_PROTO3: return "Proto3"; case Edition::EDITION_PROTO2: diff --git a/pkg/BUILD.bazel b/pkg/BUILD.bazel index 55079a67e253..4fcae93d1b00 100644 --- a/pkg/BUILD.bazel +++ b/pkg/BUILD.bazel @@ -237,6 +237,7 @@ cc_dist_library( ], tags = ["manual"], deps = [ + "//src/google/protobuf:descriptor_legacy", "//src/google/protobuf:internal_visibility_for_testing", "//src/google/protobuf:test_textproto", "//src/google/protobuf/compiler:command_line_interface_tester", diff --git a/python/descriptor.c b/python/descriptor.c index c5af6dd90e1e..29ba6b304787 100644 --- a/python/descriptor.c +++ b/python/descriptor.c @@ -1367,12 +1367,6 @@ static PyObject* PyUpb_FileDescriptor_GetPublicDependencies(PyObject* _self, return PyUpb_GenericSequence_New(&funcs, self->def, self->pool); } -static PyObject* PyUpb_FileDescriptor_GetEdition(PyObject* _self, - void* closure) { - PyUpb_DescriptorBase* self = (void*)_self; - return PyLong_FromLong(upb_FileDef_Edition(self->def)); -} - static PyObject* PyUpb_FileDescriptor_GetHasOptions(PyObject* _self, void* closure) { PyUpb_DescriptorBase* self = (void*)_self; @@ -1421,7 +1415,6 @@ static PyGetSetDef PyUpb_FileDescriptor_Getters[] = { {"public_dependencies", PyUpb_FileDescriptor_GetPublicDependencies, NULL, "Dependencies"}, {"has_options", PyUpb_FileDescriptor_GetHasOptions, NULL, "Has Options"}, - {"edition", PyUpb_FileDescriptor_GetEdition, (setter)NULL, "Edition"}, {NULL}, }; diff --git a/python/google/protobuf/descriptor.py b/python/google/protobuf/descriptor.py index 29885ff39587..a1b94b3c660b 100755 --- a/python/google/protobuf/descriptor.py +++ b/python/google/protobuf/descriptor.py @@ -1279,13 +1279,6 @@ def CopyToProto(self, proto): def _parent(self): return None - @property - def edition(self): - # pylint: disable=g-import-not-at-top - from google.protobuf import descriptor_pb2 - - return descriptor_pb2.Edition.Value(self._edition) - def _ParseOptions(message, string): """Parses serialized options. diff --git a/python/google/protobuf/internal/descriptor_test.py b/python/google/protobuf/internal/descriptor_test.py index bf833a4b8ac4..c561a0411c2a 100755 --- a/python/google/protobuf/internal/descriptor_test.py +++ b/python/google/protobuf/internal/descriptor_test.py @@ -539,9 +539,6 @@ def testFileDescriptor(self): self.assertEqual(self.my_file.package, 'protobuf_unittest') self.assertEqual(self.my_file.pool, self.pool) self.assertFalse(self.my_file.has_options) - self.assertEqual( - self.my_file.edition, descriptor_pb2.Edition.EDITION_PROTO2 - ) file_proto = descriptor_pb2.FileDescriptorProto() self.my_file.CopyToProto(file_proto) self.assertEqual(self.my_file.serialized_pb, diff --git a/python/google/protobuf/pyext/descriptor.cc b/python/google/protobuf/pyext/descriptor.cc index 2c5bd1a0fe06..a4a323adfa0a 100644 --- a/python/google/protobuf/pyext/descriptor.cc +++ b/python/google/protobuf/pyext/descriptor.cc @@ -1529,10 +1529,6 @@ static int SetSerializedOptions(PyFileDescriptor *self, PyObject *value, return CheckCalledFromGeneratedFile("_serialized_options"); } -static PyObject* GetEdition(PyFileDescriptor* self, void* closure) { - return PyLong_FromLong(_GetDescriptor(self)->edition()); -} - static PyObject* CopyToProto(PyFileDescriptor *self, PyObject *target) { return CopyToPythonProto(_GetDescriptor(self), target); } @@ -1559,7 +1555,6 @@ static PyGetSetDef Getters[] = { {"_options", (getter) nullptr, (setter)SetOptions, "Options"}, {"_serialized_options", (getter) nullptr, (setter)SetSerializedOptions, "Serialized Options"}, - {"edition", (getter)GetEdition, (setter) nullptr, "Edition"}, {nullptr}, }; diff --git a/src/google/protobuf/BUILD.bazel b/src/google/protobuf/BUILD.bazel index 5e7f5479c58e..4b1b72655c29 100644 --- a/src/google/protobuf/BUILD.bazel +++ b/src/google/protobuf/BUILD.bazel @@ -593,6 +593,18 @@ cc_library( ], ) +cc_library( + name = "descriptor_legacy", + hdrs = ["descriptor_legacy.h"], + copts = COPTS, + linkopts = LINK_OPTS, + strip_include_prefix = "/src", + visibility = ["//:__subpackages__"], + deps = [ + ":protobuf_nowkt", + ], +) + filegroup( name = "well_known_type_protos", srcs = [ @@ -1030,6 +1042,7 @@ cc_test( }), deps = [ ":cc_test_protos", + ":descriptor_legacy", ":protobuf", ":test_textproto", "//src/google/protobuf/compiler:importer", diff --git a/src/google/protobuf/compiler/code_generator.h b/src/google/protobuf/compiler/code_generator.h index 7ce2cfba7265..a41bddd3673e 100644 --- a/src/google/protobuf/compiler/code_generator.h +++ b/src/google/protobuf/compiler/code_generator.h @@ -156,6 +156,11 @@ class PROTOC_EXPORT CodeGenerator { return ::google::protobuf::internal::InternalFeatureHelper::GetUnresolvedFeatures( descriptor, extension); } + + // Retrieves the edition of a built file descriptor. + static Edition GetEdition(const FileDescriptor& file) { + return ::google::protobuf::internal::InternalFeatureHelper::GetEdition(file); + } }; // CodeGenerators generate one or more files in a given directory. This diff --git a/src/google/protobuf/compiler/command_line_interface.cc b/src/google/protobuf/compiler/command_line_interface.cc index e879f149bda3..c011bd7b68c1 100644 --- a/src/google/protobuf/compiler/command_line_interface.cc +++ b/src/google/protobuf/compiler/command_line_interface.cc @@ -991,8 +991,8 @@ bool ContainsProto3Optional(const Descriptor* desc) { return false; } -bool ContainsProto3Optional(const FileDescriptor* file) { - if (file->edition() == Edition::EDITION_PROTO3) { +bool ContainsProto3Optional(Edition edition, const FileDescriptor* file) { + if (edition == Edition::EDITION_PROTO3) { for (int i = 0; i < file->message_type_count(); i++) { if (ContainsProto3Optional(file->message_type(i))) { return true; @@ -1600,7 +1600,8 @@ bool CommandLineInterface::ParseInputFiles( if (!experimental_editions_ && !absl::StartsWith(parsed_file->name(), "google/protobuf/") && !absl::StartsWith(parsed_file->name(), "upb/")) { - if (parsed_file->edition() >= Edition::EDITION_2023) { + if (::google::protobuf::internal::InternalFeatureHelper::GetEdition(*parsed_file) >= + Edition::EDITION_2023) { std::cerr << parsed_file->name() << ": This file uses editions, but --experimental_editions has not " @@ -2533,7 +2534,8 @@ bool CommandLineInterface::EnforceProto3OptionalSupport( supported_features & CodeGenerator::FEATURE_PROTO3_OPTIONAL; if (!supports_proto3_optional) { for (const auto fd : parsed_files) { - if (ContainsProto3Optional(fd)) { + if (ContainsProto3Optional( + ::google::protobuf::internal::InternalFeatureHelper::GetEdition(*fd), fd)) { std::cerr << fd->name() << ": is a proto3 file that contains optional fields, but " "code generator " @@ -2558,7 +2560,9 @@ bool CommandLineInterface::EnforceEditionsSupport( return true; } for (const auto* fd : parsed_files) { - if (fd->edition() < Edition::EDITION_2023) { + Edition edition = + ::google::protobuf::internal::InternalFeatureHelper::GetEdition(*fd); + if (edition < Edition::EDITION_2023) { // Legacy proto2/proto3 files don't need any checks. continue; } @@ -2576,19 +2580,19 @@ bool CommandLineInterface::EnforceEditionsSupport( fd->name(), codegen_name); return false; } - if (fd->edition() < minimum_edition) { + if (edition < minimum_edition) { std::cerr << absl::Substitute( "$0: is a file using edition $2, which isn't supported by code " "generator $1. Please upgrade your file to at least edition $3.", - fd->name(), codegen_name, fd->edition(), minimum_edition); + fd->name(), codegen_name, edition, minimum_edition); return false; } - if (fd->edition() > maximum_edition) { + if (edition > maximum_edition) { std::cerr << absl::Substitute( "$0: is a file using edition $2, which isn't supported by code " "generator $1. Please ask the owner of this code generator to add " "support or switch back to a maximum of edition $3.", - fd->name(), codegen_name, fd->edition(), maximum_edition); + fd->name(), codegen_name, edition, maximum_edition); return false; } } diff --git a/src/google/protobuf/compiler/java/generator.h b/src/google/protobuf/compiler/java/generator.h index 549d0e5ab640..bd510c699529 100644 --- a/src/google/protobuf/compiler/java/generator.h +++ b/src/google/protobuf/compiler/java/generator.h @@ -58,6 +58,7 @@ class PROTOC_EXPORT JavaGenerator : public CodeGenerator { opensource_runtime_ = opensource; } + using CodeGenerator::GetEdition; using CodeGenerator::GetResolvedSourceFeatures; private: diff --git a/src/google/protobuf/compiler/java/message_lite.cc b/src/google/protobuf/compiler/java/message_lite.cc index 71b368997d11..0eb672d09b90 100644 --- a/src/google/protobuf/compiler/java/message_lite.cc +++ b/src/google/protobuf/compiler/java/message_lite.cc @@ -25,6 +25,7 @@ #include "google/protobuf/compiler/java/doc_comment.h" #include "google/protobuf/compiler/java/enum_lite.h" #include "google/protobuf/compiler/java/extension_lite.h" +#include "google/protobuf/compiler/java/generator.h" #include "google/protobuf/compiler/java/generator_factory.h" #include "google/protobuf/compiler/java/helpers.h" #include "google/protobuf/compiler/java/message_builder.h" @@ -476,9 +477,11 @@ void ImmutableMessageLiteGenerator::GenerateDynamicMethodNewBuildMessageInfo( flags |= 0x2; } if (!context_->options().strip_nonfunctional_codegen) { - if (descriptor_->file()->edition() == Edition::EDITION_PROTO2) { + if (JavaGenerator::GetEdition(*descriptor_->file()) == + Edition::EDITION_PROTO2) { flags |= 0x1; - } else if (descriptor_->file()->edition() >= Edition::EDITION_2023) { + } else if (JavaGenerator::GetEdition(*descriptor_->file()) >= + Edition::EDITION_2023) { flags |= 0x4; } } diff --git a/src/google/protobuf/compiler/mock_code_generator.cc b/src/google/protobuf/compiler/mock_code_generator.cc index 0cf407de900f..c3291603dd53 100644 --- a/src/google/protobuf/compiler/mock_code_generator.cc +++ b/src/google/protobuf/compiler/mock_code_generator.cc @@ -215,7 +215,7 @@ bool MockCodeGenerator::Generate(const FileDescriptor* file, maximum_edition_ = Edition::EDITION_PROTO2; } - if (file->edition() >= Edition::EDITION_2023 && + if (GetEdition(*file) >= Edition::EDITION_2023 && (suppressed_features_ & CodeGenerator::FEATURE_SUPPORTS_EDITIONS) == 0) { internal::VisitDescriptors(*file, [&](const auto& descriptor) { const FeatureSet& features = GetResolvedSourceFeatures(descriptor); diff --git a/src/google/protobuf/compiler/objectivec/file.cc b/src/google/protobuf/compiler/objectivec/file.cc index eccec502a7d9..9066f63dcf4c 100644 --- a/src/google/protobuf/compiler/objectivec/file.cc +++ b/src/google/protobuf/compiler/objectivec/file.cc @@ -264,10 +264,11 @@ FileGenerator::CommonState::CollectMinimalFileDepsContainingExtensions( return result; } -FileGenerator::FileGenerator(const FileDescriptor* file, +FileGenerator::FileGenerator(Edition edition, const FileDescriptor* file, const GenerationOptions& generation_options, CommonState& common_state) - : file_(file), + : edition_(edition), + file_(file), generation_options_(generation_options), common_state_(&common_state), root_class_name_(FileClassName(file)), @@ -777,7 +778,7 @@ void FileGenerator::EmitFileDescription(io::Printer* p) const { // mode. syntax = "GPBFileSyntaxUnknown"; } else { - switch (file_->edition()) { + switch (edition_) { case Edition::EDITION_UNKNOWN: syntax = "GPBFileSyntaxUnknown"; break; diff --git a/src/google/protobuf/compiler/objectivec/file.h b/src/google/protobuf/compiler/objectivec/file.h index d560a93e58f9..baef5a0a2d88 100644 --- a/src/google/protobuf/compiler/objectivec/file.h +++ b/src/google/protobuf/compiler/objectivec/file.h @@ -55,7 +55,7 @@ class FileGenerator { const bool include_custom_options; }; - FileGenerator(const FileDescriptor* file, + FileGenerator(Edition edition, const FileDescriptor* file, const GenerationOptions& generation_options, CommonState& common_state); ~FileGenerator() = default; @@ -114,6 +114,7 @@ class FileGenerator { generation_options_.headers_use_forward_declarations; } + const Edition edition_; const FileDescriptor* file_; const GenerationOptions& generation_options_; mutable CommonState* common_state_; diff --git a/src/google/protobuf/compiler/objectivec/generator.cc b/src/google/protobuf/compiler/objectivec/generator.cc index 4f74006c76ca..1f7e1498171f 100644 --- a/src/google/protobuf/compiler/objectivec/generator.cc +++ b/src/google/protobuf/compiler/objectivec/generator.cc @@ -341,7 +341,8 @@ bool ObjectiveCGenerator::GenerateAll( FileGenerator::CommonState state(!generation_options.strip_custom_options); for (const auto& file : files) { - const FileGenerator file_generator(file, generation_options, state); + const FileGenerator file_generator(GetEdition(*file), file, + generation_options, state); std::string filepath = FilePath(file); // Generate header. diff --git a/src/google/protobuf/compiler/python/generator.cc b/src/google/protobuf/compiler/python/generator.cc index 555e4edef19f..575d628a5a1f 100644 --- a/src/google/protobuf/compiler/python/generator.cc +++ b/src/google/protobuf/compiler/python/generator.cc @@ -544,8 +544,8 @@ void Generator::PrintFileDescriptor() const { m["descriptor_name"] = kDescriptorKey; m["name"] = file_->name(); m["package"] = file_->package(); - m["syntax"] = GetLegacySyntaxName(file_->edition()); - m["edition"] = Edition_Name(file_->edition()); + m["syntax"] = GetLegacySyntaxName(GetEdition(*file_)); + m["edition"] = Edition_Name(GetEdition(*file_)); m["options"] = OptionsValue(proto_.options().SerializeAsString()); m["serialized_descriptor"] = absl::CHexEscape(file_descriptor_serialized_); if (GeneratingDescriptorProto()) { diff --git a/src/google/protobuf/descriptor.cc b/src/google/protobuf/descriptor.cc index ff373708b6d3..8c79546ae825 100644 --- a/src/google/protobuf/descriptor.cc +++ b/src/google/protobuf/descriptor.cc @@ -1164,22 +1164,8 @@ 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()); +bool IsLegacyEdition(Edition edition) { + return edition < Edition::EDITION_2023; } } // anonymous namespace @@ -2759,7 +2745,7 @@ void FileDescriptor::CopyHeadingTo(FileDescriptorProto* proto) const { if (edition() == Edition::EDITION_PROTO3) { proto->set_syntax("proto3"); - } else if (!IsLegacyEdition(this)) { + } else if (!IsLegacyEdition(edition())) { proto->set_syntax("editions"); proto->set_edition(edition()); } @@ -2857,7 +2843,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() && !IsLegacyEdition(this)) { + if (is_required() && !IsLegacyEdition(file()->edition())) { // Editions files have no required keyword, and we only set this label // during descriptor build. proto->set_label(static_cast( @@ -2866,7 +2852,7 @@ void FieldDescriptor::CopyTo(FieldDescriptorProto* proto) const { proto->set_label(static_cast( absl::implicit_cast(label()))); } - if (type() == TYPE_GROUP && !IsLegacyEdition(this)) { + if (type() == TYPE_GROUP && !IsLegacyEdition(file()->edition())) { // Editions files have no group keyword, and we only set this label // during descriptor build. proto->set_type(static_cast( @@ -3002,8 +2988,9 @@ void MethodDescriptor::CopyTo(MethodDescriptorProto* proto) const { namespace { -bool IsGroupSyntax(const FieldDescriptor* desc) { - return IsLegacyEdition(desc) && desc->type() == FieldDescriptor::TYPE_GROUP; +bool IsGroupSyntax(Edition edition, const FieldDescriptor* desc) { + return IsLegacyEdition(edition) && + desc->type() == FieldDescriptor::TYPE_GROUP; } template @@ -3117,8 +3104,8 @@ bool FormatLineOptions(int depth, const Message& options, return !all_options.empty(); } -static std::string GetLegacySyntaxName(const FileDescriptor* file) { - if (file->edition() == Edition::EDITION_PROTO3) { +static std::string GetLegacySyntaxName(Edition edition) { + if (edition == Edition::EDITION_PROTO3) { return "proto3"; } return "proto2"; @@ -3201,9 +3188,9 @@ std::string FileDescriptor::DebugStringWithOptions( SourceLocationCommentPrinter syntax_comment(this, path, "", debug_string_options); syntax_comment.AddPreComment(&contents); - if (IsLegacyEdition(this)) { + if (IsLegacyEdition(edition())) { absl::SubstituteAndAppend(&contents, "syntax = \"$0\";\n\n", - GetLegacySyntaxName(this)); + GetLegacySyntaxName(edition())); } else { absl::SubstituteAndAppend(&contents, "edition = \"$0\";\n\n", edition()); } @@ -3256,7 +3243,7 @@ std::string FileDescriptor::DebugStringWithOptions( // definitions (those will be done with their group field descriptor). absl::flat_hash_set groups; for (int i = 0; i < extension_count(); i++) { - if (IsGroupSyntax(extension(i))) { + if (IsGroupSyntax(edition(), extension(i))) { groups.insert(extension(i)->message_type()); } } @@ -3331,12 +3318,12 @@ void Descriptor::DebugString(int depth, std::string* contents, // descriptor). absl::flat_hash_set groups; for (int i = 0; i < field_count(); i++) { - if (IsGroupSyntax(field(i))) { + if (IsGroupSyntax(file()->edition(), field(i))) { groups.insert(field(i)->message_type()); } } for (int i = 0; i < extension_count(); i++) { - if (IsGroupSyntax(extension(i))) { + if (IsGroupSyntax(file()->edition(), extension(i))) { groups.insert(extension(i)->message_type()); } } @@ -3447,7 +3434,7 @@ std::string FieldDescriptor::FieldTypeNameDebugString() const { switch (type()) { case TYPE_MESSAGE: case TYPE_GROUP: - if (IsGroupSyntax(this)) { + if (IsGroupSyntax(file()->edition(), this)) { return kTypeToName[type()]; } return absl::StrCat(".", message_type()->full_name()); @@ -3482,7 +3469,7 @@ void FieldDescriptor::DebugString( label.clear(); } // Label is omitted for optional and required fields under editions. - if ((is_optional() || is_required()) && !IsLegacyEdition(this)) { + if ((is_optional() || is_required()) && !IsLegacyEdition(file()->edition())) { label.clear(); } @@ -3492,7 +3479,8 @@ void FieldDescriptor::DebugString( absl::SubstituteAndAppend( contents, "$0$1$2 $3 = $4", prefix, label, field_type, - IsGroupSyntax(this) ? message_type()->name() : name(), number()); + IsGroupSyntax(file()->edition(), this) ? message_type()->name() : name(), + number()); bool bracketed = false; if (has_default_value()) { @@ -3526,7 +3514,7 @@ void FieldDescriptor::DebugString( contents->append("]"); } - if (IsGroupSyntax(this)) { + if (IsGroupSyntax(file()->edition(), this)) { if (debug_string_options.elide_group_body) { contents->append(" { ... };\n"); } else { @@ -4201,8 +4189,8 @@ class DescriptorBuilder { internal::FlatAllocator& alloc); template void ResolveFeaturesImpl( - const typename DescriptorT::Proto& proto, DescriptorT* descriptor, - typename DescriptorT::OptionsType* options, + Edition edition, const typename DescriptorT::Proto& proto, + DescriptorT* descriptor, typename DescriptorT::OptionsType* options, internal::FlatAllocator& alloc, DescriptorPool::ErrorCollector::ErrorLocation error_location, bool force_merge = false); @@ -5305,8 +5293,9 @@ static void InferLegacyProtoFeatures(const FieldDescriptorProto& proto, template void DescriptorBuilder::ResolveFeaturesImpl( - const typename DescriptorT::Proto& proto, DescriptorT* descriptor, - typename DescriptorT::OptionsType* options, internal::FlatAllocator& alloc, + Edition edition, const typename DescriptorT::Proto& proto, + DescriptorT* descriptor, typename DescriptorT::OptionsType* options, + internal::FlatAllocator& alloc, DescriptorPool::ErrorCollector::ErrorLocation error_location, bool force_merge) { const FeatureSet& parent_features = GetParentFeatures(descriptor); @@ -5326,13 +5315,12 @@ void DescriptorBuilder::ResolveFeaturesImpl( FeatureSet base_features = *descriptor->proto_features_; // Handle feature inference from proto2/proto3. - if (IsLegacyEdition(descriptor)) { + if (IsLegacyEdition(edition)) { if (descriptor->proto_features_ != &FeatureSet::default_instance()) { AddError(descriptor->name(), proto, error_location, "Features are only valid under editions."); } - InferLegacyProtoFeatures(proto, *options, GetDescriptorEdition(descriptor), - base_features); + InferLegacyProtoFeatures(proto, *options, edition, base_features); } if (base_features.ByteSizeLong() == 0 && !force_merge) { @@ -5358,8 +5346,8 @@ void DescriptorBuilder::ResolveFeatures( const typename DescriptorT::Proto& proto, DescriptorT* descriptor, typename DescriptorT::OptionsType* options, internal::FlatAllocator& alloc) { - ResolveFeaturesImpl(proto, descriptor, options, alloc, - DescriptorPool::ErrorCollector::NAME); + ResolveFeaturesImpl(descriptor->file()->edition(), proto, descriptor, options, + alloc, DescriptorPool::ErrorCollector::NAME); } void DescriptorBuilder::ResolveFeatures(const FileDescriptorProto& proto, @@ -5368,7 +5356,7 @@ void DescriptorBuilder::ResolveFeatures(const FileDescriptorProto& proto, internal::FlatAllocator& alloc) { // File descriptors always need their own merged feature set, even without // any explicit features. - ResolveFeaturesImpl(proto, descriptor, options, alloc, + ResolveFeaturesImpl(descriptor->edition(), proto, descriptor, options, alloc, DescriptorPool::ErrorCollector::EDITIONS, /*force_merge=*/true); } @@ -5444,11 +5432,11 @@ void DescriptorBuilder::AddImportError(const FileDescriptorProto& proto, } PROTOBUF_NOINLINE static bool ExistingFileMatchesProto( - const FileDescriptor* existing_file, const FileDescriptorProto& proto) { + Edition edition, const FileDescriptor* existing_file, + const FileDescriptorProto& proto) { FileDescriptorProto existing_proto; existing_file->CopyTo(&existing_proto); - if (existing_file->edition() == Edition::EDITION_PROTO2 && - proto.has_syntax()) { + if (edition == Edition::EDITION_PROTO2 && proto.has_syntax()) { existing_proto.set_syntax("proto2"); } @@ -5593,7 +5581,8 @@ const FileDescriptor* DescriptorBuilder::BuildFile( const FileDescriptor* existing_file = tables_->FindFile(filename_); if (existing_file != nullptr) { // File already in pool. Compare the existing one to the input. - if (ExistingFileMatchesProto(existing_file, proto)) { + if (ExistingFileMatchesProto(existing_file->edition(), existing_file, + proto)) { // They're identical. Return the existing descriptor. return existing_file; } @@ -7882,7 +7871,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 (IsLegacyEdition(file)) { + if (IsLegacyEdition(file->edition())) { return; } @@ -9589,6 +9578,9 @@ namespace internal { absl::string_view ShortEditionName(Edition edition) { return absl::StripPrefix(Edition_Name(edition), "EDITION_"); } +Edition InternalFeatureHelper::GetEdition(const FileDescriptor& desc) { + return desc.edition(); +} } // namespace internal } // namespace protobuf diff --git a/src/google/protobuf/descriptor.h b/src/google/protobuf/descriptor.h index 9b24cb1bb9f8..3c903a1fec36 100644 --- a/src/google/protobuf/descriptor.h +++ b/src/google/protobuf/descriptor.h @@ -277,6 +277,11 @@ class PROTOBUF_EXPORT InternalFeatureHelper { FeatureSet, TypeTraitsT, field_type, is_packed>& extension) { return descriptor.proto_features_->GetExtension(extension); } + + // Provides a restricted view exclusively to code generators to query the + // edition of files being processed. While most people should never write + // edition-dependent code, generators frequently will need to. + static Edition GetEdition(const FileDescriptor& desc); }; PROTOBUF_EXPORT absl::string_view ShortEditionName(Edition edition); @@ -1853,11 +1858,6 @@ class PROTOBUF_EXPORT FileDescriptor : private internal::SymbolBase { // descriptor.proto, and any available extensions of that message. const FileOptions& options() const; - public: - // Returns edition of this file. For legacy proto2/proto3 files, special - // EDITION_PROTO2 and EDITION_PROTO3 values are used. - Edition edition() const; - // Find a top-level message type by name (not full_name). Returns nullptr if // not found. const Descriptor* FindMessageTypeByName(absl::string_view name) const; @@ -1924,8 +1924,15 @@ class PROTOBUF_EXPORT FileDescriptor : private internal::SymbolBase { bool GetSourceLocation(const std::vector& path, SourceLocation* out_location) const; + + private: + // Returns edition of this file. For legacy proto2/proto3 files, special + // EDITION_PROTO2 and EDITION_PROTO3 values are used. + Edition edition() const; + private: friend class Symbol; + friend class FileDescriptorLegacy; typedef FileOptions OptionsType; bool is_placeholder_; diff --git a/src/google/protobuf/descriptor_legacy.h b/src/google/protobuf/descriptor_legacy.h new file mode 100644 index 000000000000..88a77b922d66 --- /dev/null +++ b/src/google/protobuf/descriptor_legacy.h @@ -0,0 +1,25 @@ +#ifndef GOOGLE_PROTOBUF_DESCRIPTOR_EDITION_H__ +#define GOOGLE_PROTOBUF_DESCRIPTOR_EDITION_H__ + +#include "google/protobuf/descriptor.pb.h" +#include "google/protobuf/descriptor.h" + +namespace google { +namespace protobuf { + +// TODO Remove this deprecated API entirely. +class FileDescriptorLegacy { + public: + explicit FileDescriptorLegacy(const FileDescriptor* file) : file_(file) {} + + // Edition shouldn't be depended on unless dealing with raw unbuilt + // descriptors, which will expose it via FileDescriptorProto.edition. + Edition edition() const { return file_->edition(); } + + private: + const FileDescriptor* file_; +}; +} // namespace protobuf +} // namespace google + +#endif // GOOGLE_PROTOBUF_DESCRIPTOR_EDITION_H__ diff --git a/src/google/protobuf/descriptor_unittest.cc b/src/google/protobuf/descriptor_unittest.cc index 7745313ff606..c90192e99c58 100644 --- a/src/google/protobuf/descriptor_unittest.cc +++ b/src/google/protobuf/descriptor_unittest.cc @@ -47,6 +47,7 @@ #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" @@ -480,7 +481,7 @@ TEST_F(FileDescriptorTest, Edition) { DescriptorPool pool; const FileDescriptor* file = pool.BuildFile(proto); ASSERT_TRUE(file != nullptr); - EXPECT_EQ(file->edition(), Edition::EDITION_PROTO2); + EXPECT_EQ(FileDescriptorLegacy(file).edition(), Edition::EDITION_PROTO2); FileDescriptorProto other; file->CopyTo(&other); EXPECT_EQ("", other.syntax()); @@ -491,7 +492,7 @@ TEST_F(FileDescriptorTest, Edition) { DescriptorPool pool; const FileDescriptor* file = pool.BuildFile(proto); ASSERT_TRUE(file != nullptr); - EXPECT_EQ(file->edition(), Edition::EDITION_PROTO3); + EXPECT_EQ(FileDescriptorLegacy(file).edition(), Edition::EDITION_PROTO3); FileDescriptorProto other; file->CopyTo(&other); EXPECT_EQ("proto3", other.syntax()); @@ -503,8 +504,7 @@ TEST_F(FileDescriptorTest, Edition) { DescriptorPool pool; const FileDescriptor* file = pool.BuildFile(proto); ASSERT_TRUE(file != nullptr); - EXPECT_EQ(file->edition(), Edition::EDITION_2023); - EXPECT_EQ(file->edition(), EDITION_2023); + EXPECT_EQ(FileDescriptorLegacy(file).edition(), Edition::EDITION_2023); FileDescriptorProto other; file->CopyTo(&other); EXPECT_EQ("editions", other.syntax()); diff --git a/src/google/protobuf/util/BUILD.bazel b/src/google/protobuf/util/BUILD.bazel index 149b86c3517f..aee4bc266f55 100644 --- a/src/google/protobuf/util/BUILD.bazel +++ b/src/google/protobuf/util/BUILD.bazel @@ -170,6 +170,7 @@ 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", @@ -186,6 +187,7 @@ cc_test( ":json_format_proto3_cc_proto", ":json_util", "//src/google/protobuf", + "//src/google/protobuf:descriptor_legacy", "//src/google/protobuf:test_util", "//src/google/protobuf/testing", "@com_google_googletest//:gtest", diff --git a/src/google/protobuf/util/type_resolver_util.cc b/src/google/protobuf/util/type_resolver_util.cc index 078e7f0535b2..ca8c661b3111 100644 --- a/src/google/protobuf/util/type_resolver_util.cc +++ b/src/google/protobuf/util/type_resolver_util.cc @@ -20,6 +20,7 @@ #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" @@ -260,7 +261,8 @@ Syntax ConvertSyntax(Edition edition) { void ConvertEnumDescriptor(const EnumDescriptor& descriptor, Enum* enum_type) { enum_type->Clear(); - enum_type->set_syntax(ConvertSyntax(descriptor.file()->edition())); + enum_type->set_syntax( + ConvertSyntax(FileDescriptorLegacy(descriptor.file()).edition())); enum_type->set_name(descriptor.full_name()); enum_type->mutable_source_context()->set_file_name(descriptor.file()->name()); @@ -281,7 +283,8 @@ void ConvertDescriptor(absl::string_view url_prefix, const Descriptor& descriptor, Type* type) { type->Clear(); type->set_name(descriptor.full_name()); - type->set_syntax(ConvertSyntax(descriptor.file()->edition())); + type->set_syntax( + ConvertSyntax(FileDescriptorLegacy(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 dae2728e9be5..77cd15156db3 100644 --- a/src/google/protobuf/util/type_resolver_util_test.cc +++ b/src/google/protobuf/util/type_resolver_util_test.cc @@ -21,6 +21,7 @@ #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" @@ -443,7 +444,7 @@ class DescriptorPoolTypeResolverSyntaxTest : public testing::Test { TEST_F(DescriptorPoolTypeResolverSyntaxTest, SyntaxProto2) { const FileDescriptor* file = BuildFile("proto2"); - ASSERT_EQ(file->edition(), Edition::EDITION_PROTO2); + ASSERT_EQ(FileDescriptorLegacy(file).edition(), Edition::EDITION_PROTO2); Type type; ASSERT_TRUE( @@ -454,7 +455,7 @@ TEST_F(DescriptorPoolTypeResolverSyntaxTest, SyntaxProto2) { TEST_F(DescriptorPoolTypeResolverSyntaxTest, SyntaxProto3) { const FileDescriptor* file = BuildFile("proto3"); - ASSERT_EQ(file->edition(), Edition::EDITION_PROTO3); + ASSERT_EQ(FileDescriptorLegacy(file).edition(), Edition::EDITION_PROTO3); Type type; ASSERT_TRUE(