Editions: Refactor feature resolution to use an intermediate message.

This places all of the tricky reflection logic of feature resolution into a single location that outputs a portable protobuf message.  With this message, feature resolution becomes drastically simpler and easy to reimplement in other languages.

Follow-up changes will provide:
- An API to specify generator features from the CodeGenerator class for C++ plugins
- A CLI for building these default mappings
- Bazel/CMake rules to help embed these mappings in target languages (i.e. for runtimes and non-C++ plugins)

PiperOrigin-RevId: 559615720

src/google/protobuf/BUILD.bazel

@@ -1109,6 +1109,9 @@ cc_test(
         "//src/google/protobuf/compiler:importer",
         "//src/google/protobuf/stubs",
         "//src/google/protobuf/testing",
+        "@com_google_absl//absl/log:absl_check",
+        "@com_google_absl//absl/log:absl_log",
+        "@com_google_absl//absl/log:die_if_null",
         "@com_google_googletest//:gtest",
         "@com_google_googletest//:gtest_main",
     ],

src/google/protobuf/compiler/code_generator_unittest.cc

@@ -177,7 +177,8 @@ TEST_F(CodeGeneratorTest, GetResolvedSourceFeaturesRoot) {
   EXPECT_EQ(features.field_presence(), FeatureSet::EXPLICIT);
   EXPECT_EQ(features.enum_type(), FeatureSet::CLOSED);
 
-  EXPECT_TRUE(ext.has_int_message_feature());
+  // TODO(b/296638633) Flip this once generators can specify their feature sets.
+  EXPECT_FALSE(ext.has_int_message_feature());
   EXPECT_EQ(ext.int_file_feature(), 8);
   EXPECT_EQ(ext.string_source_feature(), "file");
 }

src/google/protobuf/compiler/command_line_interface_unittest.cc

