Use newly split feature defaults in plugins and runtimes.

The new fields fixed_features and overridable_features can be simply merged to recover the old aggregate defaults. By splitting them though, plugins and runtimes get some extra information about lifetimes for enforcement. PiperOrigin-RevId: 625107743
protocolbuffers · Apr 17, 2024 · 7821d80 · 7821d80
1 parent b9b735c
commit 7821d80
Show file tree

Hide file tree

Showing 11 changed files with 204 additions and 89 deletions.
diff --git a/editions/defaults_test.cc b/editions/defaults_test.cc
@@ -50,11 +50,13 @@ TEST(DefaultsTest, Check2023) {
   EXPECT_EQ(defaults->defaults()[0].edition(), EDITION_PROTO2);
   EXPECT_EQ(defaults->defaults()[1].edition(), EDITION_PROTO3);
   EXPECT_EQ(defaults->defaults()[2].edition(), EDITION_2023);
-  EXPECT_EQ(defaults->defaults()[2].features().field_presence(),
+  EXPECT_EQ(defaults->defaults()[2].overridable_features().field_presence(),
             FeatureSet::EXPLICIT);
-  EXPECT_EQ(
-      defaults->defaults()[2].features().GetExtension(pb::test).file_feature(),
-      pb::VALUE3);
+  EXPECT_EQ(defaults->defaults()[2]
+                .overridable_features()
+                .GetExtension(pb::test)
+                .file_feature(),
+            pb::VALUE3);
 }
 
 TEST(DefaultsTest, CheckFuture) {
@@ -67,23 +69,29 @@ TEST(DefaultsTest, CheckFuture) {
   EXPECT_EQ(defaults->defaults()[0].edition(), EDITION_PROTO2);
   EXPECT_EQ(defaults->defaults()[1].edition(), EDITION_PROTO3);
   EXPECT_EQ(defaults->defaults()[2].edition(), EDITION_2023);
-  EXPECT_EQ(defaults->defaults()[2].features().field_presence(),
+  EXPECT_EQ(defaults->defaults()[2].overridable_features().field_presence(),
             FeatureSet::EXPLICIT);
-  EXPECT_EQ(
-      defaults->defaults()[2].features().GetExtension(pb::test).file_feature(),
-      pb::VALUE3);
+  EXPECT_EQ(defaults->defaults()[2]
+                .overridable_features()
+                .GetExtension(pb::test)
+                .file_feature(),
+            pb::VALUE3);
   EXPECT_EQ(defaults->defaults()[3].edition(), EDITION_2024);
-  EXPECT_EQ(defaults->defaults()[3].features().field_presence(),
+  EXPECT_EQ(defaults->defaults()[3].overridable_features().field_presence(),
             FeatureSet::EXPLICIT);
-  EXPECT_EQ(
-      defaults->defaults()[3].features().GetExtension(pb::test).file_feature(),
-      pb::VALUE3);
+  EXPECT_EQ(defaults->defaults()[3]
+                .overridable_features()
+                .GetExtension(pb::test)
+                .file_feature(),
+            pb::VALUE3);
   EXPECT_EQ(defaults->defaults()[4].edition(), EDITION_99997_TEST_ONLY);
-  EXPECT_EQ(defaults->defaults()[4].features().field_presence(),
+  EXPECT_EQ(defaults->defaults()[4].overridable_features().field_presence(),
             FeatureSet::EXPLICIT);
-  EXPECT_EQ(
-      defaults->defaults()[4].features().GetExtension(pb::test).file_feature(),
-      pb::VALUE4);
+  EXPECT_EQ(defaults->defaults()[4]
+                .overridable_features()
+                .GetExtension(pb::test)
+                .file_feature(),
+            pb::VALUE4);
 }
 
 TEST(DefaultsTest, CheckFarFuture) {
@@ -96,29 +104,37 @@ TEST(DefaultsTest, CheckFarFuture) {
   EXPECT_EQ(defaults->defaults()[0].edition(), EDITION_PROTO2);
   EXPECT_EQ(defaults->defaults()[1].edition(), EDITION_PROTO3);
   EXPECT_EQ(defaults->defaults()[2].edition(), EDITION_2023);
-  EXPECT_EQ(defaults->defaults()[2].features().field_presence(),
+  EXPECT_EQ(defaults->defaults()[2].overridable_features().field_presence(),
             FeatureSet::EXPLICIT);
-  EXPECT_EQ(
-      defaults->defaults()[2].features().GetExtension(pb::test).file_feature(),
-      pb::VALUE3);
+  EXPECT_EQ(defaults->defaults()[2]
+                .overridable_features()
+                .GetExtension(pb::test)
+                .file_feature(),
+            pb::VALUE3);
   EXPECT_EQ(defaults->defaults()[3].edition(), EDITION_2024);
-  EXPECT_EQ(defaults->defaults()[3].features().field_presence(),
+  EXPECT_EQ(defaults->defaults()[3].overridable_features().field_presence(),
             FeatureSet::EXPLICIT);
-  EXPECT_EQ(
-      defaults->defaults()[3].features().GetExtension(pb::test).file_feature(),
-      pb::VALUE3);
+  EXPECT_EQ(defaults->defaults()[3]
+                .overridable_features()
+                .GetExtension(pb::test)
+                .file_feature(),
+            pb::VALUE3);
   EXPECT_EQ(defaults->defaults()[4].edition(), EDITION_99997_TEST_ONLY);
-  EXPECT_EQ(defaults->defaults()[4].features().field_presence(),
+  EXPECT_EQ(defaults->defaults()[4].overridable_features().field_presence(),
             FeatureSet::EXPLICIT);
-  EXPECT_EQ(
-      defaults->defaults()[4].features().GetExtension(pb::test).file_feature(),
-      pb::VALUE4);
+  EXPECT_EQ(defaults->defaults()[4]
+                .overridable_features()
+                .GetExtension(pb::test)
+                .file_feature(),
+            pb::VALUE4);
   EXPECT_EQ(defaults->defaults()[5].edition(), EDITION_99998_TEST_ONLY);
-  EXPECT_EQ(defaults->defaults()[5].features().field_presence(),
+  EXPECT_EQ(defaults->defaults()[5].overridable_features().field_presence(),
             FeatureSet::EXPLICIT);
-  EXPECT_EQ(
-      defaults->defaults()[5].features().GetExtension(pb::test).file_feature(),
-      pb::VALUE5);
+  EXPECT_EQ(defaults->defaults()[5]
+                .overridable_features()
+                .GetExtension(pb::test)
+                .file_feature(),
+            pb::VALUE5);
 }
 
 TEST(DefaultsTest, Embedded) {
@@ -133,11 +149,13 @@ TEST(DefaultsTest, Embedded) {
   EXPECT_EQ(defaults.defaults()[0].edition(), EDITION_PROTO2);
   EXPECT_EQ(defaults.defaults()[1].edition(), EDITION_PROTO3);
   EXPECT_EQ(defaults.defaults()[2].edition(), EDITION_2023);
-  EXPECT_EQ(defaults.defaults()[2].features().field_presence(),
+  EXPECT_EQ(defaults.defaults()[2].overridable_features().field_presence(),
             FeatureSet::EXPLICIT);
-  EXPECT_EQ(
-      defaults.defaults()[2].features().GetExtension(pb::test).file_feature(),
-      pb::VALUE3);
+  EXPECT_EQ(defaults.defaults()[2]
+                .overridable_features()
+                .GetExtension(pb::test)
+                .file_feature(),
+            pb::VALUE3);
 }
 
 TEST(DefaultsTest, EmbeddedBase64) {
@@ -155,11 +173,13 @@ TEST(DefaultsTest, EmbeddedBase64) {
   EXPECT_EQ(defaults.defaults()[0].edition(), EDITION_PROTO2);
   EXPECT_EQ(defaults.defaults()[1].edition(), EDITION_PROTO3);
   EXPECT_EQ(defaults.defaults()[2].edition(), EDITION_2023);
-  EXPECT_EQ(defaults.defaults()[2].features().field_presence(),
+  EXPECT_EQ(defaults.defaults()[2].overridable_features().field_presence(),
             FeatureSet::EXPLICIT);
-  EXPECT_EQ(
-      defaults.defaults()[2].features().GetExtension(pb::test).file_feature(),
-      pb::VALUE3);
+  EXPECT_EQ(defaults.defaults()[2]
+                .overridable_features()
+                .GetExtension(pb::test)
+                .file_feature(),
+            pb::VALUE3);
 }
 
 }  // namespace

diff --git a/java/core/src/main/java/com/google/protobuf/Descriptors.java b/java/core/src/main/java/com/google/protobuf/Descriptors.java
@@ -127,18 +127,18 @@ static FeatureSet getEditionDefaults(Edition edition) {
               + javaEditionDefaults.getMaximumEdition()
               + "!");
     }
-    FeatureSet found = null;
+    FeatureSetEditionDefault found = null;
     for (FeatureSetEditionDefault editionDefault : javaEditionDefaults.getDefaultsList()) {
       if (editionDefault.getEdition().getNumber() > edition.getNumber()) {
         break;
       }
-      found = editionDefault.getFeatures();
+      found = editionDefault;
     }
     if (found == null) {
       throw new IllegalArgumentException(
           "Edition " + edition + " does not have a valid default FeatureSet!");
     }
-    return found;
+    return found.getFixedFeatures().toBuilder().mergeFrom(found.getOverridableFeatures()).build();
   }
 
   private static FeatureSet internFeatures(FeatureSet features) {

diff --git a/java/core/src/test/java/com/google/protobuf/DescriptorsTest.java b/java/core/src/test/java/com/google/protobuf/DescriptorsTest.java
@@ -1242,7 +1242,7 @@ public static class FeatureInheritanceTest {
     public void setUp() {
       FeatureSetDefaults.Builder defaults = Descriptors.getJavaEditionDefaults().toBuilder();
       for (FeatureSetEditionDefault.Builder editionDefaults : defaults.getDefaultsBuilderList()) {
-        setTestFeature(editionDefaults.getFeaturesBuilder(), 1);
+        setTestFeature(editionDefaults.getOverridableFeaturesBuilder(), 1);
       }
       Descriptors.setTestJavaEditionDefaults(defaults.build());
 

diff --git a/python/google/protobuf/descriptor_pool.py b/python/google/protobuf/descriptor_pool.py
@@ -737,15 +737,16 @@ def _CreateDefaultFeatures(self, edition):
     for d in self._edition_defaults.defaults:
       if d.edition > edition:
         break
-      found = d.features
+      found = d
     if found is None:
       raise TypeError(
           'No valid default found for edition %s!'
           % descriptor_pb2.Edition.Name(edition)
       )
 
     defaults = descriptor_pb2.FeatureSet()
-    defaults.CopyFrom(found)
+    defaults.CopyFrom(found.fixed_features)
+    defaults.MergeFrom(found.overridable_features)
     return defaults
 
   def _InternFeatures(self, features):

diff --git a/python/google/protobuf/internal/descriptor_pool_test.py b/python/google/protobuf/internal/descriptor_pool_test.py
@@ -1131,19 +1131,49 @@ def testDefault(self):
         file._GetFeatures().HasExtension(unittest_features_pb2.test)
     )
 
+  def testMergedDefaults(self):
+    pool = descriptor_pool.DescriptorPool()
+    fixed = descriptor_pb2.FeatureSet()
+    fixed.CopyFrom(unittest_features_pb2.DESCRIPTOR._GetFeatures())
+    fixed.field_presence = descriptor_pb2.FeatureSet.IMPLICIT
+    fixed.ClearField('message_encoding')
+    defaults = descriptor_pb2.FeatureSetDefaults(
+        defaults=[
+            descriptor_pb2.FeatureSetDefaults.FeatureSetEditionDefault(
+                edition=descriptor_pb2.Edition.EDITION_PROTO2,
+                fixed_features=fixed,
+                overridable_features=descriptor_pb2.FeatureSet(
+                    message_encoding=descriptor_pb2.FeatureSet.DELIMITED
+                ),
+            )
+        ],
+        minimum_edition=descriptor_pb2.Edition.EDITION_PROTO2,
+        maximum_edition=descriptor_pb2.Edition.EDITION_2023,
+    )
+    pool.SetFeatureSetDefaults(defaults)
+    file_desc = descriptor_pb2.FileDescriptorProto(name='some/file.proto')
+    file = pool.AddSerializedFile(file_desc.SerializeToString())
+    self.assertEqual(
+        file._GetFeatures().message_encoding,
+        descriptor_pb2.FeatureSet.DELIMITED,
+    )
+    self.assertEqual(
+        file._GetFeatures().field_presence, descriptor_pb2.FeatureSet.IMPLICIT
+    )
+
   def testOverride(self):
     pool = descriptor_pool.DescriptorPool()
     defaults = descriptor_pb2.FeatureSetDefaults(
         defaults=[
             descriptor_pb2.FeatureSetDefaults.FeatureSetEditionDefault(
                 edition=descriptor_pb2.Edition.EDITION_PROTO2,
-                features=unittest_features_pb2.DESCRIPTOR._GetFeatures(),
+                overridable_features=unittest_features_pb2.DESCRIPTOR._GetFeatures(),
             )
         ],
         minimum_edition=descriptor_pb2.Edition.EDITION_PROTO2,
         maximum_edition=descriptor_pb2.Edition.EDITION_2023,
     )
-    defaults.defaults[0].features.Extensions[
+    defaults.defaults[0].overridable_features.Extensions[
         unittest_features_pb2.test
     ].file_feature = unittest_features_pb2.VALUE9
     pool.SetFeatureSetDefaults(defaults)
@@ -1177,7 +1207,7 @@ def testInvalidEditionRange(self):
               defaults=[
                   descriptor_pb2.FeatureSetDefaults.FeatureSetEditionDefault(
                       edition=descriptor_pb2.Edition.EDITION_PROTO2,
-                      features=unittest_features_pb2.DESCRIPTOR._GetFeatures(),
+                      overridable_features=unittest_features_pb2.DESCRIPTOR._GetFeatures(),
                   )
               ],
               minimum_edition=descriptor_pb2.Edition.EDITION_2023,
@@ -1197,11 +1227,11 @@ def testNotStrictlyIncreasing(self):
               defaults=[
                   descriptor_pb2.FeatureSetDefaults.FeatureSetEditionDefault(
                       edition=descriptor_pb2.Edition.EDITION_PROTO3,
-                      features=unittest_features_pb2.DESCRIPTOR._GetFeatures(),
+                      overridable_features=unittest_features_pb2.DESCRIPTOR._GetFeatures(),
                   ),
                   descriptor_pb2.FeatureSetDefaults.FeatureSetEditionDefault(
                       edition=descriptor_pb2.Edition.EDITION_PROTO2,
-                      features=unittest_features_pb2.DESCRIPTOR._GetFeatures(),
+                      overridable_features=unittest_features_pb2.DESCRIPTOR._GetFeatures(),
                   ),
               ],
               minimum_edition=descriptor_pb2.Edition.EDITION_PROTO2,
@@ -1217,11 +1247,11 @@ def testUnknownEdition(self):
               defaults=[
                   descriptor_pb2.FeatureSetDefaults.FeatureSetEditionDefault(
                       edition=descriptor_pb2.Edition.EDITION_UNKNOWN,
-                      features=unittest_features_pb2.DESCRIPTOR._GetFeatures(),
+                      overridable_features=unittest_features_pb2.DESCRIPTOR._GetFeatures(),
                   ),
                   descriptor_pb2.FeatureSetDefaults.FeatureSetEditionDefault(
                       edition=descriptor_pb2.Edition.EDITION_PROTO2,
-                      features=unittest_features_pb2.DESCRIPTOR._GetFeatures(),
+                      overridable_features=unittest_features_pb2.DESCRIPTOR._GetFeatures(),
                   ),
               ],
               minimum_edition=descriptor_pb2.Edition.EDITION_PROTO2,
@@ -1238,7 +1268,7 @@ def testChangeAfterBuild(self):
         defaults=[
             descriptor_pb2.FeatureSetDefaults.FeatureSetEditionDefault(
                 edition=descriptor_pb2.Edition.EDITION_PROTO2,
-                features=unittest_features_pb2.DESCRIPTOR._GetFeatures(),
+                overridable_features=unittest_features_pb2.DESCRIPTOR._GetFeatures(),
             )
         ],
         minimum_edition=descriptor_pb2.Edition.EDITION_PROTO2,
@@ -1252,7 +1282,7 @@ def testChangeDefaultPool(self):
         defaults=[
             descriptor_pb2.FeatureSetDefaults.FeatureSetEditionDefault(
                 edition=descriptor_pb2.Edition.EDITION_PROTO2,
-                features=unittest_features_pb2.DESCRIPTOR._GetFeatures(),
+                overridable_features=unittest_features_pb2.DESCRIPTOR._GetFeatures(),
             )
         ],
         minimum_edition=descriptor_pb2.Edition.EDITION_PROTO2,
@@ -1267,7 +1297,7 @@ def testNoValidFeatures(self):
         defaults=[
             descriptor_pb2.FeatureSetDefaults.FeatureSetEditionDefault(
                 edition=descriptor_pb2.Edition.EDITION_2023,
-                features=unittest_features_pb2.DESCRIPTOR._GetFeatures(),
+                overridable_features=unittest_features_pb2.DESCRIPTOR._GetFeatures(),
             )
         ],
         minimum_edition=descriptor_pb2.Edition.EDITION_PROTO2,
@@ -1285,7 +1315,7 @@ def testBelowMinimum(self):
         defaults=[
             descriptor_pb2.FeatureSetDefaults.FeatureSetEditionDefault(
                 edition=descriptor_pb2.Edition.EDITION_PROTO3,
-                features=unittest_features_pb2.DESCRIPTOR._GetFeatures(),
+                overridable_features=unittest_features_pb2.DESCRIPTOR._GetFeatures(),
             )
         ],
         minimum_edition=descriptor_pb2.Edition.EDITION_PROTO3,
@@ -1305,7 +1335,7 @@ def testAboveMaximum(self):
         defaults=[
             descriptor_pb2.FeatureSetDefaults.FeatureSetEditionDefault(
                 edition=descriptor_pb2.Edition.EDITION_PROTO2,
-                features=unittest_features_pb2.DESCRIPTOR._GetFeatures(),
+                overridable_features=unittest_features_pb2.DESCRIPTOR._GetFeatures(),
             )
         ],
         minimum_edition=descriptor_pb2.Edition.EDITION_PROTO2,

diff --git a/python/google/protobuf/internal/descriptor_test.py b/python/google/protobuf/internal/descriptor_test.py
@@ -1455,13 +1455,13 @@ class ReturnObject:
         defaults=[
             descriptor_pb2.FeatureSetDefaults.FeatureSetEditionDefault(
                 edition=descriptor_pb2.Edition.EDITION_PROTO2,
-                features=unittest_pb2.TestAllTypes.DESCRIPTOR._GetFeatures(),
+                overridable_features=unittest_pb2.TestAllTypes.DESCRIPTOR._GetFeatures(),
             )
         ],
         minimum_edition=descriptor_pb2.Edition.EDITION_PROTO2,
         maximum_edition=descriptor_pb2.Edition.EDITION_2023,
     )
-    defaults.defaults[0].features.Extensions[
+    defaults.defaults[0].overridable_features.Extensions[
         unittest_features_pb2.test
     ].multiple_feature = 1
     ret.pool.SetFeatureSetDefaults(defaults)

diff --git a/src/google/protobuf/compiler/command_line_interface_unittest.cc b/src/google/protobuf/compiler/command_line_interface_unittest.cc
@@ -2102,21 +2102,31 @@ TEST_F(CommandLineInterfaceTest, EditionDefaultsWithExtension) {
   EXPECT_EQ(defaults.defaults(3).edition(), EDITION_2024);
   EXPECT_EQ(defaults.defaults(4).edition(), EDITION_99997_TEST_ONLY);
   EXPECT_EQ(defaults.defaults(5).edition(), EDITION_99998_TEST_ONLY);
-  EXPECT_EQ(
-      defaults.defaults(0).features().GetExtension(pb::test).file_feature(),
-      pb::EnumFeature::VALUE1);
-  EXPECT_EQ(
-      defaults.defaults(2).features().GetExtension(pb::test).file_feature(),
-      pb::EnumFeature::VALUE3);
-  EXPECT_EQ(
-      defaults.defaults(3).features().GetExtension(pb::test).file_feature(),
-      pb::EnumFeature::VALUE3);
-  EXPECT_EQ(
-      defaults.defaults(4).features().GetExtension(pb::test).file_feature(),
-      pb::EnumFeature::VALUE4);
-  EXPECT_EQ(
-      defaults.defaults(5).features().GetExtension(pb::test).file_feature(),
-      pb::EnumFeature::VALUE5);
+  EXPECT_EQ(defaults.defaults(0)
+                .fixed_features()
+                .GetExtension(pb::test)
+                .file_feature(),
+            pb::EnumFeature::VALUE1);
+  EXPECT_EQ(defaults.defaults(2)
+                .overridable_features()
+                .GetExtension(pb::test)
+                .file_feature(),
+            pb::EnumFeature::VALUE3);
+  EXPECT_EQ(defaults.defaults(3)
+                .overridable_features()
+                .GetExtension(pb::test)
+                .file_feature(),
+            pb::EnumFeature::VALUE3);
+  EXPECT_EQ(defaults.defaults(4)
+                .overridable_features()
+                .GetExtension(pb::test)
+                .file_feature(),
+            pb::EnumFeature::VALUE4);
+  EXPECT_EQ(defaults.defaults(5)
+                .overridable_features()
+                .GetExtension(pb::test)
+                .file_feature(),
+            pb::EnumFeature::VALUE5);
 }
 
 #ifndef _WIN32