Remove edition getter from C++ descriptor APIs

PiperOrigin-RevId: 593909007
protocolbuffers · Dec 27, 2023 · babb140 · babb140
1 parent 2f7b283
commit babb140
Show file tree

Hide file tree

Showing 24 changed files with 143 additions and 102 deletions.
diff --git a/conformance/BUILD.bazel b/conformance/BUILD.bazel
@@ -144,6 +144,7 @@ cc_library(
     includes = ["."],
     deps = [
         ":conformance_cc_proto",
+        "//src/google/protobuf/util: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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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<FileDescriptorProto>(_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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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()) {