@@ -1353,7 +1353,7 @@ TEST_F(CommandLineInterfaceTest, AllowServicesHasService) {
 
 TEST_F(CommandLineInterfaceTest, EditionsAreNotAllowed) {
   CreateTempFile("foo.proto",
-                 "edition = \"very-cool\";\n"
+                 "edition = \"2023\";\n"
                  "message FooRequest {}\n");
 
   Run("protocol_compiler --proto_path=$tmpdir --test_out=$tmpdir foo.proto");
@@ -1365,7 +1365,7 @@ TEST_F(CommandLineInterfaceTest, EditionsAreNotAllowed) {
 
 TEST_F(CommandLineInterfaceTest, EditionsFlagExplicitTrue) {
   CreateTempFile("foo.proto",
-                 "edition = \"very-cool\";\n"
+                 "edition = \"2023\";\n"
                  "message FooRequest {}\n");
 
   Run("protocol_compiler --proto_path=$tmpdir --test_out=$tmpdir "
@@ -1477,14 +1477,16 @@ TEST_F(CommandLineInterfaceTest, FeatureExtensionError) {
     import "features.proto";
     message Foo {
       int32 bar = 1;
-      int32 baz = 2 [features.(pb.test).int_feature = 5];
+      int32 baz = 2 [features.(pb.test).repeated_feature = 5];
     })schema");
 
   Run("protocol_compiler --proto_path=$tmpdir --test_out=$tmpdir "
       "--experimental_editions foo.proto");
-  ExpectErrorSubstring(
-      "Feature field pb.TestFeatures.repeated_feature is an unsupported "
-      "repeated field");
+  // TODO(b/296638633) Flip this once generators can specify their feature sets.
+  ExpectNoErrors();
+  // ExpectErrorSubstring(
+  //     "Feature field pb.TestFeatures.repeated_feature is an unsupported "
+  //     "repeated field");
 }
 
 TEST_F(CommandLineInterfaceTest, Plugin_LegacyFeatures) {

src/google/protobuf/descriptor.cc

@@ -44,6 +44,7 @@
 #include <sstream>
 #include <string>
 #include <type_traits>
+#include <utility>
 #include <vector>
 
 #include "google/protobuf/stubs/common.h"
@@ -57,6 +58,7 @@
 #include "absl/hash/hash.h"
 #include "absl/log/absl_check.h"
 #include "absl/log/absl_log.h"
+#include "absl/memory/memory.h"
 #include "absl/status/statusor.h"
 #include "absl/strings/ascii.h"
 #include "absl/strings/escaping.h"
@@ -1111,6 +1113,18 @@ bool AllowedExtendeeInProto3(const std::string& name) {
          allowed_proto3_extendees->end();
 }
 
+const FeatureSetDefaults& GetCppFeatureSetDefaults() {
+  static const FeatureSetDefaults* default_spec = [] {
+    auto default_spec = FeatureResolver::CompileDefaults(
+        FeatureSet::descriptor(),
+        // TODO(b/297261063) Move this range to a central location.
+        {pb::CppFeatures::descriptor()->file()->extension(0)}, "2023", "2023");
+    ABSL_CHECK(default_spec.ok()) << default_spec.status();
+    return new FeatureSetDefaults(std::move(default_spec).value());
+  }();
+  return *default_spec;
+}
+
 // Create global defaults for proto2/proto3 compatibility.
 FeatureSet* CreateProto2DefaultFeatures() {
   FeatureSet* features = new FeatureSet();
@@ -4577,14 +4591,7 @@ class DescriptorBuilder {
 
 const FileDescriptor* DescriptorPool::BuildFile(
     const FileDescriptorProto& proto) {
-  ABSL_CHECK(fallback_database_ == nullptr)
-      << "Cannot call BuildFile on a DescriptorPool that uses a "
-         "DescriptorDatabase.  You must instead find a way to get your file "
-         "into the underlying database.";
-  ABSL_CHECK(mutex_ == nullptr);  // Implied by the above ABSL_CHECK.
-  tables_->known_bad_symbols_.clear();
-  tables_->known_bad_files_.clear();
-  return DescriptorBuilder::New(this, tables_.get(), nullptr)->BuildFile(proto);
+  return BuildFileCollectingErrors(proto, nullptr);
 }
 
 const FileDescriptor* DescriptorPool::BuildFileCollectingErrors(
@@ -4596,6 +4603,7 @@ const FileDescriptor* DescriptorPool::BuildFileCollectingErrors(
   ABSL_CHECK(mutex_ == nullptr);  // Implied by the above ABSL_CHECK.
   tables_->known_bad_symbols_.clear();
   tables_->known_bad_files_.clear();
+  build_started_ = true;
   return DescriptorBuilder::New(this, tables_.get(), error_collector)
       ->BuildFile(proto);
 }
@@ -4603,6 +4611,7 @@ const FileDescriptor* DescriptorPool::BuildFileCollectingErrors(
 const FileDescriptor* DescriptorPool::BuildFileFromDatabase(
     const FileDescriptorProto& proto) const {
   mutex_->AssertHeld();
+  build_started_ = true;
   if (tables_->known_bad_files_.contains(proto.name())) {
     return nullptr;
   }
@@ -4615,6 +4624,14 @@ const FileDescriptor* DescriptorPool::BuildFileFromDatabase(
   return result;
 }
 
+void DescriptorPool::SetFeatureSetDefaults(FeatureSetDefaults spec) {
+  ABSL_CHECK(!build_started_)
+      << "Feature set defaults can't be changed once the pool has started "
+         "building.";
+  feature_set_defaults_spec_ =
+      absl::make_unique<FeatureSetDefaults>(std::move(spec));
+}
+
 DescriptorBuilder::DescriptorBuilder(
     const DescriptorPool* pool, DescriptorPool::Tables* tables,
     DescriptorPool::ErrorCollector* error_collector)
@@ -5699,8 +5716,13 @@ FileDescriptor* DescriptorBuilder::BuildFileImpl(
     }
     ABSL_CHECK(descriptor);
 
+    const FeatureSetDefaults& defaults =
+        pool_->feature_set_defaults_spec_ == nullptr
+            ? GetCppFeatureSetDefaults()
+            : *pool_->feature_set_defaults_spec_;
+
     absl::StatusOr<FeatureResolver> feature_resolver =
-        FeatureResolver::Create(proto.edition(), descriptor);
+        FeatureResolver::Create(proto.edition(), defaults);
     if (!feature_resolver.ok()) {
       AddError(
           proto.name(), proto, DescriptorPool::ErrorCollector::EDITIONS,
@@ -5816,16 +5838,6 @@ FileDescriptor* DescriptorBuilder::BuildFileImpl(
       return nullptr;
     }
 
-    // Look for feature extensions in regular imports.
-    if (feature_resolver_.has_value() && dependency != nullptr) {
-      absl::Status status = feature_resolver_->RegisterExtensions(*dependency);
-      if (!status.ok()) {
-        AddError(dependency->name(), proto,
-                 DescriptorPool::ErrorCollector::EDITIONS,
-                 [&] { return std::string(status.message()); });
-      }
-    }
-
     if (dependency == nullptr) {
       if (!pool_->lazily_build_dependencies_) {
         if (pool_->allow_unknown_ ||

src/google/protobuf/descriptor.h

@@ -122,6 +122,7 @@ class MethodOptions;
 class FileOptions;
 class UninterpretedOption;
 class FeatureSet;
+class FeatureSetDefaults;
 class SourceCodeInfo;
 
 // Defined in message_lite.h
@@ -2289,6 +2290,13 @@ class PROTOBUF_EXPORT DescriptorPool {
   // DescriptorPool will report a import not found error.
   void EnforceWeakDependencies(bool enforce) { enforce_weak_ = enforce; }
 
+  // Sets the default feature mappings used during the build. If this function
+  // isn't called, the C++ feature set defaults are used.  If this function is
+  // called, these defaults will be used instead.
+  // FeatureSetDefaults includes a minimum/maximum supported edition, which will
+  // be enforced while building proto files.
+  void SetFeatureSetDefaults(FeatureSetDefaults spec);
+
   // Toggles enforcement of extension declarations.
   // This enforcement is disabled by default because it requires full
   // descriptors with source-retention options, which are generally not
@@ -2469,11 +2477,16 @@ class PROTOBUF_EXPORT DescriptorPool {
   bool enforce_extension_declarations_;
   bool disallow_enforce_utf8_;
   bool deprecated_legacy_json_field_conflicts_;
+  mutable bool build_started_ = false;
 
   // Set of files to track for unused imports. The bool value when true means
   // unused imports are treated as errors (and as warnings when false).
   absl::flat_hash_map<std::string, bool> unused_import_track_files_;
 
+  // Specification of defaults to use for feature resolution.  This defaults to
+  // just the global and C++ features, but can be overridden for other runtimes.
+  std::unique_ptr<FeatureSetDefaults> feature_set_defaults_spec_;
+
   // Returns true if the field extends an option message of descriptor.proto.
   bool IsExtendingDescriptor(const FieldDescriptor& field) const;
 

src/google/protobuf/descriptor_unittest.cc

@@ -41,6 +41,7 @@
 #include <memory>
 #include <string>
 #include <tuple>
+#include <utility>
 #include <vector>
 
 #include "google/protobuf/stubs/common.h"
@@ -67,6 +68,7 @@
 #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"
 #include "google/protobuf/io/zero_copy_stream_impl_lite.h"
 #include "google/protobuf/test_textproto.h"
@@ -514,17 +516,17 @@ TEST_F(FileDescriptorTest, Syntax) {
   }
   {
     proto.set_syntax("editions");
-    proto.set_edition("very-cool");
+    proto.set_edition("2023");
     DescriptorPool pool;
     const FileDescriptor* file = pool.BuildFile(proto);
     ASSERT_TRUE(file != nullptr);
     EXPECT_EQ(FileDescriptorLegacy::Syntax::SYNTAX_EDITIONS,
               FileDescriptorLegacy(file).syntax());
-    EXPECT_EQ("very-cool", file->edition());
+    EXPECT_EQ("2023", file->edition());
     FileDescriptorProto other;
     file->CopyTo(&other);
     EXPECT_EQ("editions", other.syntax());
-    EXPECT_EQ("very-cool", other.edition());
+    EXPECT_EQ("2023", other.edition());
   }
 }
 
@@ -552,7 +554,7 @@ TEST_F(FileDescriptorTest, CopyHeadingTo) {
   EXPECT_EQ(&other.options().features(), &FeatureSet::default_instance());
   {
     proto.set_syntax("editions");
-    proto.set_edition("very-cool");
+    proto.set_edition("2023");
 
     DescriptorPool pool;
     const FileDescriptor* file = pool.BuildFile(proto);
@@ -563,7 +565,7 @@ TEST_F(FileDescriptorTest, CopyHeadingTo) {
     EXPECT_EQ(other.name(), "foo.proto");
     EXPECT_EQ(other.package(), "foo.bar.baz");
     EXPECT_EQ(other.syntax(), "editions");
-    EXPECT_EQ(other.edition(), "very-cool");
+    EXPECT_EQ(other.edition(), "2023");
     EXPECT_EQ(other.options().java_package(), "foo.bar.baz");
     EXPECT_TRUE(other.message_type().empty());
     EXPECT_EQ(&other.options().features(), &FeatureSet::default_instance());
@@ -7235,7 +7237,24 @@ TEST_F(ValidationErrorTest, UnusedImportWithOtherError) {
       "foo.proto: Foo.foo: EXTENDEE: \"Baz\" is not defined.\n");
 }
 
-using FeaturesTest = ValidationErrorTest;
+using FeaturesBaseTest = ValidationErrorTest;
+
+class FeaturesTest : public FeaturesBaseTest {
+ protected:
+  void SetUp() override {
+    ValidationErrorTest::SetUp();
+
+    auto default_spec = FeatureResolver::CompileDefaults(
+        FeatureSet::descriptor(),
+        {pb::CppFeatures::descriptor()->file()->extension(0),
+         pb::TestFeatures::descriptor()->file()->extension(0),
+         pb::TestMessage::descriptor()->extension(0),
+         pb::TestMessage::Nested::descriptor()->extension(0)},
+        "2023", "2025");
+    ASSERT_OK(default_spec);
+    pool_.SetFeatureSetDefaults(std::move(default_spec).value());
+  }
+};
 
 template <typename T>
 const FeatureSet& GetFeatures(const T* descriptor) {
@@ -7544,12 +7563,36 @@ TEST_F(FeaturesTest, Edition2023Defaults) {
         [pb.cpp] { legacy_closed_enum: false utf8_validation: VERIFY_PARSE }
       )pb"));
 
-  // Since pb::test is linked in, it should end up with defaults in our
-  // FeatureSet.
+  // Since pb::test is registered in the pool, it should end up with defaults in
+  // our FeatureSet.
   EXPECT_TRUE(GetFeatures(file).HasExtension(pb::test));
   EXPECT_EQ(GetFeatures(file).GetExtension(pb::test).int_file_feature(), 1);
 }
 
+TEST_F(FeaturesBaseTest, DefaultEdition2023Defaults) {
+  BuildDescriptorMessagesInTestPool();
+  BuildFileInTestPool(pb::TestFeatures::descriptor()->file());
+  const FileDescriptor* file = BuildFile(R"pb(
+    name: "foo.proto"
+    syntax: "editions"
+    edition: "2023"
+    dependency: "google/protobuf/unittest_features.proto"
+  )pb");
+  ASSERT_NE(file, nullptr);
+
+  EXPECT_THAT(file->options(), EqualsProto(""));
+  EXPECT_THAT(
+      GetFeatures(file), EqualsProto(R"pb(
+        field_presence: EXPLICIT
+        enum_type: OPEN
+        repeated_field_encoding: PACKED
+        message_encoding: LENGTH_PREFIXED
+        json_format: ALLOW
+        [pb.cpp] { legacy_closed_enum: false utf8_validation: VERIFY_PARSE }
+      )pb"));
+  EXPECT_FALSE(GetFeatures(file).HasExtension(pb::test));
+}
+
 TEST_F(FeaturesTest, ClearsOptions) {
   BuildDescriptorMessagesInTestPool();
   const FileDescriptor* file = BuildFile(R"pb(
@@ -7842,9 +7885,8 @@ TEST_F(FeaturesTest, InvalidEdition) {
       R"pb(
         name: "foo.proto" syntax: "editions" edition: "2022"
       )pb",
-      "foo.proto: foo.proto: EDITIONS: No valid default found for edition 2022 "
-      "in "
-      "feature field google.protobuf.FeatureSet.field_presence\n");
+      "foo.proto: foo.proto: EDITIONS: Edition 2022 is earlier than the "
+      "minimum supported edition 2023\n");
 }
 
 TEST_F(FeaturesTest, FileFeatures) {
@@ -9110,48 +9152,6 @@ TEST_F(FeaturesTest, FeaturesOutsideEditions) {
       "editions.\n");
 }
 
-TEST_F(FeaturesTest, InvalidExtensionNonMessage) {
-  BuildDescriptorMessagesInTestPool();
-  ASSERT_NE(BuildFile(R"pb(
-              name: "unittest_invalid_features.proto"
-              syntax: "proto2"
-              package: "pb"
-              dependency: "google/protobuf/descriptor.proto"
-              message_type {
-                name: "TestInvalid"
-                extension {
-                  name: "scalar_extension"
-                  number: 9996
-                  label: LABEL_OPTIONAL
-                  type: TYPE_STRING
-                  extendee: ".google.protobuf.FeatureSet"
-                }
-              }
-            )pb"),
-            nullptr);
-  BuildFileWithErrors(
-      R"pb(
-        name: "foo.proto"
-        syntax: "editions"
-        edition: "2023"
-        dependency: "unittest_invalid_features.proto"
-        options {
-          uninterpreted_option {
-            name { name_part: "features" is_extension: false }
-            name {
-              name_part: "pb.TestInvalid.scalar_extension"
-              is_extension: true
-            }
-            identifier_value: "hello"
-          }
-        }
-      )pb",
-      "foo.proto: unittest_invalid_features.proto: EDITIONS: FeatureSet "
-      "extension pb.TestInvalid.scalar_extension is not of message type.  "
-      "Feature extensions should always use messages to allow for "
-      "evolution.\n");
-}
-
 TEST_F(FeaturesTest, InvalidFieldImplicitDefault) {
   BuildDescriptorMessagesInTestPool();
   BuildFileWithErrors(

src/google/protobuf/feature_resolver.cc

@@ -31,6 +31,7 @@
 #include "google/protobuf/feature_resolver.h"
 
 #include <algorithm>
+#include <cstddef>
 #include <iterator>
 #include <memory>
 #include <string>
@@ -38,13 +39,17 @@
 #include <vector>
 
 #include "absl/algorithm/container.h"
+#include "absl/container/btree_set.h"
+#include "absl/container/flat_hash_set.h"
 #include "absl/log/absl_check.h"
 #include "absl/memory/memory.h"
 #include "absl/status/status.h"
 #include "absl/status/statusor.h"
 #include "absl/strings/str_cat.h"
+#include "absl/strings/str_join.h"
 #include "absl/strings/str_split.h"
 #include "absl/strings/string_view.h"
+#include "absl/types/span.h"
 #include "google/protobuf/cpp_features.pb.h"
 #include "google/protobuf/descriptor.h"
 #include "google/protobuf/descriptor.pb.h"
@@ -71,24 +76,25 @@ absl::Status Error(Args... args) {
   return absl::FailedPreconditionError(absl::StrCat(args...));
 }
 
-bool EditionsLessThan(absl::string_view a, absl::string_view b) {
-  std::vector<absl::string_view> as = absl::StrSplit(a, '.');
-  std::vector<absl::string_view> bs = absl::StrSplit(b, '.');
-  size_t min_size = std::min(as.size(), bs.size());
-  for (size_t i = 0; i < min_size; ++i) {
-    if (as[i].size() != bs[i].size()) {
-      return as[i].size() < bs[i].size();
-    } else if (as[i] != bs[i]) {
-      return as[i] < bs[i];
+struct EditionsLessThan {
+  bool operator()(absl::string_view a, absl::string_view b) const {
+    std::vector<absl::string_view> as = absl::StrSplit(a, '.');
+    std::vector<absl::string_view> bs = absl::StrSplit(b, '.');
+    size_t min_size = std::min(as.size(), bs.size());
+    for (size_t i = 0; i < min_size; ++i) {
+      if (as[i].size() != bs[i].size()) {
+        return as[i].size() < bs[i].size();
+      } else if (as[i] != bs[i]) {
+        return as[i] < bs[i];
+      }
     }
+    // Both strings are equal up until an extra element, which makes that string
+    // more recent.
+    return as.size() < bs.size();
   }
-  // Both strings are equal up until an extra element, which makes that string
-  // more recent.
-  return as.size() < bs.size();
-}
+};
 
-absl::Status ValidateDescriptor(absl::string_view edition,
-                                const Descriptor& descriptor) {
+absl::Status ValidateDescriptor(const Descriptor& descriptor) {
   if (descriptor.oneof_decl_count() > 0) {
     return Error("Type ", descriptor.full_name(),
                  " contains unsupported oneof feature fields.");
@@ -113,35 +119,57 @@ absl::Status ValidateDescriptor(absl::string_view edition,
   return absl::OkStatus();
 }
 
-absl::Status ValidateExtension(const FieldDescriptor& extension) {
-  if (extension.message_type() == nullptr) {
-    return Error("FeatureSet extension ", extension.full_name(),
+absl::Status ValidateExtension(const Descriptor& feature_set,
+                               const FieldDescriptor* extension) {
+  if (extension == nullptr) {
+    return Error("Unknown extension of ", feature_set.full_name(), ".");
+  }
+
+  if (extension->containing_type() != &feature_set) {
+    return Error("Extension ", extension->full_name(),
+                 " is not an extension of ", feature_set.full_name(), ".");
+  }
+
+  if (extension->message_type() == nullptr) {
+    return Error("FeatureSet extension ", extension->full_name(),
                  " is not of message type.  Feature extensions should "
                  "always use messages to allow for evolution.");
   }
 
-  if (extension.is_repeated()) {
+  if (extension->is_repeated()) {
     return Error(
         "Only singular features extensions are supported.  Found "
         "repeated extension ",
-        extension.full_name());
+        extension->full_name());
   }
 
-  if (extension.message_type()->extension_count() > 0 ||
-      extension.message_type()->extension_range_count() > 0) {
+  if (extension->message_type()->extension_count() > 0 ||
+      extension->message_type()->extension_range_count() > 0) {
     return Error("Nested extensions in feature extension ",
-                 extension.full_name(), " are not supported.");
+                 extension->full_name(), " are not supported.");
   }
 
   return absl::OkStatus();
 }
 
+void CollectEditions(const Descriptor& descriptor,
+                     absl::string_view minimum_edition,
+                     absl::string_view maximum_edition,
+                     absl::btree_set<std::string, EditionsLessThan>& editions) {
+  for (int i = 0; i < descriptor.field_count(); ++i) {
+    for (const auto& def : descriptor.field(i)->options().edition_defaults()) {
+      if (EditionsLessThan()(maximum_edition, def.edition())) continue;
+      editions.insert(def.edition());
+    }
+  }
+}
+
 absl::Status FillDefaults(absl::string_view edition, Message& msg) {
   const Descriptor& descriptor = *msg.GetDescriptor();
 
   auto comparator = [](const FieldOptions::EditionDefault& a,
                        const FieldOptions::EditionDefault& b) {
-    return EditionsLessThan(a.edition(), b.edition());
+    return EditionsLessThan()(a.edition(), b.edition());
   };
   FieldOptions::EditionDefault edition_lookup;
   edition_lookup.set_edition(edition);
@@ -206,124 +234,100 @@ absl::Status ValidateMergedFeatures(const Message& msg) {
   return absl::OkStatus();
 }
 
-// Forces calculation of the defaults for any features from the generated pool
-// that may be missed if the proto file doesn't import them, giving the final
-// resolved FeatureSet object maximal coverage.
-absl::StatusOr<FeatureSet> FillGeneratedDefaults(absl::string_view edition,
-                                                 const DescriptorPool* pool) {
-  const Descriptor* desc =
-      pool->FindMessageTypeByName(FeatureSet::descriptor()->full_name());
-  // If there's no FeatureSet, there's no extensions.
-  if (desc == nullptr) return FeatureSet();
-
-  DynamicMessageFactory message_factory;
-  auto defaults = absl::WrapUnique(message_factory.GetPrototype(desc)->New());
-  std::vector<const FieldDescriptor*> extensions;
-  pool->FindAllExtensions(desc, &extensions);
-  for (const auto* extension : extensions) {
-    RETURN_IF_ERROR(ValidateExtension(*extension));
-    Message* msg =
-        defaults->GetReflection()->MutableMessage(defaults.get(), extension);
-    ABSL_CHECK(msg != nullptr);
-    RETURN_IF_ERROR(FillDefaults(edition, *msg));
-  }
-
-  // Our defaults are reflectively built in a custom pool, while the
-  // returned object here is in the generated pool.  We parse/serialize to
-  // convert from the potentially skewed FeatureSet descriptors.
-  FeatureSet features;
-  ABSL_CHECK(features.MergeFromString(defaults->SerializeAsString()));
-  return features;
-}
-
 }  // namespace
 
-absl::StatusOr<FeatureResolver> FeatureResolver::Create(
-    absl::string_view edition, const Descriptor* descriptor,
-    const DescriptorPool* generated_pool) {
-  if (descriptor == nullptr) {
+absl::StatusOr<FeatureSetDefaults> FeatureResolver::CompileDefaults(
+    const Descriptor* feature_set,
+    absl::Span<const FieldDescriptor* const> extensions,
+    absl::string_view minimum_edition, absl::string_view maximum_edition) {
+  // Find and validate the FeatureSet in the pool.
+  if (feature_set == nullptr) {
     return Error(
         "Unable to find definition of google.protobuf.FeatureSet in descriptor pool.");
   }
+  RETURN_IF_ERROR(ValidateDescriptor(*feature_set));
 
-  RETURN_IF_ERROR(ValidateDescriptor(edition, *descriptor));
-
-  auto message_factory = absl::make_unique<DynamicMessageFactory>();
-  auto defaults =
-      absl::WrapUnique(message_factory->GetPrototype(descriptor)->New());
-
-  RETURN_IF_ERROR(FillDefaults(edition, *defaults));
-
-  FeatureSet generated_defaults;
-  if (descriptor->file()->pool() == generated_pool) {
-    // If we're building the generated pool, the best we can do is load the C++
-    // language features, which should always be present for code using the C++
-    // runtime.
-    RETURN_IF_ERROR(
-        FillDefaults(edition, *generated_defaults.MutableExtension(pb::cpp)));
-  } else {
-    absl::StatusOr<FeatureSet> defaults =
-        FillGeneratedDefaults(edition, generated_pool);
-    RETURN_IF_ERROR(defaults.status());
-    generated_defaults = std::move(defaults).value();
+  // Collect and validate all the FeatureSet extensions.
+  for (const auto* extension : extensions) {
+    RETURN_IF_ERROR(ValidateExtension(*feature_set, extension));
+    RETURN_IF_ERROR(ValidateDescriptor(*extension->message_type()));
   }
 
-  return FeatureResolver(edition, *descriptor, std::move(message_factory),
-                         std::move(defaults), std::move(generated_defaults));
-}
-
-absl::Status FeatureResolver::RegisterExtension(
-    const FieldDescriptor& extension) {
-  if (!extension.is_extension() ||
-      extension.containing_type() != &descriptor_ ||
-      extensions_.contains(&extension)) {
-    // These are valid but irrelevant extensions, just return ok.
-    return absl::OkStatus();
+  // Collect all the editions with unique defaults.
+  absl::btree_set<std::string, EditionsLessThan> editions;
+  CollectEditions(*feature_set, minimum_edition, maximum_edition, editions);
+  for (const auto* extension : extensions) {
+    CollectEditions(*extension->message_type(), minimum_edition,
+                    maximum_edition, editions);
   }
 
-  ABSL_CHECK(descriptor_.IsExtensionNumber(extension.number()));
-
-  RETURN_IF_ERROR(ValidateExtension(extension));
-
-  RETURN_IF_ERROR(ValidateDescriptor(edition_, *extension.message_type()));
-
-  Message* msg =
-      defaults_->GetReflection()->MutableMessage(defaults_.get(), &extension);
-  ABSL_CHECK(msg != nullptr);
-  RETURN_IF_ERROR(FillDefaults(edition_, *msg));
-
-  extensions_.insert(&extension);
-  return absl::OkStatus();
+  // Fill the default spec.
+  FeatureSetDefaults defaults;
+  defaults.set_minimum_edition(minimum_edition);
+  defaults.set_maximum_edition(maximum_edition);
+  auto message_factory = absl::make_unique<DynamicMessageFactory>();
+  for (const auto& edition : editions) {
+    auto defaults_dynamic =
+        absl::WrapUnique(message_factory->GetPrototype(feature_set)->New());
+    RETURN_IF_ERROR(FillDefaults(edition, *defaults_dynamic));
+    for (const auto* extension : extensions) {
+      RETURN_IF_ERROR(FillDefaults(
+          edition, *defaults_dynamic->GetReflection()->MutableMessage(
+                       defaults_dynamic.get(), extension)));
+    }
+    auto* edition_defaults = defaults.mutable_defaults()->Add();
+    edition_defaults->set_edition(edition);
+    edition_defaults->mutable_features()->MergeFromString(
+        defaults_dynamic->SerializeAsString());
+  }
+  return defaults;
 }
 
-absl::Status FeatureResolver::RegisterExtensions(const FileDescriptor& file) {
-  for (int i = 0; i < file.extension_count(); ++i) {
-    RETURN_IF_ERROR(RegisterExtension(*file.extension(i)));
+absl::StatusOr<FeatureResolver> FeatureResolver::Create(
+    absl::string_view edition, const FeatureSetDefaults& compiled_defaults) {
+  if (EditionsLessThan()(edition, compiled_defaults.minimum_edition())) {
+    return Error("Edition ", edition,
+                 " is earlier than the minimum supported edition ",
+                 compiled_defaults.minimum_edition());
   }
-  for (int i = 0; i < file.message_type_count(); ++i) {
-    RETURN_IF_ERROR(RegisterExtensions(*file.message_type(i)));
+  if (EditionsLessThan()(compiled_defaults.maximum_edition(), edition)) {
+    return Error("Edition ", edition,
+                 " is later than the maximum supported edition ",
+                 compiled_defaults.maximum_edition());
   }
-  return absl::OkStatus();
-}
 
-absl::Status FeatureResolver::RegisterExtensions(const Descriptor& message) {
-  for (int i = 0; i < message.extension_count(); ++i) {
-    RETURN_IF_ERROR(RegisterExtension(*message.extension(i)));
+  // Validate compiled defaults.
+  absl::string_view prev_edition;
+  for (const auto& edition_default : compiled_defaults.defaults()) {
+    if (!prev_edition.empty()) {
+      if (!EditionsLessThan()(prev_edition, edition_default.edition())) {
+        return Error(
+            "Feature set defaults are not strictly increasing.  Edition ",
+            prev_edition, " is greater than or equal to edition ",
+            edition_default.edition(), ".");
+      }
+    }
+    prev_edition = edition_default.edition();
   }
-  for (int i = 0; i < message.nested_type_count(); ++i) {
-    RETURN_IF_ERROR(RegisterExtensions(*message.nested_type(i)));
+
+  // Select the matching edition defaults.
+  auto comparator = [](const auto& a, const auto& b) {
+    return EditionsLessThan()(a.edition(), b.edition());
+  };
+  FeatureSetDefaults::FeatureSetEditionDefault search;
+  search.set_edition(edition);
+  auto first_nonmatch =
+      absl::c_upper_bound(compiled_defaults.defaults(), search, comparator);
+  if (first_nonmatch == compiled_defaults.defaults().begin()) {
+    return Error("No valid default found for edition ", edition);
   }
-  return absl::OkStatus();
+
+  return FeatureResolver(std::prev(first_nonmatch)->features());
 }
 
 absl::StatusOr<FeatureSet> FeatureResolver::MergeFeatures(
     const FeatureSet& merged_parent, const FeatureSet& unmerged_child) const {
-  FeatureSet merged = generated_defaults_;
-
-  // Our defaults are a dynamic message located in the build pool, while the
-  // returned object here is in the generated pool.  We parse/serialize to
-  // convert from the potentially skewed FeatureSet descriptors.
-  ABSL_CHECK(merged.MergeFromString(defaults_->SerializeAsString()));
+  FeatureSet merged = defaults_;
   merged.MergeFrom(merged_parent);
   merged.MergeFrom(unmerged_child);
 

src/google/protobuf/feature_resolver.h

@@ -39,6 +39,7 @@
 #include "absl/status/status.h"
 #include "absl/status/statusor.h"
 #include "absl/strings/string_view.h"
+#include "absl/types/span.h"
 #include "google/protobuf/descriptor.h"
 #include "google/protobuf/descriptor.pb.h"
 #include "google/protobuf/dynamic_message.h"
@@ -56,48 +57,32 @@ class PROTOBUF_EXPORT FeatureResolver {
   FeatureResolver(FeatureResolver&&) = default;
   FeatureResolver& operator=(FeatureResolver&&) = delete;
 
-  // Creates a new FeatureResolver at a specific edition.  This validates the
-  // built-in features for the given edition, and calculates the default feature
-  // set.
-  static absl::StatusOr<FeatureResolver> Create(
-      absl::string_view edition, const Descriptor* descriptor,
-      const DescriptorPool* generated_pool =
-          DescriptorPool::internal_generated_pool());
+  // Compiles a set of FeatureSet extensions into a mapping of edition to unique
+  // defaults.  This is the most complicated part of feature resolution, and by
+  // abstracting this out into an intermediate message, we can make feature
+  // resolution significantly more portable.
+  static absl::StatusOr<FeatureSetDefaults> CompileDefaults(
+      const Descriptor* feature_set,
+      absl::Span<const FieldDescriptor* const> extensions,
+      absl::string_view minimum_edition, absl::string_view maximum_edition);
 
-  // Registers a potential extension of the FeatureSet proto.  Any visible
-  // extensions will be used during merging.  Returns an error if the extension
-  // is a feature extension but fails validation.
-  absl::Status RegisterExtensions(const FileDescriptor& file);
+  // Creates a new FeatureResolver at a specific edition.  This calculates the
+  // default feature set for that edition, using the output of CompileDefaults.
+  static absl::StatusOr<FeatureResolver> Create(
+      absl::string_view edition, const FeatureSetDefaults& defaults);
 
   // Creates a new feature set using inheritance and default behavior. This is
   // designed to be called recursively, and the parent feature set is expected
-  // to be a fully merged one.
-  // The returned FeatureSet will be fully resolved for any extensions that were
-  // explicitly registered (in the custom pool) or linked into this binary (in
-  // the generated pool).
+  // to be a fully merged one.  The returned FeatureSet will be fully resolved
+  // for any extensions that were used to construct the defaults.
   absl::StatusOr<FeatureSet> MergeFeatures(
       const FeatureSet& merged_parent, const FeatureSet& unmerged_child) const;
 
  private:
-  FeatureResolver(absl::string_view edition, const Descriptor& descriptor,
-                  std::unique_ptr<DynamicMessageFactory> message_factory,
-                  std::unique_ptr<Message> defaults,
-                  FeatureSet generated_defaults)
-      : edition_(edition),
-        descriptor_(descriptor),
-        message_factory_(std::move(message_factory)),
-        defaults_(std::move(defaults)),
-        generated_defaults_(std::move(generated_defaults)) {}
-
-  absl::Status RegisterExtensions(const Descriptor& message);
-  absl::Status RegisterExtension(const FieldDescriptor& extension);
+  explicit FeatureResolver(FeatureSet defaults)
+      : defaults_(std::move(defaults)) {}
 
-  std::string edition_;
-  const Descriptor& descriptor_;
-  absl::flat_hash_set<const FieldDescriptor*> extensions_;
-  std::unique_ptr<DynamicMessageFactory> message_factory_;
-  std::unique_ptr<Message> defaults_;
-  FeatureSet generated_defaults_;
+  FeatureSet defaults_;
 };
 
 }  // namespace protobuf

src/google/protobuf/feature_resolver_test.cc

@@ -30,10 +30,14 @@
 
 #include "google/protobuf/feature_resolver.h"
 
+#include <utility>
 #include <vector>
 
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
+#include "absl/log/absl_check.h"
+#include "absl/log/absl_log.h"
+#include "absl/log/die_if_null.h"
 #include "absl/memory/memory.h"
 #include "absl/status/status.h"
 #include "absl/status/statusor.h"
@@ -43,6 +47,7 @@
 #include "google/protobuf/descriptor.h"
 #include "google/protobuf/descriptor.pb.h"
 #include "google/protobuf/io/tokenizer.h"
+#include "google/protobuf/io/zero_copy_stream_impl_lite.h"
 #include "google/protobuf/test_textproto.h"
 #include "google/protobuf/text_format.h"
 #include "google/protobuf/unittest_custom_options.pb.h"
@@ -61,7 +66,6 @@ using ::testing::ExplainMatchResult;
 using ::testing::HasSubstr;
 
 // TODO(b/234474291): Use the gtest versions once that's available in OSS.
-absl::Status GetStatus(const absl::Status& s) { return s; }
 template <typename T>
 absl::Status GetStatus(const absl::StatusOr<T>& s) {
   return s.status();
@@ -81,45 +85,28 @@ MATCHER_P(StatusIs, status,
 #define ASSERT_OK(x) ASSERT_THAT(x, StatusIs(absl::StatusCode::kOk))
 
 template <typename ExtensionT>
-const FileDescriptor& GetExtensionFile(
+const FieldDescriptor* GetExtension(
     const ExtensionT& ext,
     const Descriptor* descriptor = FeatureSet::descriptor()) {
-  return *DescriptorPool::generated_pool()
-              ->FindExtensionByNumber(descriptor, ext.number())
-              ->file();
+  return ABSL_DIE_IF_NULL(descriptor->file()->pool()->FindExtensionByNumber(
+      descriptor, ext.number()));
 }
 
-const DescriptorPool* EmptyPool() {
-  static DescriptorPool* empty_pool = new DescriptorPool();
-  return empty_pool;
-}
-
-template <typename... Extensions>
-absl::StatusOr<FeatureResolver> SetupFeatureResolverImpl(
-    absl::string_view edition, const DescriptorPool* pool,
-    Extensions... extensions) {
-  auto resolver =
-      FeatureResolver::Create(edition, FeatureSet::descriptor(), pool);
-  RETURN_IF_ERROR(resolver.status());
-  std::vector<absl::Status> results{
-      resolver->RegisterExtensions(GetExtensionFile(extensions))...};
-  for (absl::Status result : results) {
-    RETURN_IF_ERROR(result);
-  }
-  return resolver;
-}
 template <typename... Extensions>
 absl::StatusOr<FeatureResolver> SetupFeatureResolver(absl::string_view edition,
                                                      Extensions... extensions) {
-  return SetupFeatureResolverImpl(edition, EmptyPool(), extensions...);
+  absl::StatusOr<FeatureSetDefaults> defaults =
+      FeatureResolver::CompileDefaults(FeatureSet::descriptor(),
+                                       {GetExtension(extensions)...}, "2023",
+                                       "2023");
+  RETURN_IF_ERROR(defaults.status());
+  return FeatureResolver::Create(edition, *defaults);
 }
 
-template <typename... Extensions>
-absl::StatusOr<FeatureSet> GetDefaultsImpl(absl::string_view edition,
-                                           const DescriptorPool* pool,
-                                           Extensions... extensions) {
+absl::StatusOr<FeatureSet> GetDefaults(absl::string_view edition,
+                                       const FeatureSetDefaults& defaults) {
   absl::StatusOr<FeatureResolver> resolver =
-      SetupFeatureResolverImpl(edition, pool, extensions...);
+      FeatureResolver::Create(edition, defaults);
   RETURN_IF_ERROR(resolver.status());
   FeatureSet parent, child;
   return resolver->MergeFeatures(parent, child);
@@ -128,7 +115,12 @@ absl::StatusOr<FeatureSet> GetDefaultsImpl(absl::string_view edition,
 template <typename... Extensions>
 absl::StatusOr<FeatureSet> GetDefaults(absl::string_view edition,
                                        Extensions... extensions) {
-  return GetDefaultsImpl(edition, EmptyPool(), extensions...);
+  absl::StatusOr<FeatureSetDefaults> defaults =
+      FeatureResolver::CompileDefaults(FeatureSet::descriptor(),
+                                       {GetExtension(extensions)...}, "0",
+                                       "99999");
+  RETURN_IF_ERROR(defaults.status());
+  return GetDefaults(edition, *defaults);
 }
 
 FileDescriptorProto GetProto(const FileDescriptor* file) {
@@ -184,8 +176,10 @@ TEST(FeatureResolverTest, DefaultsTestMessageExtension) {
   EXPECT_EQ(merged->enum_type(), FeatureSet::OPEN);
   EXPECT_EQ(merged->repeated_field_encoding(), FeatureSet::PACKED);
   EXPECT_EQ(merged->message_encoding(), FeatureSet::LENGTH_PREFIXED);
+  EXPECT_FALSE(merged->HasExtension(pb::test));
 
-  const pb::TestFeatures& ext = merged->GetExtension(pb::test);
+  const pb::TestFeatures& ext =
+      merged->GetExtension(pb::TestMessage::test_message);
   EXPECT_EQ(ext.int_file_feature(), 1);
   EXPECT_EQ(ext.int_extension_range_feature(), 1);
   EXPECT_EQ(ext.int_message_feature(), 1);
@@ -212,8 +206,10 @@ TEST(FeatureResolverTest, DefaultsTestNestedExtension) {
   EXPECT_EQ(merged->enum_type(), FeatureSet::OPEN);
   EXPECT_EQ(merged->repeated_field_encoding(), FeatureSet::PACKED);
   EXPECT_EQ(merged->message_encoding(), FeatureSet::LENGTH_PREFIXED);
+  EXPECT_FALSE(merged->HasExtension(pb::test));
 
-  const pb::TestFeatures& ext = merged->GetExtension(pb::test);
+  const pb::TestFeatures& ext =
+      merged->GetExtension(pb::TestMessage::Nested::test_nested);
   EXPECT_EQ(ext.int_file_feature(), 1);
   EXPECT_EQ(ext.int_extension_range_feature(), 1);
   EXPECT_EQ(ext.int_message_feature(), 1);
@@ -238,25 +234,19 @@ TEST(FeatureResolverTest, DefaultsGeneratedPoolCustom) {
       nullptr);
   ASSERT_NE(pool.BuildFile(GetProto(pb::TestFeatures::descriptor()->file())),
             nullptr);
-  absl::StatusOr<FeatureSet> merged = GetDefaultsImpl("2023", &pool);
-  ASSERT_OK(merged);
+  absl::StatusOr<FeatureSetDefaults> defaults =
+      FeatureResolver::CompileDefaults(
+          pool.FindMessageTypeByName("google.protobuf.FeatureSet"),
+          {pool.FindExtensionByName("pb.test")}, "2023", "2023");
+  ASSERT_OK(defaults);
+  ASSERT_EQ(defaults->defaults().size(), 1);
+  ASSERT_EQ(defaults->defaults().at(0).edition(), "2023");
+  FeatureSet merged = defaults->defaults().at(0).features();
 
-  EXPECT_EQ(merged->field_presence(), FeatureSet::EXPLICIT);
-  EXPECT_TRUE(merged->HasExtension(pb::test));
-  EXPECT_EQ(merged->GetExtension(pb::test).int_file_feature(), 1);
-  EXPECT_FALSE(merged->HasExtension(pb::cpp));
-}
-
-TEST(FeatureResolverTest, DefaultsGeneratedPoolGenerated) {
-  absl::StatusOr<FeatureSet> merged =
-      GetDefaultsImpl("2023", DescriptorPool::generated_pool());
-  ASSERT_OK(merged);
-
-  EXPECT_EQ(merged->field_presence(), FeatureSet::EXPLICIT);
-  EXPECT_FALSE(merged->HasExtension(pb::test));
-  EXPECT_TRUE(merged->HasExtension(pb::cpp));
-  EXPECT_EQ(merged->GetExtension(pb::cpp).utf8_validation(),
-            pb::CppFeatures::VERIFY_PARSE);
+  EXPECT_EQ(merged.field_presence(), FeatureSet::EXPLICIT);
+  EXPECT_TRUE(merged.HasExtension(pb::test));
+  EXPECT_EQ(merged.GetExtension(pb::test).int_file_feature(), 1);
+  EXPECT_FALSE(merged.HasExtension(pb::cpp));
 }
 
 TEST(FeatureResolverTest, DefaultsTooEarly) {
@@ -328,28 +318,48 @@ TEST(FeatureResolverTest, DefaultsMessageMerge) {
   }
 }
 
-TEST(FeatureResolverTest, InitializePoolMissingDescriptor) {
-  DescriptorPool pool;
-  EXPECT_THAT(FeatureResolver::Create("2023", nullptr, EmptyPool()),
-              HasError(HasSubstr("find definition of google.protobuf.FeatureSet")));
+TEST(FeatureResolverTest, CreateFromUnsortedDefaults) {
+  FeatureSetDefaults defaults = ParseTextOrDie(R"pb(
+    minimum_edition: "2022"
+    maximum_edition: "2024"
+    defaults {
+      edition: "2022"
+      features {}
+    }
+    defaults {
+      edition: "2024"
+      features {}
+    }
+    defaults {
+      edition: "2023"
+      features {}
+    }
+  )pb");
+  EXPECT_THAT(
+      FeatureResolver::Create("2023", defaults),
+      HasError(AllOf(
+          HasSubstr("not strictly increasing."),
+          HasSubstr("Edition 2024 is greater than or equal to edition 2023"))));
 }
 
-TEST(FeatureResolverTest, RegisterExtensionDifferentContainingType) {
-  auto resolver =
-      FeatureResolver::Create("2023", FeatureSet::descriptor(), EmptyPool());
-  ASSERT_OK(resolver);
-
-  EXPECT_OK(resolver->RegisterExtensions(
-      GetExtensionFile(protobuf_unittest::file_opt1, FileOptions::descriptor())));
+TEST(FeatureResolverTest, CompileDefaultsMissingDescriptor) {
+  EXPECT_THAT(FeatureResolver::CompileDefaults(nullptr, {}, "2023", "2023"),
+              HasError(HasSubstr("find definition of google.protobuf.FeatureSet")));
 }
 
-TEST(FeatureResolverTest, RegisterExtensionTwice) {
-  auto resolver =
-      FeatureResolver::Create("2023", FeatureSet::descriptor(), EmptyPool());
-  ASSERT_OK(resolver);
+TEST(FeatureResolverTest, CompileDefaultsMissingExtension) {
+  EXPECT_THAT(FeatureResolver::CompileDefaults(FeatureSet::descriptor(),
+                                               {nullptr}, "2023", "2023"),
+              HasError(HasSubstr("Unknown extension")));
+}
 
-  EXPECT_OK(resolver->RegisterExtensions(GetExtensionFile(pb::test)));
-  EXPECT_OK(resolver->RegisterExtensions(GetExtensionFile(pb::test)));
+TEST(FeatureResolverTest, CompileDefaultsInvalidExtension) {
+  EXPECT_THAT(
+      FeatureResolver::CompileDefaults(
+          FeatureSet::descriptor(),
+          {GetExtension(protobuf_unittest::file_opt1, FileOptions::descriptor())},
+          "2023", "2023"),
+      HasError(HasSubstr("is not an extension of")));
 }
 
 TEST(FeatureResolverTest, MergeFeaturesChildOverrideCore) {
@@ -508,13 +518,17 @@ TEST(FeatureResolverTest, MergeFeaturesExtensionEnumUnknown) {
             pb::TestFeatures::TEST_ENUM_FEATURE_UNKNOWN);
 }
 
-// TODO(b/273611177) Move the following tests to descriptor_unittest.cc once
-// FeatureResolver is hooked up.  These will all fail during the descriptor
-// build, invalidating the test setup.
-//
-// Note: We should make sure to keep coverage over the custom DescriptorPool
-// cases.  These are the only tests covering issues there (the ones above use
-// the generated descriptor pool).
+TEST(FeatureResolverTest, MergeFeaturesDistantPast) {
+  EXPECT_THAT(SetupFeatureResolver("1000"),
+              HasError(AllOf(HasSubstr("Edition 1000"),
+                             HasSubstr("minimum supported edition 2023"))));
+}
+
+TEST(FeatureResolverTest, MergeFeaturesDistantFuture) {
+  EXPECT_THAT(SetupFeatureResolver("9999"),
+              HasError(AllOf(HasSubstr("Edition 9999"),
+                             HasSubstr("maximum supported edition 2023"))));
+}
 
 class FakeErrorCollector : public io::ErrorCollector {
  public:
@@ -534,6 +548,12 @@ class FeatureResolverPoolTest : public testing::Test {
     FileDescriptorProto file;
     FileDescriptorProto::GetDescriptor()->file()->CopyTo(&file);
     ASSERT_NE(pool_.BuildFile(file), nullptr);
+    feature_set_ = pool_.FindMessageTypeByName("google.protobuf.FeatureSet");
+    ASSERT_NE(feature_set_, nullptr);
+    auto defaults =
+        FeatureResolver::CompileDefaults(feature_set_, {}, "2023", "2023");
+    ASSERT_OK(defaults);
+    defaults_ = std::move(defaults).value();
   }
 
   const FileDescriptor* ParseSchema(absl::string_view schema) {
@@ -552,9 +572,11 @@ class FeatureResolverPoolTest : public testing::Test {
 
   DescriptorPool pool_;
   FileDescriptorProto file_proto_;
+  const Descriptor* feature_set_;
+  FeatureSetDefaults defaults_;
 };
 
-TEST_F(FeatureResolverPoolTest, RegisterExtensionNonMessage) {
+TEST_F(FeatureResolverPoolTest, CompileDefaultsInvalidNonMessage) {
   const FileDescriptor* file = ParseSchema(R"schema(
     syntax = "proto2";
     package test;
@@ -567,16 +589,14 @@ TEST_F(FeatureResolverPoolTest, RegisterExtensionNonMessage) {
   )schema");
   ASSERT_NE(file, nullptr);
 
-  auto resolver = FeatureResolver::Create(
-      "2023", pool_.FindMessageTypeByName("google.protobuf.FeatureSet"), EmptyPool());
-  ASSERT_OK(resolver);
-
-  EXPECT_THAT(resolver->RegisterExtensions(*file),
-              HasError(AllOf(HasSubstr("test.bar"),
-                             HasSubstr("is not of message type"))));
+  const FieldDescriptor* ext = file->extension(0);
+  EXPECT_THAT(
+      FeatureResolver::CompileDefaults(feature_set_, {ext}, "2023", "2023"),
+      HasError(
+          AllOf(HasSubstr("test.bar"), HasSubstr("is not of message type"))));
 }
 
-TEST_F(FeatureResolverPoolTest, RegisterExtensionRepeated) {
+TEST_F(FeatureResolverPoolTest, CompileDefaultsInvalidRepeated) {
   const FileDescriptor* file = ParseSchema(R"schema(
     syntax = "proto2";
     package test;
@@ -589,16 +609,13 @@ TEST_F(FeatureResolverPoolTest, RegisterExtensionRepeated) {
   )schema");
   ASSERT_NE(file, nullptr);
 
-  auto resolver = FeatureResolver::Create(
-      "2023", pool_.FindMessageTypeByName("google.protobuf.FeatureSet"), EmptyPool());
-  ASSERT_OK(resolver);
-
+  const FieldDescriptor* ext = file->extension(0);
   EXPECT_THAT(
-      resolver->RegisterExtensions(*file),
+      FeatureResolver::CompileDefaults(feature_set_, {ext}, "2023", "2023"),
       HasError(AllOf(HasSubstr("test.bar"), HasSubstr("repeated extension"))));
 }
 
-TEST_F(FeatureResolverPoolTest, RegisterExtensionWithExtensions) {
+TEST_F(FeatureResolverPoolTest, CompileDefaultsInvalidWithExtensions) {
   const FileDescriptor* file = ParseSchema(R"schema(
     syntax = "proto2";
     package test;
@@ -619,16 +636,13 @@ TEST_F(FeatureResolverPoolTest, RegisterExtensionWithExtensions) {
   )schema");
   ASSERT_NE(file, nullptr);
 
-  auto resolver = FeatureResolver::Create(
-      "2023", pool_.FindMessageTypeByName("google.protobuf.FeatureSet"), EmptyPool());
-  ASSERT_OK(resolver);
-
+  const FieldDescriptor* ext = file->extension(0);
   EXPECT_THAT(
-      resolver->RegisterExtensions(*file),
+      FeatureResolver::CompileDefaults(feature_set_, {ext}, "2023", "2023"),
       HasError(AllOf(HasSubstr("test.bar"), HasSubstr("Nested extensions"))));
 }
 
-TEST_F(FeatureResolverPoolTest, RegisterExtensionWithOneof) {
+TEST_F(FeatureResolverPoolTest, CompileDefaultsInvalidWithOneof) {
   const FileDescriptor* file = ParseSchema(R"schema(
     syntax = "proto2";
     package test;
@@ -652,16 +666,14 @@ TEST_F(FeatureResolverPoolTest, RegisterExtensionWithOneof) {
   )schema");
   ASSERT_NE(file, nullptr);
 
-  auto resolver = FeatureResolver::Create(
-      "2023", pool_.FindMessageTypeByName("google.protobuf.FeatureSet"), EmptyPool());
-  ASSERT_OK(resolver);
-
-  EXPECT_THAT(resolver->RegisterExtensions(*file),
-              HasError(AllOf(HasSubstr("test.Foo"),
-                             HasSubstr("oneof feature fields"))));
+  const FieldDescriptor* ext = file->extension(0);
+  EXPECT_THAT(
+      FeatureResolver::CompileDefaults(feature_set_, {ext}, "2023", "2023"),
+      HasError(
+          AllOf(HasSubstr("test.Foo"), HasSubstr("oneof feature fields"))));
 }
 
-TEST_F(FeatureResolverPoolTest, RegisterExtensionWithRequired) {
+TEST_F(FeatureResolverPoolTest, CompileDefaultsInvalidWithRequired) {
   const FileDescriptor* file = ParseSchema(R"schema(
     syntax = "proto2";
     package test;
@@ -679,16 +691,14 @@ TEST_F(FeatureResolverPoolTest, RegisterExtensionWithRequired) {
   )schema");
   ASSERT_NE(file, nullptr);
 
-  auto resolver = FeatureResolver::Create(
-      "2023", pool_.FindMessageTypeByName("google.protobuf.FeatureSet"), EmptyPool());
-  ASSERT_OK(resolver);
-
-  EXPECT_THAT(resolver->RegisterExtensions(*file),
-              HasError(AllOf(HasSubstr("test.Foo.required_field"),
-                             HasSubstr("required field"))));
+  const FieldDescriptor* ext = file->extension(0);
+  EXPECT_THAT(
+      FeatureResolver::CompileDefaults(feature_set_, {ext}, "2023", "2023"),
+      HasError(AllOf(HasSubstr("test.Foo.required_field"),
+                     HasSubstr("required field"))));
 }
 
-TEST_F(FeatureResolverPoolTest, RegisterExtensionWithRepeated) {
+TEST_F(FeatureResolverPoolTest, CompileDefaultsInvalidWithRepeated) {
   const FileDescriptor* file = ParseSchema(R"schema(
     syntax = "proto2";
     package test;
@@ -706,16 +716,14 @@ TEST_F(FeatureResolverPoolTest, RegisterExtensionWithRepeated) {
   )schema");
   ASSERT_NE(file, nullptr);
 
-  auto resolver = FeatureResolver::Create(
-      "2023", pool_.FindMessageTypeByName("google.protobuf.FeatureSet"), EmptyPool());
-  ASSERT_OK(resolver);
-
-  EXPECT_THAT(resolver->RegisterExtensions(*file),
-              HasError(AllOf(HasSubstr("test.Foo.repeated_field"),
-                             HasSubstr("repeated field"))));
+  const FieldDescriptor* ext = file->extension(0);
+  EXPECT_THAT(
+      FeatureResolver::CompileDefaults(feature_set_, {ext}, "2023", "2023"),
+      HasError(AllOf(HasSubstr("test.Foo.repeated_field"),
+                     HasSubstr("repeated field"))));
 }
 
-TEST_F(FeatureResolverPoolTest, RegisterExtensionWithMissingTarget) {
+TEST_F(FeatureResolverPoolTest, CompileDefaultsInvalidWithMissingTarget) {
   const FileDescriptor* file = ParseSchema(R"schema(
     syntax = "proto2";
     package test;
@@ -732,16 +740,15 @@ TEST_F(FeatureResolverPoolTest, RegisterExtensionWithMissingTarget) {
   )schema");
   ASSERT_NE(file, nullptr);
 
-  auto resolver = FeatureResolver::Create(
-      "2023", pool_.FindMessageTypeByName("google.protobuf.FeatureSet"), EmptyPool());
-  ASSERT_OK(resolver);
-
-  EXPECT_THAT(resolver->RegisterExtensions(*file),
-              HasError(AllOf(HasSubstr("test.Foo.int_field"),
-                             HasSubstr("no target specified"))));
+  const FieldDescriptor* ext = file->extension(0);
+  EXPECT_THAT(
+      FeatureResolver::CompileDefaults(feature_set_, {ext}, "2023", "2023"),
+      HasError(AllOf(HasSubstr("test.Foo.int_field"),
+                     HasSubstr("no target specified"))));
 }
 
-TEST_F(FeatureResolverPoolTest, RegisterExtensionDefaultsMessageParsingError) {
+TEST_F(FeatureResolverPoolTest,
+       CompileDefaultsInvalidDefaultsMessageParsingError) {
   const FileDescriptor* file = ParseSchema(R"schema(
     syntax = "proto2";
     package test;
@@ -762,16 +769,14 @@ TEST_F(FeatureResolverPoolTest, RegisterExtensionDefaultsMessageParsingError) {
   )schema");
   ASSERT_NE(file, nullptr);
 
-  auto resolver = FeatureResolver::Create(
-      "2023", pool_.FindMessageTypeByName("google.protobuf.FeatureSet"), EmptyPool());
-  ASSERT_OK(resolver);
+  const FieldDescriptor* ext = file->extension(0);
   EXPECT_THAT(
-      resolver->RegisterExtensions(*file),
+      FeatureResolver::CompileDefaults(feature_set_, {ext}, "2023", "2023"),
       HasError(AllOf(HasSubstr("in edition_defaults"), HasSubstr("9987"))));
 }
 
 TEST_F(FeatureResolverPoolTest,
-       RegisterExtensionDefaultsMessageParsingErrorMerged) {
+       CompileDefaultsInvalidDefaultsMessageParsingErrorMerged) {
   const FileDescriptor* file = ParseSchema(R"schema(
     syntax = "proto2";
     package test;
@@ -794,16 +799,14 @@ TEST_F(FeatureResolverPoolTest,
   )schema");
   ASSERT_NE(file, nullptr);
 
-  auto resolver = FeatureResolver::Create(
-      "2024", pool_.FindMessageTypeByName("google.protobuf.FeatureSet"), EmptyPool());
-  ASSERT_OK(resolver);
+  const FieldDescriptor* ext = file->extension(0);
   EXPECT_THAT(
-      resolver->RegisterExtensions(*file),
+      FeatureResolver::CompileDefaults(feature_set_, {ext}, "2023", "2025"),
       HasError(AllOf(HasSubstr("in edition_defaults"), HasSubstr("9987"))));
 }
 
 TEST_F(FeatureResolverPoolTest,
-       RegisterExtensionDefaultsMessageParsingErrorSkipped) {
+       CompileDefaultsInvalidDefaultsMessageParsingErrorSkipped) {
   const FileDescriptor* file = ParseSchema(R"schema(
     syntax = "proto2";
     package test;
@@ -826,15 +829,19 @@ TEST_F(FeatureResolverPoolTest,
   )schema");
   ASSERT_NE(file, nullptr);
 
-  auto resolver = FeatureResolver::Create(
-      "2024", pool_.FindMessageTypeByName("google.protobuf.FeatureSet"), EmptyPool());
+  const FieldDescriptor* ext = file->extension(0);
+  auto defaults =
+      FeatureResolver::CompileDefaults(feature_set_, {ext}, "2023", "2024");
+  ASSERT_OK(defaults);
+
+  auto resolver = FeatureResolver::Create("2023", *defaults);
   ASSERT_OK(resolver);
-  EXPECT_OK(resolver->RegisterExtensions(*file));
   FeatureSet parent, child;
   EXPECT_OK(resolver->MergeFeatures(parent, child));
 }
 
-TEST_F(FeatureResolverPoolTest, RegisterExtensionDefaultsScalarParsingError) {
+TEST_F(FeatureResolverPoolTest,
+       CompileDefaultsInvalidDefaultsScalarParsingError) {
   const FileDescriptor* file = ParseSchema(R"schema(
     syntax = "proto2";
     package test;
@@ -852,16 +859,14 @@ TEST_F(FeatureResolverPoolTest, RegisterExtensionDefaultsScalarParsingError) {
   )schema");
   ASSERT_NE(file, nullptr);
 
-  auto resolver = FeatureResolver::Create(
-      "2023", pool_.FindMessageTypeByName("google.protobuf.FeatureSet"), EmptyPool());
-  ASSERT_OK(resolver);
+  const FieldDescriptor* ext = file->extension(0);
   EXPECT_THAT(
-      resolver->RegisterExtensions(*file),
+      FeatureResolver::CompileDefaults(feature_set_, {ext}, "2023", "2023"),
       HasError(AllOf(HasSubstr("in edition_defaults"), HasSubstr("1.23"))));
 }
 
 TEST_F(FeatureResolverPoolTest,
-       RegisterExtensionDefaultsScalarParsingErrorSkipped) {
+       CompileDefaultsInvalidDefaultsScalarParsingErrorSkipped) {
   const FileDescriptor* file = ParseSchema(R"schema(
     syntax = "proto2";
     package test;
@@ -880,41 +885,39 @@ TEST_F(FeatureResolverPoolTest,
   )schema");
   ASSERT_NE(file, nullptr);
 
-  auto resolver = FeatureResolver::Create(
-      "2023", pool_.FindMessageTypeByName("google.protobuf.FeatureSet"), EmptyPool());
+  const FieldDescriptor* ext = file->extension(0);
+  auto defaults =
+      FeatureResolver::CompileDefaults(feature_set_, {ext}, "2023", "2023");
+  ASSERT_OK(defaults);
+
+  auto resolver = FeatureResolver::Create("2023", *defaults);
   ASSERT_OK(resolver);
-  EXPECT_OK(resolver->RegisterExtensions(*file));
   FeatureSet parent, child;
   EXPECT_OK(resolver->MergeFeatures(parent, child));
 }
 
-TEST_F(FeatureResolverPoolTest, GeneratedPoolInvalidExtension) {
+TEST_F(FeatureResolverPoolTest, CompileDefaultsInvalidDefaultsTooEarly) {
   const FileDescriptor* file = ParseSchema(R"schema(
     syntax = "proto2";
     package test;
     import "google/protobuf/descriptor.proto";
 
-    message Foo {}
     extend google.protobuf.FeatureSet {
-      optional string bar = 9999;
+      optional Foo bar = 9999;
+    }
+    message Foo {
+      optional int32 int_field_feature = 12 [
+        targets = TARGET_TYPE_FIELD,
+        edition_defaults = { edition: "2022", value: "1" }
+      ];
     }
   )schema");
   ASSERT_NE(file, nullptr);
 
-  EXPECT_THAT(GetDefaultsImpl("2023", &pool_),
-              HasError(AllOf(HasSubstr("test.bar"),
-                             HasSubstr("is not of message type"))));
-}
-
-TEST_F(FeatureResolverPoolTest, GeneratedPoolDefaultsFailure) {
-  ASSERT_NE(
-      pool_.BuildFile(GetProto(google::protobuf::DescriptorProto::descriptor()->file())),
-      nullptr);
-  ASSERT_NE(pool_.BuildFile(GetProto(pb::TestFeatures::descriptor()->file())),
-            nullptr);
+  const FieldDescriptor* ext = file->extension(0);
   EXPECT_THAT(
-      GetDefaultsImpl("2022", &pool_),
-      HasError(AllOf(HasSubstr("No valid default found"), HasSubstr("2022"))));
+      FeatureResolver::CompileDefaults(feature_set_, {ext}, "2023", "2023"),
+      HasError(HasSubstr("No valid default found for edition 2022")));
 }
 
 }  // namespace