Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove edition getter from C++ descriptor APIs #15201

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions conformance/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
4 changes: 3 additions & 1 deletion conformance/conformance_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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:
Expand Down
1 change: 1 addition & 0 deletions pkg/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
7 changes: 0 additions & 7 deletions python/descriptor.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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},
};

Expand Down
7 changes: 0 additions & 7 deletions python/google/protobuf/descriptor.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
3 changes: 0 additions & 3 deletions python/google/protobuf/internal/descriptor_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
5 changes: 0 additions & 5 deletions python/google/protobuf/pyext/descriptor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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},
};

Expand Down
13 changes: 13 additions & 0 deletions src/google/protobuf/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down Expand Up @@ -1030,6 +1042,7 @@ cc_test(
}),
deps = [
":cc_test_protos",
":descriptor_legacy",
":protobuf",
":test_textproto",
"//src/google/protobuf/compiler:importer",
Expand Down
5 changes: 5 additions & 0 deletions src/google/protobuf/compiler/code_generator.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
22 changes: 13 additions & 9 deletions src/google/protobuf/compiler/command_line_interface.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 "
Expand Down Expand Up @@ -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 "
Expand All @@ -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;
}
Expand All @@ -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;
}
}
Expand Down
1 change: 1 addition & 0 deletions src/google/protobuf/compiler/java/generator.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ class PROTOC_EXPORT JavaGenerator : public CodeGenerator {
opensource_runtime_ = opensource;
}

using CodeGenerator::GetEdition;
using CodeGenerator::GetResolvedSourceFeatures;

private:
Expand Down
7 changes: 5 additions & 2 deletions src/google/protobuf/compiler/java/message_lite.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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;
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/google/protobuf/compiler/mock_code_generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
7 changes: 4 additions & 3 deletions src/google/protobuf/compiler/objectivec/file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
Expand Down Expand Up @@ -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;
Expand Down
3 changes: 2 additions & 1 deletion src/google/protobuf/compiler/objectivec/file.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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_;
Expand Down
3 changes: 2 additions & 1 deletion src/google/protobuf/compiler/objectivec/generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions src/google/protobuf/compiler/python/generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand Down
Loading
Loading