Fix python codegen crash when C++ features are used.

src/google/protobuf/compiler/cpp/BUILD.bazel

@@ -105,6 +105,7 @@ cc_library(
     visibility = [
         "//pkg:__pkg__",
         "//src/google/protobuf/compiler:__pkg__",
+        "//src/google/protobuf/compiler/python:__pkg__",  # For testing only.
         "@io_kythe//kythe/cxx/tools:__subpackages__",
     ],
     deps = [

src/google/protobuf/compiler/python/BUILD.bazel

@@ -51,7 +51,11 @@ cc_test(
     copts = COPTS,
     deps = [
         ":python",
+        "//src/google/protobuf",
+        "//src/google/protobuf/compiler:code_generator",
         "//src/google/protobuf/compiler:command_line_interface",
+        "//src/google/protobuf/compiler:command_line_interface_tester",
+        "//src/google/protobuf/compiler/cpp",
         "//src/google/protobuf/io",
         "//src/google/protobuf/io:printer",
         "//src/google/protobuf/testing",

src/google/protobuf/compiler/python/generator.cc

@@ -479,8 +479,17 @@ std::string Generator::GetResolvedFeatures(
     // Assume these are all enums.  If we add non-enum global features or any
     // python-specific features, we will need to come back and improve this
     // logic.
-    ABSL_CHECK(field->enum_type() != nullptr)
-        << "Unexpected non-enum field found!";
+    if (field->type() != FieldDescriptor::TYPE_ENUM) {
+      ABSL_CHECK(field->is_extension())
+          << "Unsupported non-enum global feature found: "
+          << field->full_name();
+      // Placeholder for python-specific features.
+      ABSL_CHECK(field->number() != 1003)
+          << "Unsupported python-specific feature found: "
+          << field->full_name();
+      // Skip any non-python language-specific features.
+      continue;
+    }
     if (field->options().retention() == FieldOptions::RETENTION_SOURCE) {
       // Skip any source-retention features.
       continue;

src/google/protobuf/compiler/python/plugin_unittest.cc

@@ -9,15 +9,20 @@
 
 #include <memory>
 #include <string>
+#include <utility>
 #include <vector>
 
 #include "google/protobuf/testing/file.h"
-#include "google/protobuf/testing/file.h"
-#include "google/protobuf/compiler/command_line_interface.h"
-#include "google/protobuf/compiler/python/generator.h"
 #include <gtest/gtest.h>
 #include "absl/log/absl_check.h"
+#include "absl/strings/str_cat.h"
 #include "absl/strings/str_split.h"
+#include "absl/strings/substitute.h"
+#include "google/protobuf/compiler/code_generator.h"
+#include "google/protobuf/compiler/command_line_interface_tester.h"
+#include "google/protobuf/compiler/cpp/generator.h"
+#include "google/protobuf/compiler/python/generator.h"
+#include "google/protobuf/cpp_features.pb.h"
 #include "google/protobuf/io/printer.h"
 #include "google/protobuf/io/zero_copy_stream.h"
 
@@ -100,6 +105,58 @@ TEST(PythonPluginTest, ImportTest) {
   EXPECT_TRUE(found_expected_import);
 }
 
+class PythonGeneratorTest : public CommandLineInterfaceTester,
+                            public testing::WithParamInterface<bool> {
+ protected:
+  PythonGeneratorTest() {
+    auto generator = std::make_unique<Generator>();
+    generator->set_opensource_runtime(GetParam());
+    RegisterGenerator("--python_out", "--python_opt", std::move(generator),
+                      "Python test generator");
+
+    // Generate built-in protos.
+    CreateTempFile(
+        google::protobuf::DescriptorProto::descriptor()->file()->name(),
+        google::protobuf::DescriptorProto::descriptor()->file()->DebugString());
+  }
+};
+
+TEST_P(PythonGeneratorTest, PythonWithCppFeatures) {
+  // Test that the presence of C++ features does not break Python generation.
+  RegisterGenerator("--cpp_out", "--cpp_opt",
+                    std::make_unique<cpp::CppGenerator>(),
+                    "C++ test generator");
+  CreateTempFile("google/protobuf/cpp_features.proto",
+                 pb::CppFeatures::descriptor()->file()->DebugString());
+  CreateTempFile("foo.proto",
+                 R"schema(
+    edition = "2023";
+
+    import "google/protobuf/cpp_features.proto";
+
+    package foo;
+
+    enum Bar {
+      AAA = 0;
+      BBB = 1;
+    }
+
+    message Foo {
+      Bar bar_enum = 1 [features.(pb.cpp).legacy_closed_enum = true];
+    })schema");
+
+  RunProtoc(absl::Substitute(
+      "protocol_compiler --proto_path=$$tmpdir --cpp_out=$$tmpdir "
+      "--python_out=$$tmpdir foo.proto $0 "
+      "google/protobuf/cpp_features.proto",
+      google::protobuf::DescriptorProto::descriptor()->file()->name()));
+
+  ExpectNoErrors();
+}
+
+INSTANTIATE_TEST_SUITE_P(PythonGeneratorTest, PythonGeneratorTest,
+                         testing::Bool());
+
 }  // namespace
 }  // namespace python
 }  // namespace compiler

src/google/protobuf/descriptor.cc

@@ -5537,7 +5537,7 @@ static void InferLegacyProtoFeatures(const ProtoT& proto,
 static void InferLegacyProtoFeatures(const FieldDescriptorProto& proto,
                                      const FieldOptions& options,
                                      Edition edition, FeatureSet& features) {
-  if (!features.MutableExtension(pb::cpp)->has_string_type()) {
+  if (!features.GetExtension(pb::cpp).has_string_type()) {
     if (options.ctype() == FieldOptions::CORD) {
       features.MutableExtension(pb::cpp)->set_string_type(
           pb::CppFeatures::CORD);

src/google/protobuf/descriptor.proto

@@ -1135,6 +1135,11 @@ message FeatureSet {
       type: ".pb.JavaFeatures"
     },
     declaration = { number: 1002, full_name: ".pb.go", type: ".pb.GoFeatures" },
+    declaration = {
+      number: 1003,
+      full_name: ".pb.python",
+      type: ".pb.PythonFeatures"
+    },
     declaration = {
       number: 9990,
       full_name: ".pb.proto1",

src/google/protobuf/descriptor_unittest.cc

@@ -11799,6 +11799,49 @@ TEST_F(DescriptorPoolFeaturesTest, OverrideDefaults) {
               )pb"));
 }
 
+TEST_F(DescriptorPoolFeaturesTest, OverrideFieldDefaults) {
+  FeatureSetDefaults defaults = ParseTextOrDie(R"pb(
+    defaults {
+      edition: EDITION_PROTO2
+      overridable_features {
+        field_presence: EXPLICIT
+        enum_type: CLOSED
+        repeated_field_encoding: EXPANDED
+        utf8_validation: VERIFY
+        message_encoding: LENGTH_PREFIXED
+        json_format: ALLOW
+        enforce_naming_style: STYLE_LEGACY
+      }
+    }
+    minimum_edition: EDITION_PROTO2
+    maximum_edition: EDITION_2023
+  )pb");
+  EXPECT_OK(pool_.SetFeatureSetDefaults(std::move(defaults)));
+
+  FileDescriptorProto file_proto = ParseTextOrDie(R"pb(
+    name: "foo.proto"
+    syntax: "editions"
+    edition: EDITION_PROTO3
+    message_type {
+      name: "Foo"
+      field { name: "bar" number: 1 label: LABEL_OPTIONAL type: TYPE_INT64 }
+    }
+  )pb");
+
+  BuildDescriptorMessagesInTestPool();
+  const FileDescriptor* file = ABSL_DIE_IF_NULL(pool_.BuildFile(file_proto));
+  const FieldDescriptor* field = file->message_type(0)->field(0);
+  EXPECT_THAT(GetFeatures(field), EqualsProto(R"pb(
+                field_presence: EXPLICIT
+                enum_type: CLOSED
+                repeated_field_encoding: EXPANDED
+                utf8_validation: VERIFY
+                message_encoding: LENGTH_PREFIXED
+                json_format: ALLOW
+                enforce_naming_style: STYLE_LEGACY
+              )pb"));
+}
+
 TEST_F(DescriptorPoolFeaturesTest, ResolvesFeaturesForCppDefault) {
   EXPECT_FALSE(pool_.ResolvesFeaturesFor(pb::test));
   EXPECT_FALSE(pool_.ResolvesFeaturesFor(pb::TestMessage::test_message));